Make WordPress Core

Opened 16 months ago

Closed 8 days ago

Last modified 8 days ago

#58479 closed enhancement (fixed)

Add notice if no posts are selected in Bulk Edit

Reported by: sumitsingh's profile sumitsingh Owned by: joedolson's profile joedolson
Milestone: 6.7 Priority: normal
Severity: normal Version:
Component: Quick/Bulk Edit Keywords: has-patch commit
Focuses: accessibility, javascript, administration Cc:

Description (last modified by sabernhardt)

On the Posts screen(s), clicking the bulk actions Apply button without selecting any posts does not show an error message.

  1. Visit the Posts screen.
  2. Choose "Edit" in the Bulk actions dropdown.
  3. Without checking any checkboxes, click the Apply button. Nothing seems to happen.
  4. Choose "Move to Trash" in the Bulk actions dropdown.
  5. Click the Apply button. The page reloads but cannot move anything to Trash.
  6. Choose "Bulk actions" at the top of the Bulk actions dropdown.
  7. Click the Apply button. The page reloads with a query string in the URL.

I have checked and it should be a message on the page/post list as per added notice message on the plugin page.

For more information, you can see mentioned quick video.

Thank you

Attachments (6)

Posts ‹ wptest — WordPress.mp4 (941.1 KB) - added by sumitsingh 16 months ago.
here quick video for more clear about suggetion
Posts-‹-wptest-—-WordPress.png (57.2 KB) - added by sumitsingh 16 months ago.
My suggetion
Plugins-‹-wptest-—-WordPress.png (237.6 KB) - added by sumitsingh 16 months ago.
existing example
58479-1.diff (1.9 KB) - added by nihar007 16 months ago.
I added only "no post selected" alert
58479-2.diff (4.5 KB) - added by royho 3 weeks ago.
58479.png (22.0 KB) - added by joedolson 8 days ago.
current screenshot

Download all attachments as: .zip

Change History (54)

@sumitsingh
16 months ago

here quick video for more clear about suggetion

@sumitsingh
16 months ago

existing example

@nihar007
16 months ago

I added only "no post selected" alert

#1 @sabernhardt
16 months ago

  • Component changed from Posts, Post Types to Quick/Bulk Edit
  • Description modified (diff)
  • Focuses javascript added; coding-standards removed
  • Keywords has-patch added
  • Summary changed from We need to add notice on page/post similar to plugin list page to Add notice if no posts are selected in Bulk Edit

#2 @sabernhardt
16 months ago

Categories and Tags pages may need something similar, too (separate ticket?). Those pages reload without an error/notice if no items are selected.

#3 @sumitsingh
16 months ago

I think we can manage in this ticket @sabernhardt. Let me know if I need to create separate for other screen like this validation overall backed side.

This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.


15 months ago

#5 @joedolson
15 months ago

  • Milestone changed from Awaiting Review to 6.4
  • Owner set to joedolson
  • Status changed from new to accepted

#6 @oglekler
14 months ago

@sumitsingh and @joedolson, I wonder if the better approach is to disable button until something will be selected, otherwise page reload is pointless. And message needs to be appear close to button if person is trying to click it that nothing will be changed.

#7 follow-up: @joedolson
14 months ago

See also #45006 and #31634, as well. Disabling the button was tried for 31634, but was not as great a solution as it seemed. There were a lot of accessibility problems.

This patch looks like it could use some refinement, as well.

1) This should probably be an inline admin notice. Then we don't need to disruptively change the viewport position for the user.
2) It should also use wp.a11y.speak so that there's a verbal announcements.

#8 @joedolson
13 months ago

  • Keywords changes-requested added

This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.


13 months ago

This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.


13 months ago

#11 @joedolson
13 months ago

  • Owner changed from joedolson to quadthemes
  • Status changed from accepted to assigned

#12 in reply to: ↑ 7 @quadthemes
13 months ago

Adding inline admin notice to bulk action section would result in error being shown at both places(before and after the list). Though it's much better than scroll to top that is currently happening on plugins page. Any opinions? @joedolson

Replying to joedolson:

See also #45006 and #31634, as well. Disabling the button was tried for 31634, but was not as great a solution as it seemed. There were a lot of accessibility problems.

This patch looks like it could use some refinement, as well.

1) This should probably be an inline admin notice. Then we don't need to disruptively change the viewport position for the user.
2) It should also use wp.a11y.speak so that there's a verbal announcements.

Last edited 13 months ago by quadthemes (previous) (diff)

This ticket was mentioned in Slack in #core by marybaum. View the logs.


12 months ago

#14 @marybaum
12 months ago

Can yall get this patch refined in a week? We will look at this again next Tuesday.

#16 @oglekler
12 months ago

  • Keywords needs-testing added; changes-requested removed

Thank you, @quadthemes for the patch. Let's test it.

This ticket was mentioned in Slack in #core-test by oglekler. View the logs.


12 months ago

#18 @huzaifaalmesbah
12 months ago

I've been trying to test and haven't noticed any changes.

#19 @Ankit K Gupta
12 months ago

  • Keywords 2nd-opinion added

Tested the patch https://github.com/WordPress/wordpress-develop/pull/5423 and it works as explained here:

Adding inline admin notice to bulk action section would result in error being shown at both places(before and after the list). Though it's much better than scroll to top that is currently happening on the plugins page.

Plugins Page After the patch:

https://i.imgur.com/blPyiSV.jpg

Posts Page After the Patch:

https://i.imgur.com/WD8QCqJ.jpg

IMO, just showing the message at the top is fine instead of showing the same message in two places on same screen.

Let's wait for another opinion.

#20 @fnpen
12 months ago

@ankit-k-gupta, I also see it only on the top of the page (Plugins), and does not work on posts page.

Last edited 12 months ago by fnpen (previous) (diff)

This ticket was mentioned in PR #5433 on WordPress/wordpress-develop by @fnpen.


12 months ago
#21

#22 @fnpen
12 months ago

This part of the features has a partial implementation for Plugins, but it does not show when action is not selected.

Also, we can select actions for Users and Change Roles where it is a similar requirement to pick rows. The plugins page shows only one notification on top and scrolls the page up.

Implementation:

  • Covers cases of any list table with role selector and plugins page.
  • Shows one notification when no action or item is selected

'addAdminNotice' will be a good idea to make it global and universal.

This ticket was mentioned in Slack in #core by oglekler. View the logs.


12 months ago

#24 @oglekler
12 months ago

  • Keywords 2nd-opinion removed
  • Milestone changed from 6.4 to 6.5

We have RC1 tomorrow, so, I am moving this to the next milestone. Right now we can work further on the ticket, and it can land into the 6.5 as soon as it will be ready.

This ticket was mentioned in Slack in #core by abhanonstopnews. View the logs.


10 months ago

#26 @webcommsat
10 months ago

Discussed amongst maintainers ahead of today's bug scrub. @quadthemes (sunnyssr on Slack) asked if any additional help needed or if there was an update to move this ticket forward. There are some comments above from testing and a potential implementation suggested by @fnpen .
Thanks everyone.

This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.


8 months ago

#28 @ukdrahul
8 months ago

Test Report

Environment

  • WordPress: 6.5-beta1
  • PHP: 8.1.23
  • Server: Apache v2.4.58
  • Database: MySQL 8.0.16
  • Browser: Chrome v121.0.6167.160 (Official Build) (arm64)
  • OS: macOS Sonoma v14.2.1 (23C71)
  • Theme: Twenty Twenty-Four v1.0

Verdict

The issue reported in this ticket still persists in the tested environment.

Last edited 8 months ago by ukdrahul (previous) (diff)

#29 @swissspidy
8 months ago

  • Milestone changed from 6.5 to 6.6
  • Type changed from defect (bug) to enhancement

Moving to 6.6 as this is an enhancement and we're already in 6.5 beta

This ticket was mentioned in Slack in #core by oglekler. View the logs.


5 months ago

This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.


5 months ago

#33 @audrasjb
4 months ago

  • Milestone changed from 6.6 to Future Release

Moving this ticket to Future Release pending a new patch as we're one week before 6.6 beta 1.

This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.


4 months ago

#35 @joedolson
4 months ago

  • Keywords needs-patch added; has-patch needs-testing removed
  • Milestone changed from Future Release to 6.7
  • Owner changed from quadthemes to joedolson
  • Status changed from assigned to accepted
  • Version 6.2.2 deleted

Let's finish this up in 6.7. Since the existing patch is incomplete, I'm setting this back to needs patch.

This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.


8 weeks ago

#37 @royho
3 weeks ago

I've tested https://github.com/WordPress/wordpress-develop/pull/5433 and it works for the most part.

Since the admin notice is shown async (without page reload), scrolling to the top is expected. Also this PR(5433) adds this same behavior across pages, users, and comments which is nice. In addition, notice is read out when notice is displayed on screen readers.

One problem is on the Users page, wp.a11y module doesn't exist. So Speak would not work there. I believe once that can be addressed, it would be good to go.

@royho
3 weeks ago

#38 @royho
3 weeks ago

I've attached a patch (58479-2.diff) fixing the issue from my previous comment: https://core.trac.wordpress.org/ticket/58479#comment:37

This patch is based off of @fnpen's work: https://github.com/WordPress/wordpress-develop/pull/5433

I just fixed the error present on the Users page because a11y wasn't available on that page.

This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.


2 weeks ago

This ticket was mentioned in PR #7453 on WordPress/wordpress-develop by @joedolson.


11 days ago
#40

  • Keywords has-patch added; needs-patch removed

#41 @joedolson
11 days ago

  • Keywords needs-screenshots added; has-screenshots removed

PR makes a few changes to this.

1) Add wp-a11y as a dependency to common.js rather than loading it in admin.php
2) Abstracts the arguments a bit in addAdminNotice to make it more flexible.
3) Unsets the value of bulkAction in updates.js so that the 'no items selected' message will fire instead of the delete or filesystem dialogs.

Needs updated screenshots to reflect that there's only one notice now & it's dismissible.

#42 @joedolson
11 days ago

Possible changes: updates.js has common.js as a dependency. That means we could remove wp.updates.addAdminNotice and make the version of that in common a globally available function. This would open up a path to standardizing that function similarly to the PHP equivalent.

@costdev Thoughts?

#43 @costdev
11 days ago

@joedolson As long as we factor in any differences in how wp.updates.addAdminNotice() is called and how a standardized addAdminNotice() is called, that should be fine and it would be a great addition.

That may mean that we end up keeping wp.updates.addAdminNotice() in place, make the necessary adjustments, and internally call the general addAdminNotice() so that extender usage of wp.updates.addAdminNotice() isn't affected and BC is protected.

#44 @joedolson
10 days ago

So, I was looking into the potential changes for extending addAdminNotice() to be globally usable pass wp.updates.addAdminNotice() into it, but that's really a much more complex change than feels appropriate for this ticket.

I'm going to open a new ticket to do that. I think it's worthwhile, but I don't think there's anything about this current patch that would create a future blocker.

I noticed that this patch is mostly OK, but doesn't fire on the 'Edit' bulk action in posts lists. Looks like it's overridden by the onclick event in inline-edit-post.js that's fired on #doaction, so that'll need to be exited if there are no selections.

This ticket was mentioned in Slack in #core by stoyangeorgiev. View the logs.


8 days ago

@joedolson
8 days ago

current screenshot

#46 @joedolson
8 days ago

  • Keywords commit added; needs-screenshots removed

With this issue resolved, marking for commit.

#47 @joedolson
8 days ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 59134:

Quick/Bulk Edit: Add notice if no items selected.

Add an error notice if a user attempts to apply bulk edits with no items selected. Applies to post lists, comments, taxonomies, and plugins screens.

Props garrett-eclipse, nrqsnchz, sumitsingh, nihar007, royho, sabernhardt, oglekler, quadthemes, ankit-k-gupta, fnpen, ukdrahul, joedolson.
Fixes #45006, #58479.

@joedolson commented on PR #7453:


8 days ago
#48

in r59134

Note: See TracTickets for help on using tickets.