Make WordPress Core

Opened 13 years ago

Closed 3 years ago

Last modified 3 years ago

#19278 closed enhancement (fixed)

Allow WP_List_Table ::get_bulk_items() to receive a nested array and output optgroups

Reported by: goldenapples's profile goldenapples Owned by: johnbillion's profile johnbillion
Milestone: 5.6 Priority: normal
Severity: normal Version:
Component: Administration Keywords: has-patch commit has-unit-tests dev-reviewed
Focuses: ui, administration Cc:

Description

I'm just putting this out there as a possible enhancement to the WP_List_Table... would like feedback as to whether anyone else would find it helpful. If so, I'm more than happy to write up a patch for this.

I think it may be useful to be able to pass a nested array to WP_List_Table::get_bulk_actions() and have it output the options within the subarray in an <optgroup>. My specific use case is in applying meta fields to a custom table. I would like to be able to define something like this in my get_bulk_items function:

function get_bulk_actions() {
	$actions = array(
		'delete' => 'Delete',
		'outofstock' => 'Mark out of stock',
		'applytags' => array(
			'label' => 'Apply tags to products',
			'actions' => array(
				'featured' => 'Featured',
				'sale' => 'On Sale'
				)
			)
		);
	return $actions;
}

and have the output look something like this:

<select name="action">
	<option value='-1' selected='selected'>Bulk Actions</option>

	<option value='delete'>Delete</option>
	<option value='outofstock'>Mark out of stock</option>
	<optgroup label="Apply tags to products">
		<option value='feature'>Featured</option>
		<option value='sale'>On sale</option>
	</optgroup>
</select>

A very minor feature, and possibly too fringe of a use case to bother with. But I've worked on a couple projects now where being able to specify markup like that would have made that screen more user-friendly. Any thoughts?

Attachments (4)

19278.diff (1.2 KB) - added by mattkeys 6 years ago.
Screen Shot 2020-02-14 at 7.37.01 PM.png (55.2 KB) - added by valentinbora 4 years ago.
Screenshot with 19278.diff applied
19278-grouped-bulk-actions-test-posts.jpg (31.9 KB) - added by davidbaumwald 4 years ago.
Test with PR #567 applied
19278.2.diff (749 bytes) - added by SergeyBiryukov 4 years ago.

Download all attachments as: .zip

Change History (33)

#1 @goldenapples
13 years ago

  • Component changed from General to Administration

#2 @scribu
13 years ago

  • Type changed from feature request to enhancement

Well, officially, you're not even able to add new bulk actions. #16031

It's unlikely this will get any attention until the above is handled.

#3 @goldenapples
13 years ago

Ha! I didn't even see that ticket, and I was wondering why I had to manually trigger $this->process_bulk_actions() in order to get them to work. Thanks... I'll take a look at that ticket.

#4 @chriscct7
10 years ago

  • Keywords needs-patch added

#5 @chriscct7
8 years ago

  • Severity changed from minor to normal

@mattkeys
6 years ago

#6 @mattkeys
6 years ago

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

Going to play necromancer here a bit and submit a patch for this. I work with WP List Table often and I know many other developers use it in their plugins/themes (despite the codex warning). Optgroup's would be a nice way to organize bulk actions.

The patch I submitted just checks if the current value from the existing bulk actions foreach is an array. If it is, the key becomes the optgroup label, and a new foreach is used to loop over the optgroup's children. Otherwise it proceeds as usual. I also modified a couple variable names to semantic reasons.

I would love to hear back from others about any unforeseen impacts that this change might have.

#7 @valentinbora
4 years ago

  • Focuses ui administration added
  • Milestone changed from Awaiting Review to Future Release
  • Owner set to valentinbora
  • Status changed from new to accepted

@goldenapples thank you for reporting this. I know it's been a (long) while, but this still sounds like a great addition.

Also, thank you @mattkeys for the patch. I'll check it out and get back with updates.

#8 @valentinbora
4 years ago

  • Milestone changed from Future Release to 5.5

Happy to report that it works mighty fine onto [47289]. The patch applies cleanly and works. Here's a code sample:

<?php

add_filter( 'bulk_actions-edit-post', 'register_my_bulk_actions' );

function register_my_bulk_actions( $bulk_actions ) {
        $bulk_actions['Grouped actions'] = [
                'hello' => 'Hello',
                'world' => 'World'
        ];

        return $bulk_actions;
}

Looking forward to more feedback from others regarding the addition.

#9 @mattkeys
4 years ago

Thanks for testing, excited to see this get some traction.

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


4 years ago

#11 @davidbaumwald
4 years ago

  • Milestone changed from 5.5 to Future Release
  • Owner valentinbora deleted
  • Status changed from accepted to assigned

Nothing stops multiple people from working on the same ticket at the same time. But I'm removing the ticket owner to make it clear no one is actively working on this one(the current owner has not participated in a long period of time).

Also, with Beta 1 landing tomorrow, this ticket is being moved to Future Release. If any maintainer or committer feels the remainder of this ticket can be resolved in time, or wishes to assume ownership during a specific cycle, feel free to update the milestone accordingly.

#12 @SergeyBiryukov
4 years ago

  • Milestone changed from Future Release to 5.6

This ticket was mentioned in Slack in #core-editor by talldanwp. View the logs.


4 years ago

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


4 years ago

#15 @francina
4 years ago

  • Keywords needs-refresh added; needs-testing removed

This ticket was mentioned in PR #567 on WordPress/wordpress-develop by dream-encode.


4 years ago
#16

  • Keywords needs-refresh removed

Allow WP_List_Table::get_bulk_items to accept a nested array of actions, outputting them as optgroups.

Trac ticket: https://core.trac.wordpress.org/ticket/19278

#17 @davidbaumwald
4 years ago

Refreshed the patch and tested to make sure this still works using code similar to https://core.trac.wordpress.org/ticket/19278#comment:8. Screenshot to follow.

#18 @johnbillion
4 years ago

  • Keywords commit added; dev-feedback removed

@davidbaumwald I updated your PR with two improvements:

  • Escape the attributes
  • Update the documentation

I can think of several use cases for this feature. The change looks good to go.

#19 @johnbillion
4 years ago

  • Owner set to johnbillion
  • Status changed from assigned to reviewing

#20 @johnbillion
4 years ago

  • Keywords has-unit-tests added

#21 @johnbillion
4 years ago

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

In 49190:

Administration: Allow WP_List_Table::get_bulk_items() to receive a nested array in order to output optgroups.

The allowed format for bulk actions is now an associative array where each element represents either a top level option value and label, or an array representing an optgroup and its options.

For a standard option, the array element key is the field value and the array element value is the field label.

For an optgroup, the array element key is the label and the array element value is an associative array of options as above.

Props goldenapples, mattkeys, valentinbora, davidbaumwald

Fixes #19278

#22 @SergeyBiryukov
4 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

The test added in [49190] fails on Windows, because the heredoc syntax (<<<) uses platform-specific EOLs, however WP_List_Table::get_bulk_items() specifically uses \n.

19278.2.diff fixes the failure.

In the future, this would probably benefit from something like assertContainsIgnoreEOL(), similar to assertEqualsIgnoreEOL() added in [46612] / #31432.

However, since PHPUnit 7.5+ deprecates using assertContains() for strings and introduces assertStringContainsString() instead, let's skip adding a new method for now and just fix the test.

#23 @SergeyBiryukov
4 years ago

In 49691:

Tests: Ignore EOL differences in WP_List_Table::get_bulk_items() test for optgroups.

This avoids a misleading failure due to Unix vs. Windows EOL style mismatches and allows the test to pass on Windows.

Follow-up to [46612], [49190].

See #19278.

#24 @SergeyBiryukov
4 years ago

  • Keywords dev-feedback added

Marking [49691] for backporting to the 5.6 branch after a second committer's review.

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


3 years ago

This ticket was mentioned in Slack in #core-committers by sergey. View the logs.


3 years ago

#27 @desrosj
3 years ago

  • Keywords dev-reviewed added; dev-feedback removed

[49691] looks good to backport to 5.6.

#28 @SergeyBiryukov
3 years ago

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

In 49730:

Tests: Ignore EOL differences in WP_List_Table::get_bulk_items() test for optgroups.

This avoids a misleading failure due to Unix vs. Windows EOL style mismatches and allows the test to pass on Windows.

Follow-up to [46612], [49190].

Reviewed by desrosj, SergeyBiryukov.
Merges [49691] to the 5.6 branch.
Fixes #19278.

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


3 years ago

Note: See TracTickets for help on using tickets.