WordPress.org

Make WordPress Core

Opened 3 years ago

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

Download all attachments as: .zip

Change History (25)

@enej
3 years ago

screenshot

@enej
3 years ago

patch that removed the selection of all.

#1 follow-up: @enej
3 years ago

  • Keywords has-patch added; needs-patch removed

This patch should fix it.

@rmarks
3 years ago

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

#2 in reply to: ↑ 1 @rmarks
3 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.

#3 @helen
3 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.

#4 @helen
3 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

#5 @helen
3 years ago

  • Keywords ui-focus added

#6 @sworddance
3 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 3 years ago by sworddance (previous) (diff)

#7 @sworddance
3 years ago

  • Cc sworddance added

@rmarks
3 years ago

#8 @rmarks
3 years ago

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

#9 @pauldewouters
3 years ago

I tested the patch and can confirm it fixes the issue

#10 @pauldewouters
3 years ago

  • Cc pauldewouters@… added

#11 @jeremyfelt
2 years ago

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

#12 @wonderboymusic
17 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

#13 @wonderboymusic
17 months ago

  • Keywords has-patch added; needs-patch removed

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

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


15 months ago

#15 @DrewAPicture
15 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 15 months ago by DrewAPicture (previous) (diff)

#16 follow-up: @wonderboymusic
15 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.

#17 in reply to: ↑ 16 @nacin
15 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".

#18 @DrewAPicture
14 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.

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


14 months ago

#20 @wonderboymusic
14 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.