WP List Table: allow filtering view switcher modes

[ragulka]

I suggest the view mode switcher for post/media/site list views should be filterable, so it's possible to:

• add additional, custom view modes (especially for custom post types)
• filter available view modes
• remove view modes completely (effectively removing the switcher from HTML)

This would allow developers to customise the switcher for different custom post types. At the moment, this is impossible - and in some cases the switcher may make no sense or have no effect at all, if a specific mode isn't implemented.

[ragulka]

Looking a bit further into this, I see at least these points that need addressing:

posts_list_mode user setting affects all post types

All custom post type list tables use the WP_Posts_List_Table class, which uses the posts_list_mode user setting to determine, which view mode to use by default. This means that the setting will be the same across all different post types. I'm not sure this behaviour is ideal in the first place - perhaps I want to use list mode for posts, but excerpt mode for books?

This raises 2 questions:

1. Should we perhaps consider using a post-type specific user setting here, instead? Something like {$custom_post_type}_list_mode
2. Or if we want to keep this still a single user setting, what happens if the posts_list_mode is excerpt and I remove excerpt from the $modes? Let's say we only have list and grid instead. Should we probably just display grid (or whichever is the first) and ignore the user setting?

Where should we do the filtering?

I can see a few possible places:

• https://core.trac.wordpress.org/browser/trunk/src/wp-admin/includes/class-wp-list-table.php#L128 - this would make the most sense, because we know this way the filter would be run only once. However, since $modes property can be provided in the child class, like https://core.trac.wordpress.org/browser/trunk/src/wp-admin/includes/class-wp-media-list-table.php#L25, this would mean that the child class would have to make sure to apply the filter as well. In core, this shouldn't be a problem (only Media list is providing custom modes), but could become messy an unreliable with custom list tables, if developers forget to apply the filter.
• https://core.trac.wordpress.org/browser/trunk/src/wp-admin/includes/class-wp-list-table.php#L544 - this would ensure that the modes are always filtered. This happens at runtime, which could possibly make it less efficient, but then again - the view mode switcher is only displayed once per screen, so in reality, it doesn't affect performance
• Perhaps a possible approach would be to make WP_Media_List_table use the filter instead of providing view modes in the constructor?

[DrewAPicture]

Hi ragulka, sorry it took so long for somebody to get back to you. Thanks for the patch.

I think there's merit in making the list mode filterable on a per-screen basis like this, as was actually suggested in comment:13:ticket:20335. Moving to 4.2 for consideration.

By the way, the only nitpick on your patch is that there should be braces on the if statement. Otherwise, thank you for including hook docs in your initial patch, it warms my heart to see it :-)

[ragulka]

Patch with braces around if statement

[azaozz]

Hooks with "dynamic" names are really hard to use. Not sure if we need another one here. Maybe consider using a "static" name for the filter and passing the screen ID as argument?

[ragulka]

Yeah, that's totally possible. I just was not sure which pattern to use, because WordPress seems to use both. So I take it that "dynamic" filter/action names are discouraged then?

[DrewAPicture]

30325.diff​ updates the hook name to view_switcher_modes and passes the screen id as a parameter. Moving for commit consideration.

[ocean90]

Replying to DrewAPicture:

30325.diff​ updates the hook name to view_switcher_modes and passes the screen id as a parameter.

I think it would be better if we pass the entire screen object and not only the screen ID. Or maybe $this?

[ocean90]

Punted from 4.2 per the bug scrub. The patch doesn't handle the case when $current_mode is removed from the list.

Also posts and media list table have a harcoded whitelist:

// Posts
$mode = $_REQUEST['mode'] == 'excerpt' ? 'excerpt' : 'list';
set_user_setting ( 'posts_list_mode', $mode );

// Media
$mode = get_user_option( 'media_library_mode', get_current_user_id() ) ? get_user_option( 'media_library_mode', get_current_user_id() ) : 'grid';
$modes = array( 'grid', 'list' );

if ( isset( $_GET['mode'] ) && in_array( $_GET['mode'], $modes ) ) {
    $mode = $_GET['mode'];
         update_user_option( get_current_user_id(), 'media_library_mode', $mode );
}

[ragulka]

Hi, ocean90 - any suggestions/ideas how missing $current_mode should be handled? Posts/Media hardcoded lists can be changed to allow custom modes, as far as I understand.

[afercia]

Not sure this still fully applies since the view mode switcher for posts is now moved into the screen options. See [35357] and [35461].