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 | Owned by: | SergeyBiryukov |
---|---|---|---|
Milestone: | 5.9 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Taxonomy | Keywords: | good-first-bug has-patch commit has-screenshots |
Focuses: | administration | Cc: |
Description
Attachments (5)
Change History (14)
#1
@
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
@
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
@
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 theTotal items
andDetermines if the current screen has results
Docblocks - we need an
@var int
mention in theTotal items
Docblock
The rest looks good to me 🙂
#5
@
3 years ago
By the way, the patch fixes the issue, so we're almost good :)
See screenshots below
@
3 years ago
Patch hides bulk edit options in terms list if no terms are found, patch includes more accurate doc blocks
#6
@
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
@
3 years ago
Hi there, thanks for the patch!
I have a minor note: the @since
version for the has_items()
methods 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
inprepare_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.
#8
@
3 years ago
54181.3.diff implements comment:7 by:
- Moving the
get_terms()
call fromdisplay_rows_or_placeholder()
toprepare_items()
. - Populating
$this->items
property inprepare_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.
Patch hides bulk edit options in terms list if no terms are found.