-
Notifications
You must be signed in to change notification settings - Fork 42
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
👻 Convert single/multi filter toolbar controls to use pf5 select #1669
Conversation
3d68556
to
f345edb
Compare
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.
Some problems I found playing with the new filter toolbars:
-
Deselecting an item in a filter list doesn't work / each time you click the same entry, another chip is added
- application inventory page, with multiple applications available
- open the name filter dropdown with or without filter text
- click to select a name
- name cannot be unselected here, only by clicking the x in the name chips list
-
The filter type drop down does not auto-close
-
If the multiselect filter select drop down opens from the twistee, keyboard focus is left on the checkbox. But on selecting an item (space bar on the checkbox), focus goes back to the filter input box and any further attempt to select items with the spacebar actually types in the filter and changes the list. Only hitting enter will allow selecting a second item, but this also closes the dropdown.
For example:
screencast-localhost_9000-2024.02.13-13_24_54.mp4
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 review is just on the single select control SelectFilterControl
. The control looks and works ok.
I have some suggestions to simplify the code in SelectFilterControl
.
client/src/app/components/FilterToolbar/SelectFilterControl.tsx
Outdated
Show resolved
Hide resolved
client/src/app/components/FilterToolbar/SelectFilterControl.tsx
Outdated
Show resolved
Hide resolved
Cleaned things up a bit to more closely match this example https://www.patternfly.org/components/menus/select#multiple-typeahead-with-checkboxes & fixed most of the bugs you mentioned. |
Signed-off-by: ibolton336 <ibolton@redhat.com>
284e089
to
6fd778a
Compare
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.
I've only looked at SelectFilterControl
so far in this version of changes. I spent a bunch of time checking out the filter category selectOptions
data against the PF5 SelectOption
data. Looks like a small change could close the data gaps...
I'll have a look at the Multiselect behavior then code next.
client/src/app/components/FilterToolbar/SelectFilterControl.tsx
Outdated
Show resolved
Hide resolved
d7f1815
to
6dead72
Compare
- Cleanup the single select - Disable if no selectable option - Cleanup multiselect filter control Signed-off-by: Ian Bolton <ibolton@redhat.com>
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.
ack
- Resolves: - https://issues.redhat.com/browse/MTA-1872 - https://issues.redhat.com/browse/MTA-467 - Upgrade our usage of the PF select component to the pf 5 supported version. --------- Signed-off-by: ibolton336 <ibolton@redhat.com> Signed-off-by: Ian Bolton <ibolton@redhat.com> Signed-off-by: Cherry Picker <noreply@github.com>
…) (#1694) - Resolves: - https://issues.redhat.com/browse/MTA-1872 - https://issues.redhat.com/browse/MTA-467 - Upgrade our usage of the PF select component to the pf 5 supported version. --------- Signed-off-by: ibolton336 <ibolton@redhat.com> Signed-off-by: Ian Bolton <ibolton@redhat.com> Signed-off-by: Cherry Picker <noreply@github.com> Signed-off-by: ibolton336 <ibolton@redhat.com> Signed-off-by: Ian Bolton <ibolton@redhat.com> Signed-off-by: Cherry Picker <noreply@github.com> Co-authored-by: Ian Bolton <ibolton@redhat.com>
Resolves:
Upgrade our usage of the PF select component to the pf 5 supported version.