Improve keybinding lookup and prioritization #16763
Open
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What it does
#16660 revealed a general weakness in the keybinding prioritization system: there was no way for it to prioritize DOM focus in cases where multiple keybindings were active according to globally set context keys.
#16668 provided an ad hoc fix for the quick input system. This PR provides a more general fix for the keybinding system by allowing the keybinding to prioritize keybindings that use context keys defined in contexts applied to the focused DOM element or its parents. It then applies such a local keybinding context to the QuickInput system to address #16660.
It also addresses a long-standing FIXME to improve keybinding lookup times by building a TernarySearchTree for lookup rather than iterating over all defined keybindings.
How to test
Esc, the quick input close command should be triggered rather than the notification hide command.Follow-ups
Breaking changes
Attribution
Review checklist
nlsservice (for details, please see the Internationalization/Localization section in the Coding Guidelines)Reminder for reviewers