WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#32757 closed task (blessed) (fixed)

Press This: use a split button for Save | Publish | Preview

Reported by: azaozz Owned by: azaozz
Milestone: 4.3 Priority: normal
Severity: normal Version:
Component: Press This Keywords:
Focuses: accessibility Cc:

Description

Would be nicer than the current buttons and can have an "(Open in) Standard Editor" button too.

Attachments (9)

32757.patch (6.7 KB) - added by azaozz 4 years ago.
paste-1428249324133.png (10.0 KB) - added by azaozz 4 years ago.
Mockup from @michael-arestad
32920_before.png (11.8 KB) - added by DrewAPicture 4 years ago.
Before [32920]
32920_after_closed.png (9.8 KB) - added by DrewAPicture 4 years ago.
After [32920] (default state)
32920_after_open.png (16.2 KB) - added by DrewAPicture 4 years ago.
After [32920] (open)
32757-unordered-list.patch (1.8 KB) - added by afercia 4 years ago.
image.jpg (135.5 KB) - added by ryan 4 years ago.
Split button on an iPhone 6+
split-button-hover.png (8.3 KB) - added by samuelsidler 4 years ago.
split-button.png (12.7 KB) - added by azaozz 4 years ago.
In Firefox, IE11 and Chrome on Win7

Download all attachments as: .zip

Change History (42)

@azaozz
4 years ago

@azaozz
4 years ago

Mockup from @michael-arestad

#1 @azaozz
4 years ago

In 32757.patch: first run. Needs some CSS refinement. Only tested in Chrome for now.

Last edited 4 years ago by azaozz (previous) (diff)

#2 @kraftbj
4 years ago

Tested in FF Dev (40.0a2) and worked well. Falling into the CSS refinement, the up arrow feels off-centered (high) to me: https://cloudup.com/cSZ0h7ZFMdS

#3 @stephdau
4 years ago

Super excited to see this starting to happen!

Comments:

  • @helen mentioned the up arrow feels kinda weird because it's so ingrained to have them pointing down, leading to a dropDOWN. I have no beef with it, personally.
  • probably a symptom of how early the patch is, but one can only close the list of available options by clicking on the arrow again (can't just click outside of it, as we usually do for menus).
  • likely the same: if I click on the arrow to open the menu, then click on it again to close it, the arrow stays underlined (link). Not sure of it should or not.

Besides that, it definitely cement how cool a new UX that'd be, WP wide.

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

#4 @azaozz
4 years ago

In 32920:

Press This: convert the Preview | Save | Publish buttons to a split button.
See #32757.

#5 @azaozz
4 years ago

  • Focuses accessibility added

All the above bugs/concerns should be fixed in [32920]. Tested in Firefox, Chrome IE(8-11), iOS Safari (iPad), Android 5.1.

There is still some room for improvement, especially the hover, focus, active styles. Currently these match the primary buttons, but probably can be better for the split button. Also needs some accessibility testing.

Couple more things to consider:

  • Do we want to "switch" the visible button on selecting a "sub-button"? I.e. when the user clicks the arrow and selects "Preview", the visible button will change from Publish to Preview (until the user clicks the arrow again and selects something else).
  • Do we want to switch the arrows up/down when the sub-buttons are open/closed? I.e. have an up arrow when closed (pointing that it opens upwards), changing to a down arrow when open? This is a bit non-standard but may make sense.

#6 @azaozz
4 years ago

In 32921:

Press This: ensure proper tab order in the split button. Tweak padding and border color a bit.
See #32757.

#7 @azaozz
4 years ago

In 32922:

Press This: fix spinner position relative to the split button.
See #32757.

@DrewAPicture
4 years ago

Before [32920]

#8 @DrewAPicture
4 years ago

Added before and after [32920] vizrecs, for posterity.

@DrewAPicture
4 years ago

After [32920] (default state)

@DrewAPicture
4 years ago

After [32920] (open)

#9 @afercia
4 years ago

Nice to see this is happening :) Tested a bit with Firefox+NVDA and all is announced nicely. Compared with the Bootstrap implementation, see http://getbootstrap.com/components/#btn-dropdowns-split, there are a couple of things that could be improved a bit.

  • the actions in the dropdown should be marked up as an unordered list, this way when navigating from the toggle button to the list, users would be informed about the number of actions available: "List with nn items"
  • pressing Escape should close the dropdown and move focus back to the toggle button
  • when the dropdown is open maybe the toggle button should be visually displayed as if it was "pressed"

Please notice Bootstrap also implements full arrows navigation, to emulate the native <select> behavior: you can open the dropdown pressing the down arrow on your keyboard and navigate through the items with the up/down arrow keys. Also, arrows navigation is constrained inside the dropdown even if the current implementation doesn't work when using a screen reader, would need a role=application to actually work. By the way I'm not sure we should try to implement arrows navigation, maybe better to keep it simple for now :)

#10 @afercia
4 years ago

32757-unordered-list.patch uses an unordered list for the dropdown

#11 follow-up: @afercia
4 years ago

Forgot one thing, maybe the screen reader text "Toggle dropdown" should use a more descriptive text. In this case maybe "More actions" would be more appropriate than a generic "Toggle dropdown" which doesn't give any clue about the dropdown content and available actions.

#12 @azaozz
4 years ago

In 32934:

Press This, split button: wrap the sub-buttons in an unordered list to aid accessibility.
Props afercia. See #32757.

#13 in reply to: ↑ 11 ; follow-up: @azaozz
4 years ago

Replying to afercia:

Sure, is "More actions" good enough or perhaps something that also suggests the button is a toggle?

Agreed that we don't need arrow navigation there. This is not a native <select> and is not a menu. Better to behave as closely as possible to the original, i.e. several buttons in a row.

Last edited 4 years ago by azaozz (previous) (diff)

#14 in reply to: ↑ 13 @afercia
4 years ago

Replying to azaozz:

or perhaps something that also suggests the button is a toggle?

Thanks to aria-haspopup and aria-expanded screen readers already announce there's a submenu that can be expanded, for example in Firefox+NVDA:

Publish
button

Toggle dropdown
menu button  collapsed  subMenu

#15 @azaozz
4 years ago

In 32944:

Press This: better screen reader text for the arrow part of the split button.
Props afercia. See #32757.

@ryan
4 years ago

Split button on an iPhone 6+

#16 @ryan
4 years ago

The menu doesn't dismiss with outside taps on iOS. Tapping in the bottom bar, for example, doesn't dismiss the menu.

This ticket was mentioned in Slack in #core-flow by boren. View the logs.


4 years ago

#18 @azaozz
4 years ago

In 32966:

Press This: fix iOS bug that prevents closing of the split button when tapping outside of it.
See #32757.

#19 follow-up: @kraftbj
4 years ago

When trying to go straight from PT clicked to the standard editor, nothing is brought over. If changes are made, an AYS is displayed when going to the standard editor.

In both cases, I'd expect a draft to save and take you to that post in the standard editor.

Steps to repo:

  1. From a parent window, click the PT bookmarklet

2a. Make no changes (leaving the prefilled content). Click the split button arrow and choose Standard Editor

Result: New window, standard editor with no content. Expected: Standard editor with content

2b. Make changes. Click the split button arrow and choose Standard Editor.

Result: AYS prompt (Changes made will be lost). If you click "Leave This Page", it dismisses the prompt and closes the PT window. No other action.

Last edited 4 years ago by kraftbj (previous) (diff)

#20 @azaozz
4 years ago

In 32983:

Press This:

  • Save a draft before opening the standard editor window.
  • While saving a post show Saving... instead of Publish for the main visible part of the split button (same as the Save Draft button before).

See #32757.

#21 in reply to: ↑ 19 @azaozz
4 years ago

Replying to kraftbj:

Yeah, was testing different ways to open the Edit Post page in the main window.

Not particularly happy with [32983]. It takes 3x the network latency time to work: send ajax request, receive answer, load the Edit Post page in the main window.

The other way this can work is to simply submit the post form, same as when doing preview. This is faster as the form submit open a new tab in the main browser window (one network trip less). However if there is an error when saving, we can't get the data back as the PT popup is already closed. We probably can leave it open, but... Not sure how that's going to look in the 99.99% of the cases when there are no errors.

Also moved the "Saving..." message to show in the "Publish" button (the main part of the split button). Before it was on the Save Draft button.

Still needs doing:

  • Maybe better styles for "selected" state of the split button.
  • Need "disabled" state styling for all buttons.

#22 @netweb
4 years ago

Running grunt jshint or grunt precommit is currently erroring via the r32983 changeset:

Running "jshint:core" (jshint) task

   src/wp-admin/js/press-this.js
    196 |                            }, 200 )
                                             ^ Missing semicolon.

>> 1 error in 83 files
Warning: Task "jshint:core" failed. Use --force to continue.

Aborted due to warnings.

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


4 years ago

#24 @jorbin
4 years ago

In 32986:

Add missing semicolon

Introduced in [32983].

see #32757.

#25 follow-up: @samuelsidler
4 years ago

The lack of shadow when hovering the split button is rather disturbing (see screenshot above). Either we should remove the shadow everywhere or keep it when hovering the split button.

#26 in reply to: ↑ 25 @azaozz
4 years ago

Replying to samuelsidler:

Yep, the buttons also need better styles for "selected" and "disabled" state. Hoping @michael-arestad will get some time soon to have a look :)

Last edited 4 years ago by azaozz (previous) (diff)

#27 follow-up: @kraftbj
4 years ago

In Chrome, there is a misalignment along the bottom between the Publish and the arrow dashicon: https://cloudup.com/c5dsJdI2med

Either reducing the Publish font-size down to 13px or bumping up the dashicon to height 20, font-size 20px evens it up, though the latter makes the arrow look a bit big to me: https://cloudup.com/cM-_UE_AhS8

@azaozz
4 years ago

In Firefox, IE11 and Chrome on Win7

#28 in reply to: ↑ 27 @azaozz
4 years ago

Replying to kraftbj:

In Chrome, there is a misalignment...

The button looks pretty good here in all common browsers. Might be related to the different font? Will test more, and in Safari.

Edit: ugh, dashicons' size shouldn't be overridden (should be 20px). This is what I have:

  ...
  width: 20px;
  height: 20px;
  font-size: 20px;
  line-height: 1;
Last edited 4 years ago by azaozz (previous) (diff)

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


4 years ago

#30 @obenland
4 years ago

  • Keywords needs-patch added
  • Owner set to azaozz
  • Status changed from new to assigned
  • Type changed from enhancement to task (blessed)

#31 @obenland
4 years ago

@azaozz, do you think you could get this over the finish line before beta3?

#32 @azaozz
4 years ago

  • Keywords needs-patch removed

This is pretty much done. The only thing that needs fixing is the missing shadow on hover: split-button-hover.png. Will patch that in a min.

Was hoping for better looking style for :focus, but maybe better to keep it the same as other buttons.

Last edited 4 years ago by azaozz (previous) (diff)

#33 @azaozz
4 years ago

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

In 33156:

Press THis: do not change the split button box-shadow on hover.
Fixes #32757.

Note: See TracTickets for help on using tickets.