Make WordPress Core

Opened 7 years ago

Closed 3 years ago

Last modified 2 years ago

#42763 closed enhancement (fixed)

class-wp-list-table unnecessarily disables first and last pagination buttons

Reported by: wp_kc's profile wp_kc Owned by: hellofromtonya's profile hellofromTonya
Milestone: 5.9 Priority: normal
Severity: normal Version:
Component: General Keywords: has-patch commit
Focuses: ui, administration Cc:

Description

protected function pagination($which) in class-wp-list-table.php unnecessarily disables the << pagination button if you are on page 2 and disabled the >> pagination button if you are on the next to last page. That seems a bit pedantic and completely unnecessary.

If I am on page 2 of a multi-page list, I should be able to click either << or < to get back to the first page. If I am on the next to last page, I should be able to click either > or >> to get to the last page. It does not make sense to disable buttons that should logically work in these two conditions.

The following code starting at line 743 of class-wp-list-table.php...

<?php
                if ( $current == 1 ) {
                        $disable_first = true;
                        $disable_prev = true;
                }
                if ( $current == 2 ) {
                        $disable_first = true;
                }
                if ( $current == $total_pages ) {
                        $disable_last = true;
                        $disable_next = true;
                }
                if ( $current == $total_pages - 1 ) {
                        $disable_last = true;
                }

...should be changed too...

<?php
                if ( $current == 1 ) {
                        $disable_first = true;
                        $disable_prev = true;
                }
                if ( $current == $total_pages ) {
                        $disable_last = true;
                        $disable_next = true;
                }

Attachments (6)

42763.patch (1018 bytes) - added by ronakganatra 7 years ago.
Agree with wp_kc there is not need of more conditions regarding this functionality and applied patch for it .
42763.diff (1018 bytes) - added by ronakganatra 7 years ago.
42763.1.diff (702 bytes) - added by sabernhardt 4 years ago.
42763.2.diff (697 bytes) - added by Hareesh Pillai 3 years ago.
Patch refreshed
3d8a40a1ed7e466b2b24cb6d434f63c5.mp4 (1.8 MB) - added by audrasjb 3 years ago.
Testing 42763.2.diff
42763-before-after.gif (8.7 MB) - added by hellofromTonya 3 years ago.
Test Report: Before and After applying 42763.2.diff patch. Works as expected ✅

Change History (19)

#1 @wp_kc
7 years ago

  • Severity changed from normal to trivial

@ronakganatra
7 years ago

Agree with wp_kc there is not need of more conditions regarding this functionality and applied patch for it .

@ronakganatra
7 years ago

#2 @ronakganatra
7 years ago

  • Keywords has-patch added
  • Severity changed from trivial to normal

#3 @wp_kc
5 years ago

  • Keywords reporter-feedback added

I haven't tracked down where or why it happened... but the code was reverted back to its original form; the extra if() statements are back.

#4 @wp_kc
5 years ago

  • Keywords needs-refresh added
  • Version 4.9.1 deleted

#5 @wp_kc
5 years ago

  • Version set to trunk

#6 @knutsp
5 years ago

  • Version changed from trunk to 4.9.1

@sabernhardt
4 years ago

#7 @sabernhardt
4 years ago

  • Keywords needs-testing added; reporter-feedback needs-refresh removed
  • Version 4.9.1 deleted

updated patch

@Hareesh Pillai
3 years ago

Patch refreshed

#8 @Hareesh Pillai
3 years ago

  • Milestone changed from Awaiting Review to 5.9

@audrasjb
3 years ago

Testing 42763.2.diff

#9 @audrasjb
3 years ago

  • Keywords commit added; needs-testing removed
  • Owner set to audrasjb
  • Status changed from new to accepted

42763.2.diff works fine (see the video capture I shared above) and it makes sense to me. Marking this ticket for commit.

@hellofromTonya
3 years ago

Test Report: Before and After applying 42763.2.diff patch. Works as expected ✅

#10 @hellofromTonya
3 years ago

Test Report

Env:

  • Localhost: wp-env
  • OS: macOS Big Sur v 11.6
  • Browser: Chrome v 93.0.4577.82 and Firefox v 92.0
  • Imported theme unit test xml
  • Set the number of items on the page to 5 with this mu-plugin
    <?php
    
    add_filter( 'edit_posts_per_page', function() { return 5; } );
    

Test Before

Steps:

  1. Go toPosts page
  2. Click the next page button => should go to Page 2
  3. On Page 2, notice that the first page « button is disabled
  4. Click on the last page » button => should go to the last page
  5. Click on the previous page button
  6. Notice the last page » button is disabled

Results:

  • First page button is disabled when on Page 2 ✅
  • Last page button is disabled when on last page - 1 ✅

Able to reproduce ✅

After applying patch

Steps:

  1. Applied 42763.2.diff patch
  2. Repeat steps above; however this time the first and last page buttons should remain enabled

Results:

  • First page button is enabled when on Page 2 ✅
  • Last page button is enabled when on last page - 1 ✅

Works as expected ✅

#11 @hellofromTonya
3 years ago

  • Owner changed from audrasjb to hellofromTonya
  • Status changed from accepted to assigned

Reassigning to me for commit.

#12 @hellofromTonya
3 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 51880:

Administration: Enable first and last page buttons in WP_List_Table::pagination().

Previously the first and last page pagination buttons were disabled when on their next or previous page respectively. This commit removes that unnecessary logic to keep these buttons enabled and avoid confusion in the user's navigation workflow.

New behavior:

  • When on page 2, the go to first page « button is enabled
  • When on the page before the last page, the go to last page » button is enabled

Follow-up to [32948], [47219].

Props wp_kc, ronakganatra, knutsp, sabernhardt, Hareesh Pillai, audrasjb, hellofromTonya.
Fixes #42763.

This ticket was mentioned in Slack in #core-test by hellofromtonya. View the logs.


2 years ago

Note: See TracTickets for help on using tickets.