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 Esc key closing Sheets #9969

Merged
merged 6 commits into from
Oct 18, 2023
Merged

Fix Esc key closing Sheets #9969

merged 6 commits into from
Oct 18, 2023

Conversation

bhollis
Copy link
Contributor

@bhollis bhollis commented Oct 12, 2023

I noticed that in some cases, sheets couldn't be closed in order by hitting "Esc" - for example, opening a loadout edit sheet, then clicking show mod placement, then hitting Esc wouldn't do anything.

The reason is that the parent sheet would re-render, which would create a new hotkey callback, which would unregister the old callback and register a new one at the front of the hotkey stack.

To fix this, I now track hotkeys by ID so they are always ordered by when they were first inserted, and added some tests.

This does not yet address #6246 since esc to unfocus only works on macOS.

@robojumper
Copy link
Member

by allowing esc to unfocus inputs

this part does not work for me in Firefox and Edge Win10.

@bhollis
Copy link
Contributor Author

bhollis commented Oct 12, 2023

Well I guess more specifically it allows escape to do whatever your platform thinks escape should do.

@robojumper
Copy link
Member

The linked issue is asking for this on Chrome + Win10 - is that something we'd still want to add to (sheet) inputs in DIM or are we happy with consistency with platform behavior?

@bhollis
Copy link
Contributor Author

bhollis commented Oct 12, 2023

Probably I'll consider this not to close that issue, and solve it more directly.

@bhollis
Copy link
Contributor Author

bhollis commented Oct 18, 2023

Besides the behavior issue of focusing inputs, do you have any opinions on the PR?

@bhollis bhollis merged commit 7424c2d into master Oct 18, 2023
5 checks passed
@bhollis bhollis deleted the esc-key branch October 18, 2023 17:20
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