js: Add and utilize function isSpecialKeypress#377
Open
nilmerg wants to merge 1 commit into
Open
Conversation
This is a way better implementation instead of just checking whether the pressed key's label starts with an uppercase char. (e.g. `Escape`) Problem is, uppercase single chars also match… This could previously be observed when trying to type a value inside an operator input in the search bar. Normally this switches to the value input automatically, but an uppercase char was tried to identify as operator. In the `Completer` implementation this uncovered dead code on the other hand. `onKeyPress` seems to have had the job to hide the suggestions in case the user typed one manually. Upon testing this now with an corrected check (`event.key` alone does not match a suggestion as it's only single char) this worked but the suggestions appeared immediately again. I've removed it now as it was broken prior and doesn't make any sense either nowadays, suggestions need to be always visible in order to confirm the validity of the input.
There was a problem hiding this comment.
Pull request overview
This PR improves keyboard handling in the search/filter inputs by replacing a brittle “uppercase key label” heuristic with a dedicated helper that detects non-character (special/control) key presses. It also removes dead/broken suggestion-hiding logic in the completer.
Changes:
- Added a shared JS helper
isSpecialKeyPress(event)and started using it in filter/input key handling. - Updated
FilterInputandBaseInputto use the shared helper instead of an uppercase-label regex. - Removed the unused/broken default-case logic from
Completer.onKeyDown()that attempted to hide suggestions.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| asset/js/widget/FilterInput.js | Imports and uses the new helper to ignore special/control key presses during context switching logic. |
| asset/js/widget/BaseInput.js | Imports and uses the new helper to avoid clearing syntax errors on control/special key presses. |
| asset/js/widget/Completer.js | Removes dead code in keydown handling that attempted to hide suggestions based on typed keys. |
| asset/js/functions.js | Introduces isSpecialKeyPress(event) as a reusable helper module. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * @returns {boolean} True if the event represents a special key press, false otherwise. | ||
| */ | ||
| function isSpecialKeyPress(event) { | ||
| return event.key.length > 1 || event.ctrlKey || event.metaKey; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This is a way better implementation instead of just checking whether the pressed key's label starts with an uppercase char. (e.g.
Escape) Problem is, uppercase single chars also match…This could previously be observed when trying to type a value inside an operator input in the search bar. Normally this switches to the value input automatically, but an uppercase char was tried to identify as operator.
In the
Completerimplementation this uncovered dead code on the other hand.onKeyPressseems to have had the job to hide the suggestions in case the user typed one manually. Upon testing this now with an corrected check (event.keyalone does not match a suggestion as it's only single char) this worked but the suggestions appeared immediately again. I've removed it now as it was broken prior and doesn't make any sense either nowadays, suggestions need to be always visible in order to confirm the validity of the input.