-
Notifications
You must be signed in to change notification settings - Fork 1k
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
[24.0] Fix history filters taking up space in GridList
#17652
[24.0] Fix history filters taking up space in GridList
#17652
Conversation
It was impossible to tab over to the popover once it is shown, so instead I used the |
Add a popover version of the `FilterMenu` component. This can be enabled via the `isPopover` prop. Fixes galaxyproject#17618
f07f9f9
to
7996b03
Compare
What do we think about this: compact_filter_menu.mp4A slightly more compact version of the |
Looks great! |
@mvdbeek thanks! Note that i updated the screencast above Unfortunately, there is one functionality that kind of breaks, i.e.: When you have the advanced filters opened, and you try to change the deletes_filters_unexpectedly.mp4However, one must note that users will typically not edit the text in the main search field when they have filters applied and they would probably edit those individual fields. |
To counter this, the main search field can be disabled in case the menu is expanded: compact_filter_menu_field_disabled.mp4 |
I did not want to bring it up, since this came a long way already, but if it is giving you any problems consider dropping the "name" search field all together. I have trouble imagining a use case where "name+tag" search field wouldn't find something that "name" search field does. |
By default, the search query (without filters) looks at name and tag (and annotation etc?) fields in the backend; and an explicit name filter obviously only looks at the name field. That's why we have separate name filters for all grids and workflow list For some classes like
and if there is no filter in the query, it automatically assumes there is a name filter because that's how the backend So, I don't know if this addresses your concern but this is the way the no-filter query works for the grids. I reckon it should be intuitive for users since a non filtered query searches "everything" and a specific filter looks at specific fields. |
the improvements in this PR are good, I am not a fan of disabling the main search field but other than that I am ready to merge this. Thank you or working on it. My other feedback is more conceptual and not 24.0-bound. I believe in grids there should be one search bar for a given set of objects. It should be powerful, have shortcuts for filters, have advanced syntax for pro users, but in the end there should be only one place where to type and the majority of real estate should be taken by results. |
Not disabling it anymore, and fixing the selenium fails... |
It groups `is` type boolean filters on one line, and in checkboxes. Adding filters applies them immediately, and there is no search button.
108a57c
to
008b652
Compare
9176b9a
to
20e4422
Compare
Thanks @ahmedhamidawan @martenson! |
Enhancement Added here to the Grids and Workflow List:
Fixes #17618
compact_filter_menu.mp4
Additional feature also added (but not enabled anywhere):
Add a popover version of the
FilterMenu
component. This can be enabled via theisPopover
prop.prepopover_filter_menu.mp4
withpopover_filter_menu.mp4
How to test the changes?
(Select all options that apply)
License