Make WordPress Core

Opened 3 years ago

Closed 3 years ago

#54181 closed defect (bug) (fixed)

Terms management screen shows bulk actions dropdown even if there are no terms

Reported by: swissspidy's profile swissspidy Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 5.9 Priority: normal
Severity: normal Version:
Component: Taxonomy Keywords: good-first-bug has-patch commit has-screenshots
Focuses: administration Cc:

Description

On the posts list table, when there are no post, the bulk actions dropdown is not displayed:

https://i.imgur.com/VH6qKg6.png

However, on the terms list table the dropdown is still displayed even if there are no terms:

https://i.imgur.com/8MgVoct.png

For consistency, it would make sense to remove the bulk options there as well if there are no terms at all.

Attachments (5)

54181.patch (1.1 KB) - added by mattoakley 3 years ago.
Patch hides bulk edit options in terms list if no terms are found.
Capture d’écran 2021-10-06 à 23.32.37.png (205.0 KB) - added by audrasjb 3 years ago.
After patch: with one or more items, the bulk actions and searchbar display correctly
Capture d’écran 2021-10-06 à 23.32.13.png (174.9 KB) - added by audrasjb 3 years ago.
After patch : when there is no item, the searchbar and the bulk actions are removed
54181.2.patch (1.2 KB) - added by mattoakley 3 years ago.
Patch hides bulk edit options in terms list if no terms are found, patch includes more accurate doc blocks
54181.3.diff (4.6 KB) - added by SergeyBiryukov 3 years ago.

Download all attachments as: .zip

Change History (14)

@mattoakley
3 years ago

Patch hides bulk edit options in terms list if no terms are found.

#1 @mattoakley
3 years ago

  • Keywords has-patch added; needs-patch removed
  • Resolution set to invalid
  • Status changed from new to closed

Hi @swissspidy, I have raised a patch for this :)

#2 @swissspidy
3 years ago

  • Keywords needs-testing added
  • Milestone changed from Awaiting Review to 5.9
  • Resolution invalid deleted
  • Status changed from closed to reopened

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


3 years ago

#4 @audrasjb
3 years ago

  • Keywords needs-refresh added; needs-testing removed

Thanks for the patch @mattoakley!

I can see a couple issues in your patch:

  • please generate it against the root directory (/wordpress-develop) instead of the location of the file
  • we need an @since 5.9.0 mention in the Total items and Determines if the current screen has results Docblocks
  • we need an @var int mention in the Total items Docblock

The rest looks good to me 🙂

#5 @audrasjb
3 years ago

By the way, the patch fixes the issue, so we're almost good :)
See screenshots below

@audrasjb
3 years ago

After patch: with one or more items, the bulk actions and searchbar display correctly

@audrasjb
3 years ago

After patch : when there is no item, the searchbar and the bulk actions are removed

@mattoakley
3 years ago

Patch hides bulk edit options in terms list if no terms are found, patch includes more accurate doc blocks

#6 @audrasjb
3 years ago

  • Keywords commit has-screenshots added; needs-refresh removed

Thanks for the quick refresh!
The patch looks good to me, marking for commit.

#7 @SergeyBiryukov
3 years ago

Hi there, thanks for the patch!

I have a minor note: the @since version for the has_items() method should be 3.1.0 instead of 5.9.0, as it was introduced in WordPress 3.1, see [17026] / #15857.

Otherwise, the patch seems simple enough and looks good for the approach it implements.

However, I would like to take a slightly different approach here and implement what the @todo note says instead:

Populate $this->items in prepare_items().

That would allow us to remove the has_items() method override altogether and just use the parent method from the WP_List_Table class. It would also be more consistent with other list table classes.

Last edited 3 years ago by SergeyBiryukov (previous) (diff)

#8 @SergeyBiryukov
3 years ago

54181.3.diff implements comment:7 by:

  • Moving the get_terms() call from display_rows_or_placeholder() to prepare_items().
  • Populating $this->items property in prepare_items().
  • Removing the has_items() override, no longer needed.

With these changes, the parent WP_List_Table::has_items() method works as expected, and the bulk actions dropdown is not displayed if there are no terms.

#9 @SergeyBiryukov
3 years ago

  • Owner set to SergeyBiryukov
  • Resolution set to fixed
  • Status changed from reopened to closed

In 51896:

Taxonomy: Populate the WP_Terms_List_Table::$items property in ::prepare_items().

This allows the parent WP_List_Table::has_items() method to work as expected, and the override in the child class can now be removed. It also makes the class more consistent with other list table classes.

As a result of this change, the "Bulk actions" dropdown is no longer unnecessarily displayed if there are no terms.

Follow-up to [15491], [17025], [17026].

Props mattoakley, swissspidy, audrasjb, SergeyBiryukov.
Fixes #54181.

Note: See TracTickets for help on using tickets.