WordPress.org

Make WordPress Core

Opened 22 months ago

Closed 8 weeks 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 22 months ago.
screenshot
24869.diff (1.1 KB) - added by enej 22 months ago.
patch that removed the selection of all.
24869.2.diff (1.1 KB) - added by rmarks 22 months 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 22 months ago.
24869.3.diff (1.7 KB) - added by wonderboymusic 4 months ago.

Download all attachments as: .zip

Change History (25)

@enej22 months ago

screenshot

@enej22 months ago

patch that removed the selection of all.

comment:1 follow-up: @enej22 months ago

  • Keywords has-patch added; needs-patch removed

This patch should fix it.

@rmarks22 months 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 @rmarks22 months 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 @helen22 months 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 @helen22 months 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 @helen22 months ago

  • Keywords ui-focus added

comment:6 @sworddance22 months 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 22 months ago by sworddance (previous) (diff)

comment:7 @sworddance22 months ago

  • Cc sworddance added

@rmarks22 months ago

comment:8 @rmarks22 months ago

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

comment:9 @pauldewouters22 months ago

I tested the patch and can confirm it fixes the issue

comment:10 @pauldewouters22 months ago

  • Cc pauldewouters@… added

comment:11 @jeremyfelt16 months ago

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

comment:12 @wonderboymusic4 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

@wonderboymusic4 months ago

comment:13 @wonderboymusic4 months ago

  • Keywords has-patch added; needs-patch removed

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

comment:14 @slackbot2 months ago

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

comment:15 @DrewAPicture2 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 2 months ago by DrewAPicture (previous) (diff)

comment:16 follow-up: @wonderboymusic2 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 @nacin2 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 @DrewAPicture2 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 @slackbot8 weeks ago

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

comment:20 @wonderboymusic8 weeks 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.