-
Notifications
You must be signed in to change notification settings - Fork 3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[No QA] Simplify and upgrade KeyboardShortcut library #8059
Changes from 13 commits
18be748
7f88aaf
af52c65
6c29498
03fcb04
80d4c0a
56e717c
acbf212
7db67a4
d31da7c
b31d30a
5ba4816
b179109
fe4467a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -48,6 +48,10 @@ const propTypes = { | |
/** Call the onPress function when Enter key is pressed */ | ||
pressOnEnter: PropTypes.bool, | ||
|
||
/** The priority to assign the enter key event listener. 0 is the highest priority. */ | ||
enterKeyEventListenerPriority: PropTypes.number, | ||
|
||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @roryabraham extra line breaker. (We really need a eslint rule for this) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there somewhere where we've explicitly added this to the review guidelines? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because we can easily turn on https://eslint.org/docs/rules/no-multiple-empty-lines There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Nope. Yeah, let's add that rule. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @roryabraham I'm gonna create a quick PR for eslint-config-expensify to turn it on. |
||
/** Additional styles to add after local styles. Applied to Pressable portion of button */ | ||
style: PropTypes.oneOfType([ | ||
PropTypes.arrayOf(PropTypes.object), | ||
|
@@ -92,6 +96,7 @@ const defaultProps = { | |
onPressIn: () => {}, | ||
onPressOut: () => {}, | ||
pressOnEnter: false, | ||
enterKeyEventListenerPriority: 0, | ||
style: [], | ||
innerStyles: [], | ||
textStyles: [], | ||
|
@@ -124,7 +129,7 @@ class Button extends Component { | |
return; | ||
} | ||
this.props.onPress(); | ||
}, shortcutConfig.descriptionKey, shortcutConfig.modifiers, true); | ||
}, shortcutConfig.descriptionKey, shortcutConfig.modifiers, true, false, this.props.enterKeyEventListenerPriority); | ||
} | ||
|
||
componentWillUnmount() { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small question: How come this is called the META key instead of COMMAND?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IDK