Make WordPress Core

Opened 11 years ago

Closed 9 years ago

Last modified 7 years ago

#25408 closed task (blessed) (fixed)

Ability to specify that a list table column is "primary"

Reported by: helen's profile helen Owned by: stephdau's profile stephdau
Milestone: 4.3 Priority: normal
Severity: normal Version:
Component: Administration Keywords:
Focuses: Cc:

Description

List tables have a "primary" column of sorts; that is, the one with the row actions. We should be able to specify which column is the primary one via API, apply an appropriate class, and display the row actions in cells in that column. Currently, developers customizing the list table columns often need to re-create row actions if they do not use the post name column, or just live with the row actions appearing in a less sensible spot.

This thought stems from three things in particular:

  1. The above customization example.
  2. Old experiments (nearly exactly 2 years ago) in making list tables responsive: http://cl.ly/3F1L0N3z1M0F0Z3t0X3W in #18198.
  3. The assortment of selectors in [25595] that are not particularly robust and not easily reused by developers. We should be able to target something like .has-row-actions instead.

Attachments (27)

25408.diff (46.1 KB) - added by DaveAl 10 years ago.
25408.2.diff (49.5 KB) - added by DaveAl 10 years ago.
25408.3.diff (57.8 KB) - added by jesin 10 years ago.
Modified 25408.2.diff based on WordPress 4.0-alpha
25408.4.diff (65.5 KB) - added by stephdau 9 years ago.
25408.5.diff (59.5 KB) - added by stephdau 9 years ago.
25408.6.diff (59.9 KB) - added by stephdau 9 years ago.
25408.7.diff (60.0 KB) - added by stephdau 9 years ago.
25408.8.diff (64.5 KB) - added by stephdau 9 years ago.
25408.9.diff (65.1 KB) - added by stephdau 9 years ago.
25408.10.diff (64.7 KB) - added by stephdau 9 years ago.
25408.11.diff (64.7 KB) - added by stephdau 9 years ago.
25408.12.diff (63.7 KB) - added by stephdau 9 years ago.
25408.13.diff (65.0 KB) - added by stephdau 9 years ago.
25408.14.diff (65.0 KB) - added by stephdau 9 years ago.
25408.15.diff (64.4 KB) - added by stephdau 9 years ago.
25408.16.diff (64.4 KB) - added by stephdau 9 years ago.
25408.17.diff (416 bytes) - added by boonebgorges 9 years ago.
25408.18.diff (1.5 KB) - added by stephdau 9 years ago.
25408.19.diff (3.1 KB) - added by helen 9 years ago.
25408-users-list-table.diff (869 bytes) - added by stephdau 9 years ago.
25408-list-table-headers-test.diff (647 bytes) - added by stephdau 9 years ago.
25408-missing-array-indexes.diff (811 bytes) - added by georgestephanis 9 years ago.
25408-list-table-headers-test.2.diff (646 bytes) - added by stephdau 9 years ago.
An alternative to 25408-list-table-headers-test.diff, if we wanted to future-proof things a bit.
25408.20.diff (730 bytes) - added by kovshenin 9 years ago.
25408.21.diff (835 bytes) - added by kovshenin 9 years ago.
25408-22.diff (1.2 KB) - added by mordauk 9 years ago.
25408.22.diff (1.3 KB) - added by helen 9 years ago.

Download all attachments as: .zip

Change History (122)

#1 @helen
11 years ago

  • Milestone changed from Awaiting Review to 3.7

Putting in 3.7 especially to address point 3 - if not done extremely soon, we should punt it again.

#2 @helen
11 years ago

  • Type changed from enhancement to task (blessed)

#3 @DaveAl
11 years ago

EDIT:
Ok, I see I've misread the requirements. In any case, I've rectified that situation and attached a patch. Feedback is appreciated.

I made some mistakes in the first attachment, so please ignore it and use the second one .

In essence:
I've added two new methods to class WP_List_Table, both intended to be over-ridden, but not abstract:

  1. get_primary_column(): Takes no arguments, returns the name of the desired primary column. Hardcoded much the same way as get_columns, but just one string.
  2. handle_actions( $item, $column_name, $primary ): Takes three arguments: the item - whether post, user, comment, etc. - the current column_name, and the name of the primary column. If the column name matches the primary, then the function returns the actions to be displayed.

I've also modified the various list tables that display row actions to use this new functionality. Some have single_row over-ridden, some don't. Some override the row_actions function, others just directly created and displayed their row actions in the default primary. The latter ones are where I extracted that code to override handle_actions.

The only one I didn't touch was class-wp-links-list-table.php because I don't have the links manager plugin to test it with yet, so I left that one alone.

Here's a list of the files I modified, which will now have the class "has-row-actions" in the primary cells.
class-wp-comments-list-table.php
class-wp-list-table.php
class-wp-media-list-table.php
class-wp-ms-sites-list-table.php
class-wp-ms-themes-list-table.php
class-wp-ms-users-list-table.php
class-wp-plugins-list-table.php
class-wp-posts-list-table.php
class-wp-terms-list-table.php
class-wp-users-list-table.php

Last edited 10 years ago by DaveAl (previous) (diff)

#4 @johnbillion
10 years ago

  • Keywords needs-patch added
  • Milestone changed from 3.7 to Future Release

@DaveAl
10 years ago

@DaveAl
10 years ago

#5 @DaveAl
10 years ago

  • Keywords has-patch needs-testing added; needs-patch removed

#6 @helen
10 years ago

Related: #28011, #26302.

Putting into 4.0 - this will simplify a lot of things in JS and responsive CSS, make accessibility better, and enhance developer happiness. Will review patch, unless somebody beats me to it.

#7 @jesin
10 years ago

  • Keywords needs-refresh added; has-patch needs-testing removed

By looking at the patch and the current file I can say that the line numbers have changed a lot since the patch was uploaded. The code in the original files have changed too.

#8 @samuelsidler
10 years ago

  • Milestone changed from Future Release to 4.0

@jesin
10 years ago

Modified 25408.2.diff based on WordPress 4.0-alpha

#9 @jesin
10 years ago

  • Keywords has-patch needs-testing added; needs-refresh removed

#10 @helen
10 years ago

  • Type changed from task (blessed) to enhancement

A lot going on in the patch, late for 4.0 (as much as obviously I want this). Patch seems to be heading in a good direction. One thing I'm noticing is setting HTML classes - should append to the set if primary column, rather than if/else, so it's easier to follow. Places where it's echoed inline should probably be set to a variable first for clarity as well.

#11 @helen
10 years ago

  • Keywords 4.1-early added
  • Milestone changed from 4.0 to Future Release

This ticket was mentioned in IRC in #wordpress-dev by helen. View the logs.


10 years ago

#13 @jeremyfelt
9 years ago

In 31674:

Reveal row actions on focus in the network users list table.

In the future, we'll re-factor this selector pile in a way that makes things cleaner. See #25408.

Props afercia.

Fixes #31003.

This ticket was mentioned in Slack in #design by helen. View the logs.


9 years ago

#15 @helen
9 years ago

  • Milestone changed from Future Release to 4.3

In conjunction with #32395.

#16 @helen
9 years ago

  • Owner set to stephdau
  • Status changed from new to assigned

#17 @stephdau
9 years ago

@helen: Kept running with 25408.3.diff, as discussed on Slack, but it never got to dealing how a primary column is actually selected/changed. The user-level setting of primary column sounds like it should be a screen option paired with the current "Show on screen" one (lists shown/hidden columns when dealing with a list table).

This is where things get a bit complex.

  • "Show on screen" is in wp-admin/includes/screen.php
  • That's JS-enabled so that what the user selects is rendered live
  • Assuming the selection of the primary column would be a drop-down (preferred because of interactions below) or series of radio (meh, considering we list columns as checkboxes above) underneath that list of displayed columns
  • We have to select the default in initial page load
  • Then, if a user hides a column, it should be taken away from the primary columns list, live, and said list revert to its default (some special columns can't be hidden).
  • Should be added back, live, if unhidden.
  • In an ideal world, user shouldn't have to "apply" primary column change, it should be live too

What's your (and others') perspective on this?

#18 @stephdau
9 years ago

Result of chat session with Helen regarding previous comment:

  • no UI, just programmatic column selection (ie: via filter)
  • 1 generic filter, with context passed via arguments
  • at least for "round 1"

That makes my day. Going with that.

@stephdau
9 years ago

#19 @stephdau
9 years ago

Here we go: 25408.4.diff:

  • was rebuilt from scratch based on the previous patches, brings the code in line with trunk (4.3-alpha, r32551)
  • builds upon the previous work: refactoring and based on ticket feedback
  • adds a list_table_primary_column filter in WP_List_Table::get_primary_column_name(), so developers can change the default column via the related API, with $default (returnable) and $context (screen id) as arguments (as requested).
Last edited 9 years ago by stephdau (previous) (diff)

#20 @stephdau
9 years ago

Noticing my IDE removed some of the whitespace around parenthesis and such (when I cut and pasted)... Will fix in a future patch. Code works the same, for testing.

@stephdau
9 years ago

#21 @stephdau
9 years ago

25408.5.diff:

  • sets the plugin column as being the default again (my mistake in prev patch)
  • cleans up some whitespace issues in the previous patch (IDE auto-reformat on paste)

#22 follow-up: @afercia
9 years ago

Tested a bit the patch, thanks very much @stephdau :) Noticed a few things worth considering:

  • small typo: for Posts/Pages the default primary column is set to "author"
  • wouldn't be better to use $this->screen->id as dynamic portion of the hook name?
  • doing the above, would we really need to pass $this->screen->id as additional variable?
  • in /network/users.php the "Sites" column needs some special treatment, it needs the "has-row-actions" CSS class (links not revealed on focus see #31003 for reference)
  • is it worth it to apply all this new stuff to Link Manager too?
  • consider to update the selector in common.js

@stephdau
9 years ago

#23 in reply to: ↑ 22 ; follow-up: @stephdau
9 years ago

Replying to afercia:

Tested a bit the patch, thanks very much @stephdau :)

Thank YOU, for the testing. :)
Added new patch: 25408.6.diff. See further info below below.

Noticed a few things worth considering:

  • small typo: for Posts/Pages the default primary column is set to "author"

Fixed in new patch, thanks. Guess that was set to my last test. :)

  • wouldn't be better to use $this->screen->id as dynamic portion of the hook name?
  • doing the above, would we really need to pass $this->screen->id as additional variable?

Personally, as a dev, I'd like both a generic with context as arg (as per prev patch), or one as you describe (context via dynamic hook name). I see value in both. Passing it as an arg is what @helen and I had decided for this round, but I'm not attached to either. See WP_List_Table::get_primary_column_name() in new patch for an example of both, from which we can decide.

  • in /network/users.php the "Sites" column needs some special treatment, it needs the "has-row-actions" CSS class (links not revealed on focus see #31003 for reference)

I can do that, but I don't understand what you mean. See https://cloudup.com/c5p3yXllT7K

  • 1st screenshot is with username as primary, edit/view show on kb focus of blog url)
  • 2nd screenshot is with blogs as primary, edit/view also show next to url, then edit link (only action for user) shows underneath the blogs list.
  • is it worth it to apply all this new stuff to Link Manager too?

Can do, so long as we're cool with the logic. I'd assume yes. Original patches I started from didn't address it, not sure why.

  • consider to update the selector in common.js

I'm not sure what that really means per se. :) Could you apply the patch, make your change, add it to the patch, and submit it, so I can see?

Thanks.

#24 in reply to: ↑ 23 @afercia
9 years ago

Replying to stephdau:

Personally, as a dev, I'd like both a generic with context as arg (as per prev patch), or one as you describe (context via dynamic hook name). I see value in both. Passing it as an arg is what @helen and I had decided for this round, but I'm not attached to either.

I'm not attached to either too :) just a bit concerned about consistency with other similar hooks noticing that they all have a dynamic hook name.

  • in /network/users.php the "Sites" column needs some special treatment, it needs the "has-row-actions" CSS class (links not revealed on focus see #31003 for reference)

I can do that, but I don't understand what you mean.

One of the side goals of this ticket would be replacing in common.js that "terrible selectors pile" as @helen named it :) with just one CSS selector. See:
https://core.trac.wordpress.org/browser/tags/4.2/src/wp-admin/js/common.js#L481
should use as selector just ".has-row-actions". Once you do that, the edit/view links in the "Sites" column won't show on keyboard focus because that column is not the primary one and doesn't have a ".has-row-actions" class.

  • consider to update the selector in common.js

I'm not sure what that really means per se.

See above :)

@stephdau
9 years ago

#25 @stephdau
9 years ago

In 25408.7.diff, I've replaced if ( $primary === $column_name ) { by if ( $primary === $column_name || 'blogs' === $column_name ) { (see L170-173 in src/wp-admin/includes/class-wp-ms-users-list-table.php in patch), which will always apply has-row-actions to the blogs/sites column.

The same update also bring the patch in line with the latest trunk.

#26 @stephdau
9 years ago

@helen: can you give us your opinion on the filter (context as argument, or context in dynamic hook name?)? See WP_List_Table::get_primary_column_name() in patch.

Links list table: one of the comments above states "The only one I didn't touch was class-wp-links-list-table.php because I don't have the links manager plugin to test it with yet, so I left that one alone.", so now I know why it wasn't previously handled. Will add to future patch.

Last edited 9 years ago by stephdau (previous) (diff)

@stephdau
9 years ago

#27 @stephdau
9 years ago

25408.8.diff adds the same logic to src/wp-admin/includes/class-wp-links-list-table.php.

Left:

  • decide how we want the filter(s) to exist in WP_List_Table::get_primary_column_name().
  • dealing with wp-admin/js/common.js if we want to do that here (I wouldn't,since it has its own ticket).

@stephdau
9 years ago

@stephdau
9 years ago

#28 @stephdau
9 years ago

25408.10.diff implements most of what @helen and I discussed on Slack:

  • also adds column-primary when applying has-row-actions, except for the blogs column in WP_MS_Users_List_Table, which needs the latter at all times, but not the former.
  • only implements a statically-named filter in WP_List_Table::get_primary_column_name(), with context as argument. Dynamically-named filters are needlessly difficult to document/use, and we don't really implement new ones.

One thing I did not do is renaming *::handle_row_actions() to *::row_actions() because they are both implemented in the parent class, are different enough (args, etc), and both are valuable as is. Opened to renaming it, just not to that.

Also switched WP_Comments_List_Table::handle_row_actions() to return and not echo, for consistency.

Last edited 9 years ago by stephdau (previous) (diff)

@stephdau
9 years ago

#29 @stephdau
9 years ago

25408.11.diff updates the previously faulty logic for the column-primary assignment in WP_MS_Users_List_Table.

This ticket was mentioned in Slack in #design by stephdau. View the logs.


9 years ago

@stephdau
9 years ago

#31 @stephdau
9 years ago

25408.12.diff merges in and fixes conflicts with r32630. Should be all set.

@stephdau
9 years ago

@stephdau
9 years ago

@stephdau
9 years ago

@stephdau
9 years ago

#32 @afercia
9 years ago

Related: #31654 and #32170

#33 @helen
9 years ago

In 32644:

List tables: introduce the concept of a "primary" column.

This becomes the column that contains the row actions, and allows for a more flexibility, particularly with custom post types and list tables. To (re)define the primary column, use the list_table_primary_column filter, which receives the column name and the screen ID as arguments.

props stephdau, DaveAl, jesin.
see #25408.

#34 follow-up: @helen
9 years ago

Leaving this open until the dust settles a bit, but woohoo!

#35 @helen
9 years ago

In 32645:

After [32644], sanity for a pile of selectors.

see #25408.

#36 in reply to: ↑ 34 @afercia
9 years ago

Replying to helen:

woohoo!

I couldn't have said it better myself :) thanks very much stephdau, DaveAl, jesin, helen.

#37 @helen
9 years ago

afercia noted that this drops some special classes for the media image/icon column, but I'd like to get rid of that as a separate column anyway, see #29993 and #32254. Leaving a note here so that if we don't drop that column (which would make #32395 more difficult, so it's unlikely we won't do it), we add those classes back.

#38 follow-up: @boonebgorges
9 years ago

Looks like we missed WP_Post_Comments_List_Table - maybe because of the way it overrides get_column_info(). See 25408.17.diff.

#39 in reply to: ↑ 38 @stephdau
9 years ago

Replying to boonebgorges:

Looks like we missed WP_Post_Comments_List_Table

I totally did.

maybe because of the way it overrides get_column_info(). See 25408.17.diff.

Nope, because I wasn't expecting a 2nd class being defined in the same file, and I never even saw it... Totally my bad. Most excellent catch!

cc @helen for quick commit (.17).

#40 @helen
9 years ago

In 32651:

Define a primary column for WP_Post_Comments_List_Table.

props boonebgorges.
see #25408.

#41 @afercia
9 years ago

Just noticed the Network Themes table cell with row-actions misses the has-row-actions column-primary CSS classes. Maybe would need same treatment given to the Plugins table, see
$extra_class = ' has-row-actions column-primary'; etc.

#42 @ocean90
9 years ago

If you click/focus one of the actions links in the Plugins table all other actions links will be hidden, see https://cloudup.com/cFt-2oOJHrt.

#43 @helen
9 years ago

Themes/plugins list tables, both single and multisite, could probably lose the has-row-actions part (there's JS that's likely what's causing @ocean90's issue), and I don't know if the primary column for those should really be allowed to be redefined. Thoughts?

#44 @DrewAPicture
9 years ago

In 32660:

Add missing return descriptions and fix formatting of inline documentation introduced in [32644].

See #25408. See #32246.

#45 @DrewAPicture
9 years ago

In 32661:

Fix formatting and add a missing return description for inline documentation introduced in [32644] for WP_Links_List_Table.

See #25408. See #32246.

#46 @DrewAPicture
9 years ago

In 32662:

Fix formatting and add missing return descriptions for inline documenation introduced in [32644] for WP_List_Table.

Also fixes an error introduced in [32661] for WP_Links_List_Table.

See #25408. See #32246.

#47 @DrewAPicture
9 years ago

In 32663:

Fix formatting and add a missing return description for inline documentation introduced in [32644] for WP_Media_List_Table.

See #25408. See #32246.

#48 @DrewAPicture
9 years ago

In 32664:

Fix formatting and add missing return descriptions for inline documentation introduced in [32644] for WP_MS_Sites_List_Table.

See #25408. See #32246.

#49 @DrewAPicture
9 years ago

In 32665:

Fix syntax and add a missing return description for inline documentation introduced in [32644] for WP_MS_Themes_List_Table.

See #25408. See #32246.

#50 @DrewAPicture
9 years ago

In 32666:

Fix formatting and add missing return descriptions for inline documentation introduced in [32644] for WP_MS_Users_List_Table.

See #25408. See #32246.

#51 @DrewAPicture
9 years ago

In 32667:

Fix formatting and add a missing return description for inline documentation introduced in [32644] for WP_Plugins_List_Table.

See #25408. See #32246.

#52 @DrewAPicture
9 years ago

In 32668:

Fix syntax and add missing return descriptions for inline documentation introduced in [32644] for WP_Posts_List_Table.

See #25408. See #32246.

#53 @DrewAPicture
9 years ago

In 32669:

Fix syntax and add missing return descriptions for inline documentation introduced in [32644] for WP_Terms_List_Table.

See #25408. See #32246.

#54 @DrewAPicture
9 years ago

In 32670:

Fix syntax and add a missing return description for inline documentation introduced in [32644] for WP_Users_List_Table.

See #25408. See #32246.

#55 @stephdau
9 years ago

WP_List_Table::get_primary_column_name() is the method with the filter.
Extending classes usually implement ::get_default_primary_column_name(), which the above uses, so their value is filtered.
For extending classes where we do not want the column to be alterable, they should simply implement ::get_primary_column_name() instead.

25408.18.diff implements the above proposed solution for:

  • plugin list table (used for both single and multisite)
  • multisite themes list (single doesn't implement listtables per se).

@stephdau
9 years ago

@helen
9 years ago

#56 @helen
9 years ago

In 32686:

List tables: tighten up primary column handling for plugins and multisite themes.

These shouldn't be able to have their primary column reassigned by default. Also removes the has-row-actions class as these list tables always have row actions visible and the JS for visual toggling conflicts. The column-primary class remains.

props stephdau.
see #25408.

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


9 years ago

#58 follow-ups: @obenland
9 years ago

  • Keywords 4.1-early removed

Custom row attributes like here are no longer working, after [32644].

#59 in reply to: ↑ 58 @jeherve
9 years ago

Replying to obenland:

Custom row attributes like here are no longer working, after [32644].

[32644] also seems to create notices with plugins like Jetpack, that build on top of WP_List_Table:

Notice: Undefined offset: 3 in /wp-admin/includes/class-wp-list-table.php on line 1120

I'll keep an eye on this ticket to see if we have to make any changes on our end.

#60 @afercia
9 years ago

Just realized the latest comments in the Dashboard screen have row-actions but they're not revealed on focus. Previously, they were targeted with .dashboard-comment-wrap now removed in r32645. They would need the has-row-actions class (removing "td" in the jQuery selector) or to be targeted with a specific selector.

#61 in reply to: ↑ 58 @stephdau
9 years ago

Replying to obenland:

Custom row attributes like here are no longer working, after [32644].

I think that is the only one that was missed. See 25408-users-list-table.diff

The only other one I recall is in the posts list, where the title column-title column also needs page-title assigned.

Last edited 9 years ago by stephdau (previous) (diff)

#62 follow-up: @georgestephanis
9 years ago

Re @jeherve's remark above, yeah --

The changed line in WP_List_Table::single_row_columns from

list( $columns, $hidden ) = $this->get_column_info();

to

list( $columns, $hidden, $sortable, $primary ) = $this->get_column_info();

is causing a notice (not an error) if get_column_info() doesn't return four indexes in the array.

I suspect it's the initial failout clause in get_column_info() that is causing it --

if ( isset( $this->_column_headers ) )
	return $this->_column_headers;

I'd suggest that for backcompat, if $this->_column_headers doesn't contain four array items, we add on a fourth with a value of $this->get_primary_column_name()

Unless there's objections, I'll try to get a patch inbound.

@stephdau
9 years ago

An alternative to 25408-list-table-headers-test.diff, if we wanted to future-proof things a bit.

#64 @georgestephanis
9 years ago

I've not tested but it should do what I was thinking.

25408-list-table-headers-test.diff has problems, as it will fail out and ignore the custom ones, so any plugins that don't update to expect the new stuff will suddenly have their columns borked.

#65 @stephdau
9 years ago

George's solution is friendlier to devs who don't keep up with things than mine, go with that.

#66 @helen
9 years ago

In 32717:

List tables:

  • Avoid notices in custom list tables that manually set $_column_headers. Any plugins using this for a specific purpose should update.
  • Restore a special class name in the users list table.

props georgestephanis, stephdau.
see #25408.

@kovshenin
9 years ago

#67 follow-up: @kovshenin
9 years ago

I think r32717 will break column headers for all plugins that explicitly set $_column_headers, because up until r32644 only three items were expected from get_column_info(), but now we check for at least four and skip the explicitly set headers entirely if that's not the case.

25408.20.diff makes sure the function passes an array of at least four items, with null as primary. Too many arrays? You bet.

#68 in reply to: ↑ 67 ; follow-up: @ocean90
9 years ago

Replying to kovshenin:

25408.20.diff makes sure the function passes an array of at least four items, with null as primary. Too many arrays? You bet.

array_replace() is PHP 5.3+ and can't be used.

@kovshenin
9 years ago

#69 in reply to: ↑ 68 @kovshenin
9 years ago

Added 25408.21.diff, revised for php 3.0.18 compatibility.

#70 @helen
9 years ago

In 32721:

List tables: Consolidate <td> output for posts.

see #25408.

#71 @helen
9 years ago

In 32722:

List tables: Better primary column back-compat.

Why are people manually setting $_column_headers other than because somebody else told them to? Maybe time will tell.

props kovshenin.
see #25408.

#72 @mordauk
9 years ago

The latest few commits appear to have completely broken custom list tables for me. The table doesn't render at all.
http://i.imgur.com/CNOD6Kk.png

#73 @mordauk
9 years ago

It's failing because of count( $this->_column_headers ) >= 4.

There are only three header items in this particular list table:

$columns = $this->get_columns();

$hidden = array();

$sortable = $this->get_sortable_columns();

$this->_column_headers = array( $columns, $hidden, $sortable );

If $this->_column_headers = array( $columns, $hidden, $sortable ); is changed to $this->_column_headers = array( $columns, $hidden, $sortable, array() );, the table shows properly.

Seems the primary column isn't properly getting set.

#74 @mordauk
9 years ago

Further note: it still fails even when not manually setting $this->_column_headers

#75 @helen
9 years ago

In 32723:

List tables:

Since the primary column is not going to be alterable for plugins and MS themes, we don't need to check in on it.

see #25408.

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


9 years ago

#77 @afercia
9 years ago

Just noticed a very minor, few pixel, difference in the post table rows height compared to 4.2. The "title" cell used to have a post-title CSS class, was:

$attributes = 'class="post-title page-title column-title"' . $style;

now post-title is gone and in /wp-admin/css/edit.css the following selector doesn't apply anymore:

td.post-title strong,
td.plugin-title strong {
	display: block;
	margin-bottom: .2em;
	font-size: 14px;
}

I'd say either re-apply the rule with an updated selector or remove td.post-title strong from the CSS.

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


9 years ago

#79 @helen
9 years ago

  • Type changed from enhancement to task (blessed)

Still tweaking based on fall-out, including stuff discovered in work on #32395, so marking this as a task.

@mordauk
9 years ago

#80 @mordauk
9 years ago

For plugins that have extended the list table class and have defined _column_headers manually, a default primary column has to be "discovered" if none is specified, otherwise the mobile view is empty (for at least most tables)

In 25408-22.diff I've set it so the first non-checkbox column gets set as the primary if none is explicitly set.

Before:
https://cldup.com/tmGlgoqZPu-3000x3000.png

After:
https://cldup.com/DGFK2t2eiT-3000x3000.png

@helen
9 years ago

#81 @helen
9 years ago

25408.22.diff is just an alternative, putting it into get_default_primary_column_name() instead. Too tired to fully engage in testing and committing this right now, would appreciate more eyes.

Last edited 9 years ago by helen (previous) (diff)

This ticket was mentioned in Slack in #design by helen. View the logs.


9 years ago

#83 @stephdau
9 years ago

I think 25408.22.diff is great as is:

  • I think having it in the default (parent's) get_default_primary_column_name() is definitely the proper place to.
  • Going with a foreach, continue, and break pattern is a smart move there.

I fail to see a case where that would fail, at the very least as ungracefully as it does without the change.

Last edited 9 years ago by stephdau (previous) (diff)

#84 @helen
9 years ago

In 33104:

List tables: Add a fallback for the primary column.

Without a primary column defined, nothing shows in the responsive view at all, which is bad.

props mordauk.
see #25408.

#85 follow-up: @mordauk
9 years ago

Sorry @helen, forgot to come back to this. Happy to follow up with more tweaks if any issues are discovered.

#86 @helen
9 years ago

In 33105:

List tables: Ensure special CSS for the title column gets applied.

This could have some side effects if a custom list table has a title column with a strong element inside that is not the post title, but that is fairly edge and we can address that if it comes up. Also moves the rules into list-tables.css.

see #25408.

#87 @helen
9 years ago

In 33106:

Show row actions on focus for the dashboard comment list.

see #25408.

#88 in reply to: ↑ 85 @helen
9 years ago

Replying to mordauk:

Sorry @helen, forgot to come back to this. Happy to follow up with more tweaks if any issues are discovered.

All good, I think we're set. You'll probably want to check on some responsive CSS for your custom list tables, but otherwise, thanks for your help!

#89 @helen
9 years ago

  • Keywords has-patch needs-testing removed

Trying to go through all the comments above - the one outstanding issue I saw was about custom attributes for a table cell, which I think we could split into another ticket and not necessarily have to fix for 4.3. Anything else before calling this fixed?

#90 @stephdau
9 years ago

I think it's pretty done. :)
New tickets, as needed, make sense.

#91 @helen
9 years ago

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

#92 @rachelbaker
9 years ago

#32612 was marked as a duplicate.

#93 @BdN3504
9 years ago

Where is the documentation on how to use this? I installed 4.3 a couple of days ago and now am having trouble big time with a custom post type that does not make use of the post title. How do I mark a column as primary? To make things simple, I'd like to be able to pass this information in the manage_edit-{cpt}_columns filter, but currently there does not seem to be a way to do it like that. Also, all other columns still are displayed, but the data from the columns gets pulled into the first column. It plainly looks weird & I have no reference how to correct it. WITTFM (Where is the f*#king manual)? Take a look at this screenshot:
http://i.imgur.com/aMAPSqq.png
Which looks perfectly fine on the big screen:
http://i.imgur.com/uj68vT6.png


Found the documentation. Please include that link in the description of this patch!

Last edited 9 years ago by BdN3504 (previous) (diff)

#94 @andddd
8 years ago

I installed WordPress 4.3 couple of days ago too and found that this feature ruined all of my tables. Because the data I display cannot be deduced to single field.

Please, before you introduce any magic feature, consider having a feature flag. Enable it for your own stuff and let others to slowly adapt.

The worst of all that there is no sane way to disable this and now there is some magic CSS that breaks ::before for table rows. It's not like somebody bothered to introduce a class for wp-list-table to turn that on/off.

Last edited 8 years ago by andddd (previous) (diff)

#95 @johnjamesjacoby
7 years ago

I've tried a few different CSS approaches and also wasn't able to come up with simple styling rules to address the custom primary-column support that's built in here.

That issue probably should have a dedicated ticket opened, referring back to this one as the source.

Note: See TracTickets for help on using tickets.