Removed ESC from global shortcuts to locally handle it in dialog. #168
Conversation
Signed-off-by: ubi de feo <me@ubidefeo.com>
| // if (key === shortcuts.ESC) { | ||
| // if (state.isConnectionDialogOpen) { | ||
| // emitter.emit('close-connection-dialog') | ||
| // } | ||
| // } |
There was a problem hiding this comment.
Please remove dead code
| if (state.isConnectionDialogOpen) { | ||
| document.addEventListener('keydown', onKeyDown) | ||
| } else { | ||
| document.removeEventListener('keydown', onKeyDown) | ||
| } |
There was a problem hiding this comment.
This doesn't work. A new ConnectionDialog instance is created on each render and the deregistration of the event listener doesn't catch that. As a result whenever you open the connection dialog a new event handler is added to the list which results not just in a memory leak but causes close-connection-dialog to fire multiple times.
There was a problem hiding this comment.
didn't think of that.
I believe if I handle it outside of the instance of ConnectDialog I should be able to remove it when it's destroyed.
Will investigate.
For now PR goes into Draft
|
Plus I would use the key code to compare the pressed key |
|
These changes are now part of a new branch |
I introduced a bug setting
ESCas a global shortcut.The
onKeyDownevent in the file-list would not be passed to the<input>field with the file/folder name to trigger a.blur()and cancel the creation of a file/folder