-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Allow for Esc
to clear all filters
#625
Conversation
Nice! Our keyboard shortcuts game is getting stronger! I like it! |
that's very cool and i was thinking to share that idea. i didn't know it's a google calendar thing but last week we started using Hey for email and they do that everywhere and I really like it (just need to get used to the different keybindings) showing the keybinding in the UI makes sense to make people aware of it and remember it and i like your implementation! and just using the first letter of what the action is makes sense to me (c for custom etc). if we introduce this, i would add a keyboard shortcuts page to the docs too. |
Sounds good (I've always been meaning to try hey but ... lazy)! How do you feel about the following, then? I think some of the more obscure ones don't need a binding, but lmk if you think there should be one for any.
|
looks good to me @Vigasaurus! good that it follows google calendar as i assume that may be the standard that's used in more apps too. |
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.
Feel free to add to the 1.2 - unreleased
section on the changelog.
assets/js/dashboard/filters.js
Outdated
handleKeyup(e) { | ||
const {query, history} = this.props | ||
|
||
if (e.ctrlKey || e.ctrlKey || e.altKey) return |
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.
Seems like ctrlKey
is checked for twice
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.
Ah haha - I just copied this from the DatePicker arrows, didn't even notice 🙈
One of them should be metaKey, I believe
* Allow for `Esc` to clear all filters * Changelog * Change duplicated ctrlKey (in DatePicker too)
Changes
Resolves #575
Tests
Changelog
Same as my other PRs heh - lmk what you want for changelog format and I'll get it sorted
Documentation
🤷♂️