-
Notifications
You must be signed in to change notification settings - Fork 80
Search UI revisited #1108
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
Closed
Closed
Search UI revisited #1108
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| /** | ||
| * Cascading Enter key handler for the search bar. | ||
| * | ||
| * When the Mantine MultiSelect dropdown has visible options, Enter | ||
| * selects the highlighted item (Mantine's default behavior — we do nothing). | ||
| * | ||
| * When there are no visible options (dropdown closed or empty), Enter | ||
| * triggers the search by clicking the search button. | ||
| */ | ||
| document.addEventListener("DOMContentLoaded", function () { | ||
| // Use MutationObserver to attach the handler once the MultiSelect input exists | ||
| var observer = new MutationObserver(function () { | ||
| var input = document.querySelector("#projects input"); | ||
| if (!input) return; | ||
|
|
||
| // Stop observing once we've found the input | ||
| observer.disconnect(); | ||
|
|
||
| input.addEventListener("keydown", function (e) { | ||
| if (e.key !== "Enter") return; | ||
|
|
||
| // Check if the dropdown has visible selectable options. | ||
| // Mantine keeps the dropdown element in the DOM while focused, | ||
| // so we check for actual option elements instead. | ||
| var options = document.querySelectorAll( | ||
| ".mantine-MultiSelect-option" | ||
| ); | ||
| if (options.length > 0) { | ||
| // Dropdown has options — let Mantine handle Enter | ||
| // to select the highlighted item | ||
| return; | ||
| } | ||
|
|
||
| // Dropdown is closed — trigger search | ||
| e.preventDefault(); | ||
| var searchButton = document.getElementById("search-button"); | ||
| if (searchButton) { | ||
| searchButton.click(); | ||
| } | ||
|
Comment on lines
+36
to
+39
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Im not a fan of simulating a click on the button - would be better if we could call the actual search function or whatever the next stage of this process is. |
||
| }); | ||
| }); | ||
|
|
||
| observer.observe(document.body, { childList: true, subtree: true }); | ||
| }); | ||
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we shouldnt be trying to look into the children to decide what to do in a parent event handler - this is backwards from the natural cascading effect of event handlers.
instead we should allow the menu items to handle their own key events (we already do this somewhere i think since pressing enter on one option already works), and simply e.PreventDefault() after doing that so it doesnt bubble up to the search bar. Then, once the menu is closed, the search bar should be in focus and a subsequent enter will hit this event handler, eliminating the need for this logic and simplifying it down to just calling the search function