Skip to content

Search UI revisited#1108

Closed
EngCaioFonseca wants to merge 2 commits intodevfrom
search_ui_revisited
Closed

Search UI revisited#1108
EngCaioFonseca wants to merge 2 commits intodevfrom
search_ui_revisited

Conversation

@EngCaioFonseca
Copy link
Copy Markdown
Contributor

@EngCaioFonseca EngCaioFonseca commented Mar 31, 2026

This PR adds a few features to improve the Search bar UI/UX based on Issue #1097

Generative AI disclosure

Please select one option:

  • This contribution was NOT assisted or created by Generative AI tools.
  • This contribution was assisted or created by Generative AI tools.

If AI tools were used, please provide details below:
- What tools were used? Claude Code, Coderabbit
- How were these tools used? Initial draft and Prototype
- Did you review these outputs before submitting this PR? Yes

Copy link
Copy Markdown
Collaborator

@cdolfi cdolfi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reading through this seems like it should be split into atleast 2 PR: One for color styling changes, one for button change, and one for the Cascading Enter key handler for the search bar.

@EngCaioFonseca EngCaioFonseca marked this pull request as ready for review April 2, 2026 13:24
@EngCaioFonseca EngCaioFonseca moved this from In Progress to Testing | Waiting on PR response | Blocked in Aspen Project Board Apr 2, 2026
Comment on lines +25 to +35
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();
Copy link
Copy Markdown
Contributor

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

Comment on lines +36 to +39
var searchButton = document.getElementById("search-button");
if (searchButton) {
searchButton.click();
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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.

@EngCaioFonseca
Copy link
Copy Markdown
Contributor Author

Closed due to #1112 #1113 and #1114

@github-project-automation github-project-automation bot moved this from Testing | Waiting on PR response | Blocked to Done in Aspen Project Board Apr 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

Enter/Return key doesn't work to complete search, unintuitive placement of icon to click

3 participants