WordPress.org

Make WordPress Core

Opened 11 months ago

Last modified 4 months ago

#45089 assigned enhancement

Update WP_List_Table::get_sortable_columns() to support 'asc' and 'desc' arguments

Reported by: Tkama Owned by: birgire
Milestone: 5.3 Priority: normal
Severity: normal Version: 4.9.8
Component: Administration Keywords: has-patch needs-testing has-unit-tests
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)

class-wp-list-table.php.patch (986 bytes) - added by Tkama 11 months ago.
45089.patch (923 bytes) - added by Shital Patel 11 months ago.
create patch
45089.diff (922 bytes) - added by Shital Patel 11 months ago.
45089.2.diff (1.4 KB) - added by Tkama 11 months ago.
45089-2.diff (5.5 KB) - added by birgire 4 months ago.

Download all attachments as: .zip

Change History (10)

#1 @SergeyBiryukov
11 months 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

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.

@Shital Patel
11 months ago

create patch

@Shital Patel
11 months ago

@Tkama
11 months ago

#2 @pento
9 months ago

  • Milestone changed from 5.1 to 5.2

Patch needs review.

#3 @Shital Patel
8 months ago

  • Keywords needs-testing added

#4 @desrosj
6 months 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.

@birgire
4 months ago

#5 @birgire
4 months 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.

Note: See TracTickets for help on using tickets.