Skip to content
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

Submit search on enter #805

Merged
merged 55 commits into from
Oct 18, 2024
Merged

Submit search on enter #805

merged 55 commits into from
Oct 18, 2024

Conversation

joshsadam
Copy link
Contributor

@joshsadam joshsadam commented Oct 3, 2024

What does this PR do and why?

Describe in detail what your merge request does and why.

  1. Standardized Search Component
  2. Search now works when the enter key is pressed, not debounced

Screenshots or screen recordings

Screenshots are required for UI changes, and strongly recommended for all other pull requests.

image

How to set up and validate locally

Numbered steps to set up and validate the change are strongly suggested.

Ensure that search still works across the project (tested through our integration tests as well)

PR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

@joshsadam joshsadam force-pushed the submit-search-on-enter branch from 3d43087 to 9a7db86 Compare October 3, 2024 20:08
@joshsadam joshsadam self-assigned this Oct 4, 2024
@joshsadam joshsadam force-pushed the submit-search-on-enter branch 3 times, most recently from b0f3b93 to aa5dfa1 Compare October 7, 2024 18:00
@joshsadam joshsadam added the enhancement New feature or request label Oct 8, 2024
@joshsadam joshsadam force-pushed the submit-search-on-enter branch from b655a04 to 49cdc95 Compare October 16, 2024 14:42

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

@joshsadam joshsadam marked this pull request as ready for review October 17, 2024 13:57
Copy link
Collaborator

@ChrisHuynh333 ChrisHuynh333 left a comment

Choose a reason for hiding this comment

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

Just through usage on the website, I noticed a few inconsistencies. This is assuming we want the search field to still be populated with the search after hitting enter, and to have focus on the search bar. As well as after leaving the page, the filter would be removed

  • Projects dashboard - Search field is emptied and not focused after search
  • Groups dashboard - If I leave the groups dashboard page and come back, the search bar is still populated with the search but the groups are not filtered
  • Data Exports - Search field is emptied and not focused after search
  • Samples - Search and filter are still enabled after leaving and coming back to the page
  • Project Files - Search field is emptied and not focused after search

I also think if the user clicks on the "X" at the end of the search bar, that should return an unfiltered request, thoughts?

@ericenns
Copy link
Member

Just through usage on the website, I noticed a few inconsistencies. This is assuming we want the search field to still be populated with the search after hitting enter, and to have focus on the search bar. As well as after leaving the page, the filter would be removed

  • Projects dashboard - Search field is emptied and not focused after search
  • Groups dashboard - If I leave the groups dashboard page and come back, the search bar is still populated with the search but the groups are not filtered
  • Data Exports - Search field is emptied and not focused after search
  • Samples - Search and filter are still enabled after leaving and coming back to the page
  • Project Files - Search field is emptied and not focused after search

I also think if the user clicks on the "X" at the end of the search bar, that should return an unfiltered request, thoughts?

Samples table search is expected to be different as we store that server side, but I agree with your remaining comments.

@joshsadam joshsadam force-pushed the submit-search-on-enter branch from 512a90b to 5152882 Compare October 17, 2024 19:01

This comment has been minimized.

@joshsadam
Copy link
Contributor Author

@ChrisHuynh333 & @ericenns Those pages are updated along with tests

This comment has been minimized.

This comment has been minimized.

Copy link
Member

@ericenns ericenns left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@ericenns ericenns dismissed ChrisHuynh333’s stale review October 18, 2024 16:07

Comments have been addressed, and I verified the changes

@ericenns ericenns merged commit 43c94ec into main Oct 18, 2024
3 checks passed
@ericenns ericenns deleted the submit-search-on-enter branch October 18, 2024 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants