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

DataViews: Remove add filter button if no available filters to add #64695

Conversation

ntsekouras
Copy link
Contributor

What?

Fixes: #64689

This PR removes add filter button if no available filters to add. @jameskoster do you know if there is a good reason to keep it disabled as is right now?

Testing Instructions

  1. In site editor go to list that uses DataViews, like the pages
  2. Add all the available filters
  3. Observe that the add filter button is not shown

@ntsekouras ntsekouras added [Type] Enhancement A suggestion for improvement. Needs Design Feedback Needs general design feedback. [Package] DataViews /packages/dataviews labels Aug 21, 2024
@ntsekouras ntsekouras self-assigned this Aug 21, 2024
Copy link

github-actions bot commented Aug 21, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: ntsekouras <ntsekouras@git.wordpress.org>
Co-authored-by: oandregal <oandregal@git.wordpress.org>
Co-authored-by: allilevine <firewatch@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@oandregal
Copy link
Member

I've commented in the original issue because I didn't realize there was a PR for it. Copy/Pasting my comment there:

The Add filter link should only show when there is another filter to add.

I agree with this. If there's one filter, there's never an opportunity for the "add filter" to become enabled.

When there are multiple filters and they've all been added, the Add filter link still shows

This case is different, though.

You cannot add more filters when you've added all of them, but the button becomes active as soon as you remove one. In this scenario the disabled/enabled states are more representative, IMO. It'll be a confusing experience to having buttons coming in/out on the screen.

Additionally, we need to pay attention to the keyboard affordances: notice how the "add filter" button gains focus when the last filter is removed:

Gravacao.do.ecra.2024-08-21.as.22.01.18.mov

@ntsekouras
Copy link
Contributor Author

Additionally, we need to pay attention to the keyboard affordances: notice how the "add filter" button gains focus when the last filter is removed:

You right @oandregal, I had added that a while ago and forgot about it. Sorry..

I'll close this PR for now in order to have the discussion at the main issue first.

@ntsekouras ntsekouras closed this Aug 22, 2024
@oandregal oandregal deleted the fix/dataviews-hide-add-filter-if-no-available-filters branch August 22, 2024 07:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Design Feedback Needs general design feedback. [Package] DataViews /packages/dataviews [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DataViews: Hide "Add filter" link if there are no filters to add
2 participants