Skip to content
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

Shortcut for Export #2979

Closed
wants to merge 1 commit into from
Closed

Shortcut for Export #2979

wants to merge 1 commit into from

Conversation

jameskerr
Copy link
Member

@jameskerr jameskerr commented Jan 19, 2024

Adds the shortcut cmd+shift+e to pull up the export dialog.

@jameskerr jameskerr requested a review from philrz January 19, 2024 23:17
@philrz
Copy link
Contributor

philrz commented Jan 19, 2024

@jameskerr: Looks great. Alas, because you've now made it so easy to quickly call up Export via the keyboard, I found myself instinctively hitting the arrow keys to move between the formats and Esc to dismiss without exporting and found a couple new bugs unrelated to this specific branch. If you'd rather I open new issues we could get to them later. But if they're easy and you're keen to fix them in this PR while you're under the hood, I would not fight that, as I suspect users named Jason might bump into them. 😄

First, I found that if I bring up Export and then immediately hit Esc, I can't seem to bring up Export again. I don't see any errors in DevTools.

Escape.mp4

Second, I found that if I bring up Export and then immediately click up/down arrows to switch formats, it jumps somewhere other than I'd expect relative to the default ZNG selection. Specifically, hitting up-arrow jumps me down to ZSON, and hitting down-arrow jumps me up to CSV.

Arrows.mp4

@philrz philrz mentioned this pull request Jan 21, 2024
@jameskerr
Copy link
Member Author

Closing this in favor of #2985 That PR will address these bugs.

@jameskerr jameskerr closed this Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants