Make WordPress Core

Opened 7 years ago

Last modified 4 years ago

#38238 reviewing enhancement

Sorting a list table by some kind of count should default to DESC initially

Reported by: helen's profile helen Owned by: helen's profile helen
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Administration Keywords: has-patch needs-testing
Focuses: Cc:

Description

List tables that can be sorted by some kind of count (posts with that term, comments on that post, etc.) should likely use DESC for that sort initially, as more frequent user workflows involve wanting to see things with many X (popularity/usage), as opposed to items with no X (cleanup). For example, you may want to see which categories are most popular (related: #36964) or which posts have generated a lot of comments.

Should also consider how sortable custom columns indicate whether to use ASC or DESC initially.

Attachments (5)

38238.diff (2.1 KB) - added by terwdan 7 years ago.
38238.2.diff (4.2 KB) - added by Harold Angenent 7 years ago.
38238.3.diff (4.6 KB) - added by drebbits.web 7 years ago.
Only added submitted date sort in the comments table
38238.4.diff (11.0 KB) - added by rcutmore 7 years ago.
Add tests and update code formatting (based on 38238.2.diff)
38238.5.diff (11.2 KB) - added by deepaklalwani 4 years ago.
Refreshes the patch based on 38238.4.diff

Download all attachments as: .zip

Change History (16)

#1 @afercia
7 years ago

Related: #32170

... review the ordering associated with the "first click" on a header link. For example, in the Posts table clicking the Comments header the first time gives an ascending order by Comments count, showing the Posts with 0 comments first, while probably would be more useful to have the Posts with the higher number of Comments displayed first.

#2 @helen
7 years ago

  • Keywords good-first-bug added

@terwdan
7 years ago

#3 @terwdan
7 years ago

  • Keywords has-patch needs-testing needs-unit-tests added; needs-patch removed

I've attached a patch I made on WCNL2016. This should hopefully fix the issue by switching the ascending and descending on the column sort.

#4 @Harold Angenent
7 years ago

Attached a patch also created on WCNL, which holds a different approach.

This patch introduces a new way to communicate with the manage_{$this->screen->id}_sortable_columns filter, which uses an associative array instead of an indexed array to make each property more clear. Usage:

$sortable['column_name'] = array(
    'query_var' => 'query_var_name',
    'order'     => 'DESC',
);

The patch also switches around the default sorting behaviour (to DESC) for:

  • WP_Links_List_Table: rating
  • WP_Media_List_Table: comment_count
  • WP_MS_Users_List_Table: registered date
  • WP_Posts_List_Table: comment_count (as proposed in the ticket description)
  • WP_Terms_List_Table: posts count (as proposed in the ticket description)

#5 @drebbits.web
7 years ago

@harold-angenent 's approach seems to be the appropriate & flexible solution and developers can easily add the 'order' argument when adding their own custom column that has some sort of count quality.

@drebbits.web
7 years ago

Only added submitted date sort in the comments table

#6 @Harold Angenent
7 years ago

@drebbits.web Thanks for your feedback and help! I consciously chose to not make the date in the comments table descending by default, because of the comments of @afercia on #32416. Since comments are already displayed 'last comment first' by default, clicking "date" for the first time should probably change this around.

#7 @johnbillion
7 years ago

  • Owner set to helen
  • Status changed from new to reviewing

@rcutmore
7 years ago

Add tests and update code formatting (based on 38238.2.diff)

#8 @rcutmore
7 years ago

  • Keywords needs-unit-tests removed

I added 38238.4.diff, which is based on 38238.2.diff due to the previous comment about the date column in the comments table). This updates the code formatting to more closely follow the project coding guidelines and adds tests to check the default sorting in the various tables.

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


5 years ago

#10 @valentinbora
4 years ago

  • Keywords needs-refresh added; good-first-bug removed

Thanks @rcutmore for the patch. Unfortunately it no longer applies cleanly so I'll mark this for a refresh.

@helen do you still want to retain ownership of the ticket or shall we free it up for someone else to pick up?

@deepaklalwani
4 years ago

Refreshes the patch based on 38238.4.diff

#11 @deepaklalwani
4 years ago

  • Keywords needs-refresh removed

Attached a refreshed patch based on 38238.4.diff

Last edited 4 years ago by deepaklalwani (previous) (diff)
Note: See TracTickets for help on using tickets.