Disable selection checkbox if no bulk actions are eligible #58950
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jorgefilipecosta
added
[Type] Enhancement
|
Size Change: +277 B (0%) Total Size: 1.71 MB
ℹ️ View Unchanged
|
|
Thanks @jorgefilipecosta, the checkbox is hidden 👍 However this only partially solves the issue because it's still possible to select items from the bulk edit menu: 
I noticed we have a similar scenario on the Templates page too – unedited theme-supplied templates can be selected for bulk actions, but no bulk actions exist: 
I wonder if the correct approach here is to check the eligibility of each record and disabled the checkbox accordingly. Is that feasible? I'd appreciate input from @WordPress/gutenberg-design on this one. Specifically; how should we handle bulk selection/actions for records that cannot have any actions applied. On the Patterns page on trunk, note that you can select theme-supplied patterns but not apply any actions to them, which is a strange UX. |
|
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. |
jorgefilipecosta
force-pushed
the
fix/do-not-show-select-checkbox-on-grid-layout-if-no-bulk-actions-are-available-on-an-item
branch
from
d82493f
to
dab0803
Compare
|
Flaky tests detected in ad82f1e93a2c7c5bb5c95a1524ece2a4820f1040. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/8017257049
|
|
Hi @jameskoster, I have applied your feedback. Specifically, bulk editing now only selects selectable items and I have implemented the same logic into the table view, which addresses the issue you saw on templates. Let me know if there is any other enhancement we could do. |
|
Thanks Jorge, I think we're pretty close. A (hopefully) small change request; instead of hiding the checkboxes could we try disabling them? That will avoid the layout shift on grid layout. Apologies, I should have been clearer in my previous comment! |
Hi @jameskoster, In that case, we will see the lock icon, and the disabled checkbox is that ok? |
|
Yes I think so. We can explore moving the lock in a follow up. |
|
Hey @jorgefilipecosta how's this looking? It'd be great to get this included in 6.5 🤞 |
|
@jorgefilipecosta Are you still targeting for 6.5? |
|
This is actually quite important – it's pretty strange that you can select items for which no bulk actions apply. |
getdave
force-pushed
the
fix/do-not-show-select-checkbox-on-grid-layout-if-no-bulk-actions-are-available-on-an-item
branch
from
c62406c
to
e5b3c79
Compare
getdave
added
[Type] Bug
|
I've taken the following actions on this PR:
Please note that Beta 3 is on Tuesday and so this needs to merge on Monday. What's left to do? |
|
@getdave checkboxes for records that have no applicable bulk actions should be |
getdave
force-pushed
the
fix/do-not-show-select-checkbox-on-grid-layout-if-no-bulk-actions-are-available-on-an-item
branch
from
e5b3c79
to
ad82f1e
Compare
getdave
reviewed
Feb 23, 2024
There was a problem hiding this comment.
Screen.Capture.on.2024-02-23.at.10-51-54.mp4
I rebased and tested and I found a possible bug. When there is a editable pattern alongside locked patterns the bulk edit "trigger" buttons shows the wrong number of items because it includes items that are inelligable for the bulk actions.
But...I'm not entirely sure because actually there may be some actinos that are eligible alongside some that are not. So perhaps it's just in this particular case that this feels strange?
It's my understanding that when using assistive tech the See https://www.digitala11y.com/aria-disabled-state/ Also I think the default styling on |
|
Of course, the a11y-friendly approach is the way to go, thanks for pointing that out @getdave.
I don't disagree, but a missing checkbox is more confusing for all users, it seems broken, especially in the templates table layout. We can adjust the styling a bit here if necessary, perhaps reducing the opacity and adding a light grey background? 
Ideally we show the disabled checkbox, with a tooltip explaining why it's disabled if required.
Yup that looks like a bug. |
jorgefilipecosta
force-pushed
the
fix/do-not-show-select-checkbox-on-grid-layout-if-no-bulk-actions-are-available-on-an-item
branch
from
ad82f1e
to
a060ec5
Compare
As long as the state isn't going to change – that is, the checkbox will always (or never) be disabled (at least for the duration of the session) – |
|
The issues identified with the selection logic were addressed. |
…ailable in an item.
jorgefilipecosta
force-pushed
the
fix/do-not-show-select-checkbox-on-grid-layout-if-no-bulk-actions-are-available-on-an-item
branch
from
f4cf225
to
dfb700c
Compare
|
Thanks @jorgefilipecosta, Patterns page looks good. I'm still not seeing disabled checkboxes on the Templates page though. |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
jorgefilipecosta
force-pushed
the
fix/do-not-show-select-checkbox-on-grid-layout-if-no-bulk-actions-are-available-on-an-item
branch
from
dfb700c
to
869f9a7
Compare
I was not aware that we wanted that behavior too on the table view. I will add it in a few minutes. |
|
The same behavior was implemented for the table view too. |
|
@getdave the style updates are in _mixins.scss so you may need to manually rebuild. |
jameskoster
approved these changes
Feb 23, 2024
|
Perfect thank you all for the reviews and insights 👍 |
jorgefilipecosta
deleted the
fix/do-not-show-select-checkbox-on-grid-layout-if-no-bulk-actions-are-available-on-an-item
branch
getdave
added a commit
that referenced
this pull request
Feb 27, 2024
Co-authored-by: jorgefilipecosta <jorgefilipecosta@git.wordpress.org> Co-authored-by: jameskoster <jameskoster@git.wordpress.org> Co-authored-by: getdave <get_dave@git.wordpress.org> Co-authored-by: jeryj <jeryj@git.wordpress.org> Co-authored-by: andrewhayward <andrewhayward@git.wordpress.org> Co-authored-by: annezazu <annezazu@git.wordpress.org> Co-authored-by: liviopv <liviopv@git.wordpress.org>
|
I just cherry-picked this PR to the cherry-pick-wp-6-5-beta-3 branch to get it included in the next release: 56c8292 |
getdave
added
Backported to WP Core
9 tasks
getdave
added a commit
that referenced
this pull request
Feb 27, 2024
Co-authored-by: jorgefilipecosta <jorgefilipecosta@git.wordpress.org> Co-authored-by: jameskoster <jameskoster@git.wordpress.org> Co-authored-by: getdave <get_dave@git.wordpress.org> Co-authored-by: jeryj <jeryj@git.wordpress.org> Co-authored-by: andrewhayward <andrewhayward@git.wordpress.org> Co-authored-by: annezazu <annezazu@git.wordpress.org> Co-authored-by: liviopv <liviopv@git.wordpress.org>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes: #58737
This pull request solves an issue where the selection checkbox was appearing on the grid layout even when there were no bulk actions eligible for the item. This issue was more prominent on patterns, where items without possible bulk actions were displaying a lock icon, causing visual pollution with the checkbox.
cc: @jameskoster
Testing Instructions