fix(FEC-9450): keyboard prevent non relevant events #440
Conversation
Speical key like alt,cntrl and cmd didn't work with keys that already maped. prevent only combine keys with handlers.
src/components/keyboard/keyboard.js
Outdated
| @@ -105,8 +104,8 @@ class Keyboard extends Component { | |||
| * | |||
| * @memberof Keyboard | |||
| */ | |||
| keyboardHandlers: {[key: number]: Function} = { | |||
| [KeyMap.SPACE]: () => { | |||
| keyboardHandlers: {[key: number]: (event: KeyboardEvent) => {preventDefault: boolean, payload: any}} = { | |||
There was a problem hiding this comment.
extract to type {preventDefault: boolean, payload: any}
| @@ -0,0 +1,5 @@ | |||
| // @flow | |||
| declare type EventHandlerOption = { | |||
There was a problem hiding this comment.
this name is extremely generic and will apply to anything which is an event, while it is a specific form for a specific handler.
| declare type EventHandlerOption = { | |
| declare type KeyboardEventResult = { |
There was a problem hiding this comment.
we could use it in other cases like this, why do you want to rename it to specific existing use case?
There was a problem hiding this comment.
cause it is specific, where else will you have prevent default and payload response?
The response object is tightly coupled with the implementation of this class
There was a problem hiding this comment.
we didn't use, it could be used on the future..
it's use case I think.
There was a problem hiding this comment.
unless you see a concrete use case then it is specific.
refactor to generalize a use case later is always possible, but there is no benefit or use case here.
Let's change it to fit the part.
src/components/keyboard/keyboard.js
Outdated
| } | ||
| return {preventDefault: false, payload: null}; |
There was a problem hiding this comment.
if {preventDefault: false, payload: null} repeats everywhere extract it to a const and use that.
const unhandledKeyboardEventResult = {preventDefault: false, payload: null};
src/components/keyboard/keyboard.js
Outdated
| * @memberof Keyboard | ||
| */ | ||
| get unhandledKeyboardEventResult(): KeyboardEventResult { | ||
| return {preventDefault: false, payload: null}; |
There was a problem hiding this comment.
as it static const make it a const unhandledKeyboardEventResult: KeyboardEventResult = {preventDefault: false, payload: null};
place it in top of file and use it internally.
Description of the Changes
Special key like alt, ctrl and cmd didn't work with keys that already mapped.
prevent only combine keys with handlers.
CheckLists