Conversation
108efcd to
f8197fe
Compare
f8197fe to
7da42a4
Compare
|
The idea on the JS event handler on this one, is due to limitations on the DMC multiselect. Instead of inspecting the children, like on the main PR (before splitting it), we listen for the 'keydown' on the search bar input, and on enter we check 'aria-expanded' on the input, which is set to 'true' when dropdown is open. |
| /* MultiSelect pill colors */ | ||
| --pill-default-bg: #555555; | ||
| --pill-default-color: #ffffff; | ||
| --pill-pending-bg: #B03A2E; | ||
| --pill-text-color: #ffffff; | ||
|
|
There was a problem hiding this comment.
This PR appears to contain styling changes
There was a problem hiding this comment.
this is from previous PRs/commits. Once it is rebased, these will be resolved.
| // 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(); |
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)
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 }); |
There was a problem hiding this comment.
Can you explain how you ended up on this solution for triggering the search?
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).
This is still allowing dash normal callbacks flow.
Idea is:
dcc.store("search-trigger") as an input to the callbacks that takes search-button.n_clicks. So the button and the store are equivalent entry points into the logic.
when dropdown is closed (on enter), a new value is stores using dash_clientside.set_props which triggers the whole flow: input -> callbacks -> repo query dispatch.
The counter adds 1, because of the dash callbacks, which only fire if there's changes.
The logic is simple as explained in the previous discussion: mantine's dropdown is open, do nothing, dropdown closed, trigger it.
The desired output in the end is the enter key and the search button goes through the same callback path, and this JS script pretty much just works by translating the keyboard event into storing info/variable.
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 <form> that (i assume) wraps the search box? if so, can we call .submit() or the equivalent on that form to use the same codepath as a normal user would encounter without needing to simulate a click?
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?
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."
This PR adds a few features to improve the Search bar UI/UX based on Issue #1097
Generative AI disclosure
Please select one option:
If AI tools were used, please provide details below:
- What tools were used? Claude - Opus
- How were these tools used? Used in the Previous main PR. Initial Draft/Prototype. This is part 3 of that PR #1108.
- Did you review these outputs before submitting this PR? Yes.