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

Fix keyboard shortcut bindings #2873

Merged
merged 1 commit into from
Nov 24, 2020
Merged

Fix keyboard shortcut bindings #2873

merged 1 commit into from
Nov 24, 2020

Conversation

arthurperton
Copy link
Contributor

This PR fixes #1794. It also fixes the same issue in one other component.

@jasonvarga
Copy link
Member

The change to Folder.vue: in what situation was something broken? Seems fine already to me.

@arthurperton
Copy link
Contributor Author

I searched the entire project and this Folder component was the one other place where this approach was used.

I see it as a problem waiting to happen. Using the stop method in this way is rather radical: it kills any other shortcut key binding on the page. Even in stacks. (And when it happens, the cause is kind of hard to find, as we experienced.)

The comment above the code says it is used to make the binding work in focused input fields too. Which is exactly what bindGlobal does, and it does it without interfering with other keybindings.

That’s my reason for replacing it in the Folder component too.

@jasonvarga
Copy link
Member

So you didn't actually run into an issue, you just found some similar code. Fair enough.

@arthurperton
Copy link
Contributor Author

Yeah that's it

@jasonvarga jasonvarga merged commit 9f562ef into statamic:master Nov 24, 2020
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.

Cmd-S not tied to Save on Navigation CP
2 participants