From ab31ad69e98386a5ed724f24f70fe2181db3469a Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Fri, 22 Jan 2021 06:09:53 -0800 Subject: [PATCH] Fix keyboard events to incorrectly listen / propagate globally Summary: As reported in https://fb.workplace.com/groups/flippersupport/permalink/1033379297142728/, using keyboard shortcuts in one component, can change the state of another. This was caused by two problems: 1. ManagedTable established a global keyboard listener, rather than a local one 2. The Layout Inspector did not stop its keyboard events from propagating up in the DOM This diff fixes both issues Reviewed By: timur-valiev Differential Revision: D26018248 fbshipit-source-id: 23d9cc38ad56d47213cb553ffaf528b05fbe1929 --- .../ui/components/elements-inspector/elements.tsx | 5 +++++ desktop/app/src/ui/components/table/ManagedTable.tsx | 12 +++++++----- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/desktop/app/src/ui/components/elements-inspector/elements.tsx b/desktop/app/src/ui/components/elements-inspector/elements.tsx index 72d2e36c2..84bd967af 100644 --- a/desktop/app/src/ui/components/elements-inspector/elements.tsx +++ b/desktop/app/src/ui/components/elements-inspector/elements.tsx @@ -571,12 +571,14 @@ export class Elements extends PureComponent { ((e.metaKey && process.platform === 'darwin') || (e.ctrlKey && process.platform !== 'darwin')) ) { + e.stopPropagation(); e.preventDefault(); clipboard.writeText(selectedElement.name); return; } if (e.key === 'ArrowUp') { + e.stopPropagation(); if (selectedIndex === 0 || flatKeys.length === 1) { return; } @@ -586,6 +588,7 @@ export class Elements extends PureComponent { } if (e.key === 'ArrowDown') { + e.stopPropagation(); if (selectedIndex === flatKeys.length - 1) { return; } @@ -595,6 +598,7 @@ export class Elements extends PureComponent { } if (e.key === 'ArrowLeft') { + e.stopPropagation(); e.preventDefault(); if (selectedElement.expanded) { // unexpand @@ -618,6 +622,7 @@ export class Elements extends PureComponent { } if (e.key === 'ArrowRight' && selectedElement.children.length > 0) { + e.stopPropagation(); e.preventDefault(); if (selectedElement.expanded) { // go to first child diff --git a/desktop/app/src/ui/components/table/ManagedTable.tsx b/desktop/app/src/ui/components/table/ManagedTable.tsx index 92137fe83..d258e7a09 100644 --- a/desktop/app/src/ui/components/table/ManagedTable.tsx +++ b/desktop/app/src/ui/components/table/ManagedTable.tsx @@ -214,8 +214,6 @@ export class ManagedTable extends React.Component< } componentDidMount() { - document.addEventListener('keydown', this.onKeyDown); - if (typeof this.props.innerRef === 'function') { this.props.innerRef(this); } else if (this.props.innerRef) { @@ -224,7 +222,6 @@ export class ManagedTable extends React.Component< } componentWillUnmount() { - document.removeEventListener('keydown', this.onKeyDown); if (typeof this.props.innerRef === 'function') { this.props.innerRef(undefined); } else if (this.props.innerRef) { @@ -330,7 +327,7 @@ export class ManagedTable extends React.Component< ); }; - onKeyDown = (e: KeyboardEvent) => { + onKeyDown = (e: React.KeyboardEvent) => { const {highlightedRows} = this.state; if (highlightedRows.size === 0) { return; @@ -340,12 +337,14 @@ export class ManagedTable extends React.Component< (e.ctrlKey && process.platform !== 'darwin')) && e.keyCode === 67 ) { + e.stopPropagation(); this.onCopy(false); } else if ( (e.keyCode === 38 || e.keyCode === 40) && this.props.highlightableRows && this.props.enableKeyboardNavigation ) { + e.stopPropagation(); // arrow navigation const {rows} = this.props; const {highlightedRows} = this.state; @@ -696,7 +695,10 @@ export class ManagedTable extends React.Component< } return ( - + {hideHeader !== true && (