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

Add icons to FilterButton dropdown #10186

Merged
merged 16 commits into from
Sep 10, 2024
Merged

Add icons to FilterButton dropdown #10186

merged 16 commits into from
Sep 10, 2024

Conversation

erwanMarmelab
Copy link
Contributor

Problem

There is no way to differentiate between filter choices and filter actions (such as save current query) in the FilterButton

when you select a filter, it disappears from the list, which changes the place of each choice item and makes it hard to remember where is the filter you want to enable ; same for the actions (Remove all filters)

To Do

  • Add a divider to separate between the filter choices and filter actions
  • Keep all choices in the list at all times, and simply gray out the items when they are not available
  • Test icons in the Filter Button Menu items
  • Add checkbox before filter names

Additional Checks

  • The PR targets master for a bugfix, or next for a feature
  • The PR includes unit tests (if not possible, describe why)
  • The PR includes one or several stories (if not possible, describe why)
    - [ ] The documentation is up to date

image

image

image

@@ -74,13 +80,18 @@ export const FilterButton = (props: FilterButtonProps) => {
}

const hiddenFilters = filters.filter(
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be renamed. allTogglableFilters?

Comment on lines 132 to 139
setTimeout(() => {
const inputElement = document.querySelector(
`input[name='${source}']`
) as HTMLInputElement;
if (inputElement) {
inputElement.focus();
}
}, 50);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we have to focus the filter input that we just removed?

@djhi
Copy link
Contributor

djhi commented Sep 6, 2024

Besides, the tests are failing

Copy link
Contributor

@djhi djhi left a comment

Choose a reason for hiding this comment

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

We're missing tests about removing filters:

  • Removing from the filter button we already had should uncheck its matching item in the menu
  • Unchecking an applied filter from the menu should remove it (both the input and the filter value)

Copy link
Contributor

@djhi djhi left a comment

Choose a reason for hiding this comment

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

Almost there!

Co-authored-by: Gildas Garcia <1122076+djhi@users.noreply.github.com>
@djhi djhi merged commit 7b5a8d5 into next Sep 10, 2024
14 checks passed
@djhi djhi deleted the feat/FilterButton/Improve-UX branch September 10, 2024 07:19
@fzaninotto fzaninotto added this to the 5.2.0 milestone Sep 13, 2024
@fzaninotto fzaninotto changed the title feat(ra-ui-materialui): Improve FilterButton UX Add icons in FilterButton dropdown Sep 13, 2024
@fzaninotto fzaninotto changed the title Add icons in FilterButton dropdown Add icons to FilterButton dropdown Sep 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFR Ready For Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants