WordPress.org

Make WordPress Core

Opened 6 months ago

Closed 4 weeks ago

#46757 closed defect (bug) (fixed)

Media Trash: The Bulk Media options when in the Trash shouldn't provide two primary buttons

Reported by: garrett-eclipse Owned by: afercia
Milestone: 5.2.3 Priority: normal
Severity: normal Version:
Component: Media Keywords: has-screenshots has-patch commit fixed-major needs-testing
Focuses: ui, accessibility, administration Cc:

Description

Hello,

This was raised by @karmastosed in the #design slack here - https://wordpress.slack.com/archives/C02S78ZAL/p1554138094057000

The question;

Can I ask the thinking in having these 2 primary?

Screen coming, but in the Media Trash when you do bulk actions you're presented with three buttons two of which are 'primary'.

This was raised in design review of #44675.

Cheers

Attachments (5)

2019-04-01 at 18.01.jpg (18.5 KB) - added by garrett-eclipse 6 months ago.
Screenshot illustrating the dual primary buttons
Screen Shot 2019-04-01 at 10.21.51 PM (1).png (89.8 KB) - added by garrett-eclipse 6 months ago.
Updated screen as buttons are changing in #44675
01 media trash buttons.png (141.5 KB) - added by afercia 3 months ago.
Proposed new buttons position and style
46757.diff (2.0 KB) - added by afercia 3 months ago.
46757.2.diff (2.0 KB) - added by garrett-eclipse 7 weeks ago.
Minor refresh to address line number changes (no code change)

Download all attachments as: .zip

Change History (27)

@garrett-eclipse
6 months ago

Screenshot illustrating the dual primary buttons

#2 @ianbelanger
6 months ago

  • Keywords needs-design-feedback added

Is there any consensus on which button should be changed and what button class it should be changed to? I did not see any suggestions in the slack discussion referenced above. It seems that the WordPress admin only has button-primary and button-secondary so our options are limited without adding a new class, which is probably not warranted.

#4 @ianbelanger
6 months ago

  • Keywords has-screenshots added

@garrett-eclipse
6 months ago

Updated screen as buttons are changing in #44675

#5 @garrett-eclipse
6 months ago

Hi @ianbelanger, there wasn't much discussion aside from the initial note there so will leave for design feedback. I just wanted to updated with the latest screen from #44675.

I see what you mean with the classes, I would almost suggest a red button class be introduced specifically for 'Delete Permanently' and potentially even the 'Move to Trash' button as their link counterparts throughout actions like the Row Actions and Publishing Actions all have the link in red as it should give the user pause and indication that they're doing a removal action.

I'll leave it to #design to indicate if warranted but IMHO I would feel that a red button would make sense here.

Cheers

#6 @afercia
5 months ago

I'd tend to think none of them should be primary. "Restore" and "Delete permanently" are of the same importance and definitely depend on the user intent. In this specific case, I'm not sure WordPress should try to predict what the user flow is and consequently identify a primary action.

I'd consider to make them normal buttons.

Aside: in the Posts trash, the "Empty Trash" button is not primary: the comparison may not fully apply, as "Empty Trash" is the only button there, but worth considering for consistency.

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


5 months ago

#8 @afercia
3 months ago

  • Focuses accessibility added
  • Milestone changed from Awaiting Review to 5.3
  • Owner set to afercia
  • Status changed from new to assigned

As in this release cycle there's some focus on improvements for the media views, due also to the WPCampus accessibility report, I'd like to propose this ticket for 5.3 consideration together with #46758.

#9 @afercia
3 months ago

suggest a red button class

Worth considering the alternate color schemes available under "Your Profile". For example, the "Midnight" color scheme makes all the buttons red. New delete red buttons would be hard to distinguish from normal buttons.

The current pattern for controls associated with destructive action in WordPress is red underlined text, whether the underlying HTML element is a link or a button. While this may be not 100% ideal, because it's unclear if the control is a link or a button, I'd like to propose to follow the current pattern. There's already a button-link-delete CSS class that can be used for this purpose.

These buttons should also be reordered, as pointed out in #46758. The primary action button should be on the left, and the destructive action button should be clearly distinguishable. Worth also reminding that for accessibility, all related controls should be visually grouped together. See #40822.

Considering the above, I've quickly tried a few simple changes. See the screenshot below, which tries to illustrate all the possible states of these buttons.

@afercia
3 months ago

Proposed new buttons position and style

@afercia
3 months ago

#10 @afercia
3 months ago

  • Keywords has-patch added

46757.diff implements the changes above, fixing also #46758. It also fixes a minor bug where the disabled state was not set correctly when switching between the select and edit modes.

Some design feedback and testing would be nice :) Reminder: to test the metia Trash, set

define( 'MEDIA_TRASH', true );

in your wp-config.php file.

#11 @ianbelanger
3 months ago

I tested 46757.diff and all seems to work as intended. All buttons are arranged, with the primary button first. All we need is design feedback now.

#12 @garrett-eclipse
3 months ago

Thanks @afercia I appreciate moving this forward and the patch. It works nicely for me so will second @ianbelanger there and aside from a final design say would be happy to see this in place.

Sidenote: This is also resolving #46758, appreciate the double-tap. Should I close the other ticket or wait for this to commit and then tie them together?

#13 @afercia
3 months ago

Should I close the other ticket or wait for this to commit and then tie them together?

Both tickets can be closed during commit.

#14 follow-up: @karmatosed
7 weeks ago

  • Keywords needs-design-feedback removed

The proposed changes seem good to me, thank you, everyone. I am going to remove the design feedback keyword to hopefully leave this open now to get into 5.3.

@garrett-eclipse
7 weeks ago

Minor refresh to address line number changes (no code change)

#15 in reply to: ↑ 14 @garrett-eclipse
7 weeks ago

  • Keywords commit added

Replying to karmatosed:

The proposed changes seem good to me, thank you, everyone. I am going to remove the design feedback keyword to hopefully leave this open now to get into 5.3.

Thanks @karmatosed I appreciate the review. Retesting myself everything is in order and working properly, unit tests are all passing. I just uploaded 46757.2.diff as a refresh to address the minor change of line numbers (no code change).

@afercia marking this for commit, please also close out #46758 during commit.

#16 @karmatosed
7 weeks ago

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

In 45701:

Media: Reduces the bulk media options to have one primary button

This fixes where 2 primary buttons were showing for bulk actions within media trash.

Props garrett-eclipse, afercia, ianbelanger, SergeyBiryukov
Fixes #46757, #46758

#17 @joemcgill
5 weeks ago

  • Keywords fixed-major added
  • Milestone changed from 5.3 to 5.2.3
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening this so it can be back-ported to the 5.2 branch.

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


5 weeks ago

#19 @audrasjb
5 weeks ago

  • Keywords needs-testing added

Adding needs-testing keyword to validate if this ticket is good to land in 5.2.3 (should be ok though).

#20 @audrasjb
5 weeks ago

Tested on my side against 5.2.2 and the patch works fine.

However, unless I'm mistaken [45701] doesn't look to apply cleanly against 5.2.2: some files seem to have dependancies to previous commits.

#21 @JeffPaul
5 weeks ago

@garrett-eclipse @karmatosed @joemcgill any insight into which other commits this may have built off of?

#22 @SergeyBiryukov
4 weeks ago

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

In 45841:

Media: Reduces the bulk media options to have one primary button.

This fixes where 2 primary buttons were showing for bulk actions within media trash.

Props garrett-eclipse, afercia, ianbelanger, SergeyBiryukov, karmatosed.
Merges [45701] to the 5.2 branch.
Fixes #46757, #46758

Note: See TracTickets for help on using tickets.