WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 4 months ago

#24869 closed defect (bug) (fixed)

"All" in post list table is incorrectly selected in some filtered views

Reported by: enej Owned by: wonderboymusic
Milestone: 4.2 Priority: normal
Severity: minor Version: 3.5.2
Component: Posts, Post Types Keywords: has-patch needs-testing
Focuses: ui, administration Cc:

Description

When you are viewing the posts table and you have selected an author the all filter is incorrectly selected.

Attachments (5)

Posts_‹_CMS_Dev_—_WordPress.png (153.6 KB) - added by enej 2 years ago.
screenshot
24869.diff (1.1 KB) - added by enej 2 years ago.
patch that removed the selection of all.
24869.2.diff (1.1 KB) - added by rmarks 2 years ago.
This patch includes 24869.diff​ by enej plus adds the cases when a category or tag is specified
24869.alt.diff (1.0 KB) - added by rmarks 2 years ago.
24869.3.diff (1.7 KB) - added by wonderboymusic 7 months ago.

Download all attachments as: .zip

Change History (25)

@enej2 years ago

screenshot

@enej2 years ago

patch that removed the selection of all.

comment:1 follow-up: @enej2 years ago

  • Keywords has-patch added; needs-patch removed

This patch should fix it.

@rmarks2 years ago

This patch includes 24869.diff​ by enej plus adds the cases when a category or tag is specified

comment:2 in reply to: ↑ 1 @rmarks2 years ago

  • Cc rmarks@… added
  • Summary changed from "All" in post list is incorectly selected when an author is selected. to "All" in post list is incorectly selected when an author, category, or tag is selected.

Replying to enej:

This patch should fix it.

I added to your patch by including the exceptions when a category or tag was specified.

comment:3 @helen2 years ago

  • Summary changed from "All" in post list is incorectly selected when an author, category, or tag is selected. to "All" in post list table is incorerctly selected in some filtered views

Is there something we can do besides continuing to add to the conditional? For example, as what is probably a TERRIBLE idea, comparing the post count of all vs. the current view.

comment:4 @helen2 years ago

  • Summary changed from "All" in post list table is incorerctly selected in some filtered views to "All" in post list table is incorrectly selected in some filtered views

comment:5 @helen2 years ago

  • Keywords ui-focus added

comment:6 @sworddance2 years ago

@helen - "Is there something we can do besides continuing to add to the conditional?"

An alternative would be to see if the only $_REQUEST parameter is post_type. so something like:

count($_REQUEST) == 0 || count($_REQUEST) == 1 && isset($_REQUEST['post_type'])

Last edited 2 years ago by sworddance (previous) (diff)

comment:7 @sworddance2 years ago

  • Cc sworddance added

@rmarks2 years ago

comment:8 @rmarks2 years ago

I have just uploaded an alternate diff that accomplishes @enej's intentions while using @sworddance's suggestions.

comment:9 @pauldewouters2 years ago

I tested the patch and can confirm it fixes the issue

comment:10 @pauldewouters2 years ago

  • Cc pauldewouters@… added

comment:11 @jeremyfelt18 months ago

  • Component changed from Administration to Posts, Post Types
  • Focuses admin added

comment:12 @wonderboymusic7 months ago

  • Keywords needs-patch added; has-patch removed
  • Milestone changed from Awaiting Review to 4.2
  • Owner set to wonderboymusic
  • Status changed from new to assigned

I have been in list tables pretty deep recently, I would like to fix this

@wonderboymusic7 months ago

comment:13 @wonderboymusic7 months ago

  • Keywords has-patch added; needs-patch removed

24869.3.diff fixes this, sorry it sat for so long, enej!

comment:14 @slackbot5 months ago

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

comment:15 @DrewAPicture5 months ago

  • Keywords needs-docs added

Latest patch still applies, appears to fix the problem. The new method, is_base_request(), should have a DocBlock. Otherwise looks ready for commit consideration.

Last edited 5 months ago by DrewAPicture (previous) (diff)

comment:16 follow-up: @wonderboymusic4 months ago

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

In 31828:

Add WP_Posts_List_Table::is_base_request() to determine if the current "view" is the "All" (default) view.

Fixes #24869.

comment:17 in reply to: ↑ 16 @nacin4 months ago

Replying to wonderboymusic:

In 31828:

Add WP_Posts_List_Table::is_base_request() to determine if the current "view" is the "All" (default) view.

Fixes #24869.

@wonderboymusic, I don't think this is a problem in terms of how this is implemented (perhaps just how it is documented), but note that for Contributors and Authors (I think), the first section is actually "Mine", not "All".

comment:18 @DrewAPicture4 months ago

  • Keywords needs-testing added; needs-docs removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

See comment:17. Also, not sure this is working correctly even for roles that lack the "Mine" section. When the "All" section is selected, none of them have the current class. I think we need another look here.

comment:19 @slackbot4 months ago

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

comment:20 @wonderboymusic4 months ago

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

In 31959:

In WP_Posts_List_Table::get_views(), don't add the current class to the all status link if ->user_posts_count has a value, which triggers the additional mine status.

See [31828].
Fixes #24869.

Note: See TracTickets for help on using tickets.