Skip to content

Commit 9617f02

Browse files
Brian Vaughnzhengjitf
Brian Vaughn
authored andcommitted
Attach DevTools Tree keyboard events to the Tree container (not the document) (facebook#24164)
We used to listen to at the document level for this event. That allowed us to listen to up/down arrow key events while another section of DevTools (like the search input) was focused. This was a minor UX positive. (We had to use ownerDocument rather than document for this, because the DevTools extension renders the Components and Profiler tabs into portals.) This approach caused a problem though: it meant that a react-devtools-inline instance could steal (and prevent/block) keyboard events from other JavaScript on the page– which could even include other react-devtools-inline instances. This is a potential major UX negative. Given the above trade offs, we now listen on the root of the Tree itself.
1 parent 2116bdc commit 9617f02

File tree

1 file changed

+17
-10
lines changed
  • packages/react-devtools-shared/src/devtools/views/Components

1 file changed

+17
-10
lines changed

packages/react-devtools-shared/src/devtools/views/Components/Tree.js

+17-10
Original file line numberDiff line numberDiff line change
@@ -126,10 +126,6 @@ export default function Tree(props: Props) {
126126
return;
127127
}
128128

129-
// TODO We should ignore arrow keys if the focus is outside of DevTools.
130-
// Otherwise the inline (embedded) DevTools might change selection unexpectedly,
131-
// e.g. when a text input or a select has focus.
132-
133129
let element;
134130
switch (event.key) {
135131
case 'ArrowDown':
@@ -192,14 +188,25 @@ export default function Tree(props: Props) {
192188
setIsNavigatingWithKeyboard(true);
193189
};
194190

195-
// It's important to listen to the ownerDocument to support the browser extension.
196-
// Here we use portals to render individual tabs (e.g. Profiler),
197-
// and the root document might belong to a different window.
198-
const ownerDocument = treeRef.current.ownerDocument;
199-
ownerDocument.addEventListener('keydown', handleKeyDown);
191+
// We used to listen to at the document level for this event.
192+
// That allowed us to listen to up/down arrow key events while another section
193+
// of DevTools (like the search input) was focused.
194+
// This was a minor UX positive.
195+
//
196+
// (We had to use ownerDocument rather than document for this, because the
197+
// DevTools extension renders the Components and Profiler tabs into portals.)
198+
//
199+
// This approach caused a problem though: it meant that a react-devtools-inline
200+
// instance could steal (and prevent/block) keyboard events from other JavaScript
201+
// on the page– which could even include other react-devtools-inline instances.
202+
// This is a potential major UX negative.
203+
//
204+
// Given the above trade offs, we now listen on the root of the Tree itself.
205+
const container = treeRef.current;
206+
container.addEventListener('keydown', handleKeyDown);
200207

201208
return () => {
202-
ownerDocument.removeEventListener('keydown', handleKeyDown);
209+
container.removeEventListener('keydown', handleKeyDown);
203210
};
204211
}, [dispatch, selectedElementID, store]);
205212

0 commit comments

Comments
 (0)