#45089 closed enhancement (fixed)
Update WP_List_Table::get_sortable_columns() to support 'asc' and 'desc' arguments
Reported by: | Tkama | Owned by: | birgire |
---|---|---|---|
Milestone: | 5.5 | Priority: | normal |
Severity: | normal | Version: | 4.9.8 |
Component: | Administration | Keywords: | has-patch needs-testing has-unit-tests commit |
Focuses: | administration | Cc: |
Description
When registering new sortable column we can set default first sort order, by setting false/true in the second parameter of array:
<?php function get_sortable_columns(){ return array( 'register_date' => array( 'register_date', false ), // false = asc 'login_date' => array( 'login_date', true ), 'rating' => array( 'rating', false ), ); }
It is much more clear to write 'asc' / 'desc' then false/true.
To achieve such behavior and not lose current one, we need to change /wp-admin/includes/class-wp-list-table.php#L16
<?php if ( $current_orderby === $orderby ) { $order = 'asc' === $current_order ? 'desc' : 'asc'; $class[] = 'sorted'; $class[] = $current_order; } else { $order = $desc_first ? 'desc' : 'asc'; $class[] = 'sortable'; $class[] = $desc_first ? 'asc' : 'desc'; }
to
<?php ... } else { if( in_array( strtolower($desc_first), array('desc', 'asc'), true ) ) $order = strtolower( $desc_first ); else $order = $desc_first ? 'desc' : 'asc'; $class[] = 'sortable'; $class[] = $order === 'desc' ? 'asc' : 'desc'; }
Attachments (5)
Change History (19)
#1
@
6 years ago
- Component changed from General to Administration
- Keywords has-patch needs-docs added
- Milestone changed from Awaiting Review to 5.1
- Summary changed from WP_List_Table to Update WP_List_Table::get_sortable_columns() to support 'asc' and 'desc' arguments
#4
@
6 years ago
- Keywords needs-docs removed
- Milestone changed from 5.2 to 5.3
Patch still needs review. Punting to 5.3 as 5.2 beta is in less than 2 days. If a committer has time to properly review and commit this, it can be moved back.
#5
@
5 years ago
- Focuses administration added
- Keywords has-unit-tests added
- Owner set to birgire
- Status changed from new to assigned
This seems like a good improvement.
In 45089-2.diff I updated the patch with minor code standard fixes and test cases:
test_sortable_columns()
test_sortable_columns_with_current_ordering()
for the WP_Comments_List_Table
class.
The WP_List_Table::get_sortable_columns()
method is protected. I therefore considered adding new test classes (as new files) that extended an existing list table class, like WP_Comments_List_Table
. That way we could modify the {{get_sortable_columns()}}} method. Anonymous classes might have made that more cleaner, but it's only available in PHP 7, so I ended with using the mock builder to override the method instead.
This will need an owner for the 5.3 milestone, so I add myself as an owner to help with that.
#6
@
5 years ago
- Milestone changed from 5.3 to 5.4
This still needs some testing, and with 5.3 beta 1 in a few hours, we've run out of time.
This ticket was mentioned in Slack in #core by david.baumwald. View the logs.
5 years ago
This ticket was mentioned in Slack in #core by david.baumwald. View the logs.
5 years ago
#9
@
5 years ago
- Milestone changed from 5.4 to Future Release
This ticket still needs testing, and with 5.4 Beta 1 landing tomorrow, this is being moved to Future Release
. If any maintainer or committer feels this can be included in 5.4 or wishes to assume ownership during a specific cycle, feel free to update the milestone accordingly.
Hi @Tkama, thanks for the patch!
The documentation for WP_List_Table::get_sortable_columns() would also need to be updated to reflect the new accepted value.