-
Notifications
You must be signed in to change notification settings - Fork 4.5k
DataViews: introduce locked filters #71075
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
Conversation
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 If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Size Change: +135 B (+0.01%) Total Size: 1.91 MB
ℹ️ View Unchanged
|
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.
Nice improvement! I tested it and it works as expected.
Just have a couple of questions, which I've left in the comments.
) && | ||
! isShowingFilter | ||
) { | ||
setIsShowingFilter( true ); |
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.
The effect sets isShowingFilter
to true, but it never resets it to false. Could this lead to any potential edge cases? Also, I wonder if it's possible to derive the value directly using useMemo
instead, so we don't need to have useEffect
and useState
?
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.
The state is previous to this PR, and this change fixes a bug: the current code doesn't consider that fields can be updated (e.g., they are empty when dataviews is mounted and updated later). The bug can be reproduced by having a primary filter in this screen (expected: is visible, actual: is not).
I've refactored a bit at b69b582
if ( ! isLocked ) { | ||
onToggle(); | ||
} | ||
} } |
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.
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 updated this at 166ba06 and also prevented locked filters from participating in the tab sequence (makes sense for primary but not for filters you cannot actually interact with).
isLocked: | ||
view.filters?.some( | ||
( f ) => f.field === field.id && !! f.isLocked | ||
) ?? false, | ||
} ); |
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.
Should we consider sorting locked filters prominently like primary filters below for a better user experience?
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.
Great thinking, thanks. I've made locked filters first, then primary, then the rest at 71931f0 I haven't pushed any other UI changes.
7a7c64c
to
b69b582
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.
Thanks @oandregal I've tested it again. Looks good to me. 🙂
Flaky tests detected in b69b582. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/16778274838
|
Related #65295 #65264
What?
Introduces "locked filters" and uses them in the Site Editor's Pages screen.
Screen.Recording.2025-08-05.at.17.30.47.mov
Why?
This replaces some custom code we had in the Pages screen that was sub-optimal (filter value wasn't visible).
Testing Instructions
Visit the Site Editor's Pages screen and navigate through the bundled views (sidebar). Verify "All pages" contains a "status" filter that can be interacted with, and the other pages have a "locked filter" (displays the value but can't be interacted with).