Search UI - Enter key handler #1114
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| /** | ||
| * Cascading Enter key handler for the search bar. | ||
| * | ||
| * When the Mantine MultiSelect dropdown is open (aria-expanded="true"), | ||
| * Enter selects the highlighted item (Mantine's default — we do nothing). | ||
| * | ||
| * When the dropdown is closed, Enter triggers the search by incrementing | ||
| * the search-trigger dcc.Store via dash_clientside.set_props. | ||
| */ | ||
| document.addEventListener("DOMContentLoaded", function () { | ||
| var observer = new MutationObserver(function () { | ||
| var input = document.querySelector("#projects input"); | ||
| if (!input) return; | ||
|
|
||
| observer.disconnect(); | ||
|
|
||
| input.addEventListener("keydown", function (e) { | ||
| if (e.key !== "Enter") return; | ||
|
|
||
| // If dropdown is open, let Mantine handle Enter to select the highlighted item | ||
| if (input.getAttribute("aria-expanded") === "true") return; | ||
|
|
||
| // Dropdown is closed — trigger search via the dcc.Store | ||
| e.preventDefault(); | ||
|
Comment on lines
+20
to
+24
Contributor
There was a problem hiding this comment. This is still fundamentally the same logic that I reviewed in the prior version. Ideally we shouldnt need to call preventDefault anywhere (with the possible exception of the event handler for the individual dropdown items)
Contributor
Author
There was a problem hiding this comment. well some of the things we've discussed it is not here anymore. It is not going in the opposite direction of the cascade of events anymore. It is a simpler logic as well, two situations, dropdown open, goes through normal events, dropdown closed, trigger the search. However, as explained on the slack channel, I think we've achieved the objective, which was to allow for navigation and trigger the search all through the keyboard, and this is done with #1113 |
||
| var store = document.querySelector("#search-trigger"); | ||
| var current = store ? JSON.parse(store.textContent || "0") : 0; | ||
| dash_clientside.set_props("search-trigger", { data: current + 1 }); | ||
|
Comment on lines
+25
to
+27
Contributor
There was a problem hiding this comment. Can you explain how you ended up on this solution for triggering the search?
Contributor
Author
There was a problem hiding this comment. this is to replace that simulation of the click with .click, so instead we store the info through a dcc.store and use it to trigger the search as a function of the dropdown(open, close). Idea is: The logic is simple as explained in the previous discussion: mantine's dropdown is open, do nothing, dropdown closed, trigger it.
Contributor
There was a problem hiding this comment. Im really skeptical of introducing new dash callback inputs just to give us an alternative path to trigger a search. is there a In other words: in the chain of all things that happen for a search (i.e. pressing the search button calling a function, a function calls some other function, that eventually becomes database calls to fetch data, that eventually updates the UI) can we call the same function that a button press would call, rather than simulating a click or introducing a new parallel path?
Collaborator
There was a problem hiding this comment. @MoralCode FYI (quoting @EngCaioFonseca in another thread): "I don't think there's a need for the PR3 (enter key handler) at this point, with the modifications made (#1113), and with this search button, it is possible to navigate the dropdown, then exit and trigger the search using the button, all with the keyboard." |
||
| }); | ||
| }); | ||
|
|
||
| observer.observe(document.body, { childList: true, subtree: true }); | ||
| }); | ||
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.
This PR appears to contain styling changes
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.
this is from previous PRs/commits. Once it is rebased, these will be resolved.