WordPress.org

Make WordPress Core

Opened 4 years ago

Last modified 3 months ago

#42066 assigned enhancement

List tables: consider a new method to generate the views links markup

Reported by: afercia Owned by: costdev
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Administration Keywords: has-patch needs-testing
Focuses: Cc:

Description

Splitting this out from #32399.

Most of the List Tables used in core comes with some "views links" to switch the table to a subset of items, say published, drafts, trashed posts or approved, pending comments or different user types.

While the logic to get the views differs depending on the type of items (post status, user role, active tab, etc.) the code that actually generates the links markup is basically always the same and could benefit from some abstraction in a general method. See also 32399#comment:32.

/cc @flixos90

Attachments (1)

42066.functions.php.diff (2.8 KB) - added by costdev 5 months ago.

Download all attachments as: .zip

Change History (10)

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


5 months ago

#2 @peterwilsoncc
5 months ago

  • Milestone changed from Awaiting Review to 5.9

#3 @costdev
5 months ago

This ticket was raised in a triage meeting on Slack and it was agreed that abstracting the link generation makes sense. The ticket has been set for the 5.9 milestone.

#4 @costdev
5 months ago

To get the ball rolling, I'm attaching 42066.functions.php.diff with a general function for discussion/consideration.

  • Doing a search for the get_views() method, I've identified 11 files that generate admin status links.
  • I have diffs that convert each of these to use the new function, with some clean up in the get_views() methods. I'll attach these if this is a goer.
  • There will be some other files with a similar format but in a method not called get_views().
  • wp-class-media-list-table.php generates <option> elements. A similar general function could be written for this as well.

I've called the function generate_links(), as it can be used in virtually any scenario.

  1. Accepts an array of arrays containing the data for each link.
  2. If the is_current (bool) key is true, the function will add class="content" aria-content="page".
  3. Allows for an array of additional attributes to be specified in the attributes key.
  4. Handles invalid/empty arguments - I'll add unit tests if this is a goer.
  5. Doesn't perform esc_url() or esc_attr() and leaves that up to the caller.

---

Calling the function looks like so:

<?php

$post_link_data = array(
    'all' => array(
        'url'        => 'edit.php',
        'label'      => 'All',
        // `generate_links()` adds/appends `class="current" and aria-current="page"`.
        // You can specify your own by setting this to `false` and adding your own
        // `class` and `aria-current` key/value pairs to the `attributes` array below.
        'is_current' => true,
        'attributes' => array(
            'title'  => _( 'View All Posts' ),
            'id'     => 'all-of-the-posts',
            'class'  => 'button-primary',
        ),
    ),
    // ...
);

$post_links = generate_links( $post_link_data );
/*
$post_links = array(
   'all' => '<a href="edit.php"
                title="View All Posts"
                id="all-of-the-posts"
                class="button-primary current"
                aria-current="page">All</a>',
    // ...
);
*/

For links that have a similar set of attributes, we could have a wrapper function specifically for say, the admin status links, to apply attributes as needed.

e.g.

<?php

function generate_admin_status_links( $link_data ) {
    // Additional attributes
    $link_data = array_map(
        function( $link ) {
            $link['attributes']['admin-attribute-1'] = 'Howdy, admin!';
            $link['attributes']['admin-attribute-2'] = 'Howdy again, admin!';
            return $link;
        },
        $link_data
    );
    return generate_links( $link_data );
}

You could also do the same for a generate_clean_links() wrapper that performs esc_url(), esc_attr(), etc. on the data before calling generate_links().

N.B. The indentation on the diff is a little messed up on trac. It was run through WPCS before uploading. It's best to download and apply for readability.

Notes

  • Can replace array_map() calls with foreach( $link_data as &$link ) for performance per this benchmark.
Last edited 4 months ago by costdev (previous) (diff)

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


4 months ago

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


3 months ago

#7 @audrasjb
3 months ago

  • Keywords has-patch needs-testing added

Reviewed during today's bug srcub.
Marking this as needs-testing to make sure it's peer-reviewed and works as expected :)

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


3 months ago

#9 @audrasjb
3 months ago

  • Milestone changed from 5.9 to Future Release
  • Owner set to costdev
  • Status changed from new to assigned

As per today's bug scrub, this ticket needs a general consensus on the proposed approach. Moving to Future release and adding @costdev as the ticket owner.

Note: See TracTickets for help on using tickets.