Make WordPress Core

Opened 9 years ago

Last modified 5 years ago

#30325 new enhancement

WP List Table: allow filtering view switcher modes

Reported by: ragulka's profile ragulka Owned by:
Milestone: Priority: normal
Severity: normal Version: 4.0
Component: General Keywords: has-patch
Focuses: ui, administration Cc:

Description

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.

Attachments (3)

view-switcher-modes-filter.30325.diff (1.1 KB) - added by ragulka 9 years ago.
view-switcher-modes-filter.30325-2.diff (1.1 KB) - added by ragulka 9 years ago.
Patch with braces around if statement
30325.diff (1.0 KB) - added by DrewAPicture 9 years ago.

Download all attachments as: .zip

Change History (14)

#1 @ragulka
9 years ago

  • Focuses ui added

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:

#2 @DrewAPicture
9 years ago

  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 4.2

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
9 years ago

Patch with braces around if statement

#3 @azaozz
9 years ago

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?

#4 @ragulka
9 years ago

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?

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


9 years ago

@DrewAPicture
9 years ago

#6 follow-up: @DrewAPicture
9 years ago

  • Keywords commit added

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

#7 in reply to: ↑ 6 @ocean90
9 years ago

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?

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


9 years ago

#9 @ocean90
9 years ago

  • Keywords commit removed
  • Milestone changed from 4.2 to Future Release

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 );
}

#10 @ragulka
9 years ago

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.

#11 @afercia
8 years ago

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

Note: See TracTickets for help on using tickets.