New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Chrome: Update the design of the ellipsis menu #3456

Merged
merged 6 commits into from Nov 20, 2017

Conversation

Projects
None yet
6 participants
@youknowriad
Contributor

youknowriad commented Nov 13, 2017

related to #3311 (comment)
I don't know if there's an issue for this.

This PR updates the design of the ellipsis menu to match the proposed design.

screen shot 2017-11-13 at 15 01 16

@youknowriad youknowriad self-assigned this Nov 13, 2017

@youknowriad youknowriad requested a review from jasmussen Nov 13, 2017

@jasmussen

This comment has been minimized.

Show comment
Hide comment
@jasmussen

jasmussen Nov 13, 2017

Contributor

I think this is a huge improvement. It's not exactly what I had in mind in most recent designs, where I was imagining the items not having any icons, just a blank space, and the selected one had a checkmark:

screen shot 2017-11-13 at 15 09 18

But that example above is not ideal either, because some of those plugins, like analyze readability, are also toggles. Which means that mockup has two patterns, 1 pattern which shows two options and a checkmark next to the chosen one in the group, and 1 pattern which shows a mix of direct links and "toggle links" where the toggle link simply gets a checkmark.

One idea is to have a menu like this:

-------------------------------
|     Edit HTML               |
-------------------------------
|     Fix to block            |
-------------------------------
|     Analyze Readability     |
|     Configure Publicize     |
-------------------------------

Options 1 2 and 3 above are "toggles", and you could then end up with something like this:

-------------------------------
|  ✓  Edit HTML               |
-------------------------------
|     Fix to block            |
-------------------------------
|  ✓  Analyze Readability     |
|     Configure Publicize     |
-------------------------------

I also think not requiring an icon would make it easier to add items to the menu.

But — I'm curious if mergint the two HTML/Visual mode switches into a single toggle is a good idea or not. I'm also not sure my ideas above should hold this PR from getting merged. Curious about your thoughts, @karmatosed

Contributor

jasmussen commented Nov 13, 2017

I think this is a huge improvement. It's not exactly what I had in mind in most recent designs, where I was imagining the items not having any icons, just a blank space, and the selected one had a checkmark:

screen shot 2017-11-13 at 15 09 18

But that example above is not ideal either, because some of those plugins, like analyze readability, are also toggles. Which means that mockup has two patterns, 1 pattern which shows two options and a checkmark next to the chosen one in the group, and 1 pattern which shows a mix of direct links and "toggle links" where the toggle link simply gets a checkmark.

One idea is to have a menu like this:

-------------------------------
|     Edit HTML               |
-------------------------------
|     Fix to block            |
-------------------------------
|     Analyze Readability     |
|     Configure Publicize     |
-------------------------------

Options 1 2 and 3 above are "toggles", and you could then end up with something like this:

-------------------------------
|  ✓  Edit HTML               |
-------------------------------
|     Fix to block            |
-------------------------------
|  ✓  Analyze Readability     |
|     Configure Publicize     |
-------------------------------

I also think not requiring an icon would make it easier to add items to the menu.

But — I'm curious if mergint the two HTML/Visual mode switches into a single toggle is a good idea or not. I'm also not sure my ideas above should hold this PR from getting merged. Curious about your thoughts, @karmatosed

@codecov

This comment has been minimized.

Show comment
Hide comment
@codecov

codecov bot Nov 13, 2017

Codecov Report

Merging #3456 into master will increase coverage by 1.2%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #3456     +/-   ##
=========================================
+ Coverage   33.83%   35.03%   +1.2%     
=========================================
  Files         253      283     +30     
  Lines        6741     8245   +1504     
  Branches     1223     1633    +410     
=========================================
+ Hits         2281     2889    +608     
- Misses       3761     4420    +659     
- Partials      699      936    +237
Impacted Files Coverage Δ
editor/header/ellipsis-menu/index.js 0% <ø> (ø) ⬆️
components/menu-items/menu-items-toggle.js 0% <0%> (ø)
editor/header/fixed-toolbar-toggle/index.js 0% <0%> (ø) ⬆️
editor/header/mode-switcher/index.js 0% <0%> (ø) ⬆️
components/menu-items/menu-items-group.js 0% <0%> (ø)
blocks/library/verse/index.js 28.57% <0%> (-8.93%) ⬇️
blocks/library/text-columns/index.js 25.92% <0%> (-7.41%) ⬇️
blocks/library/shortcode/index.js 44.44% <0%> (-5.56%) ⬇️
blocks/library/preformatted/index.js 44.44% <0%> (-5.56%) ⬇️
blocks/library/heading/index.js 23.68% <0%> (-4.89%) ⬇️
... and 80 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c1ccd85...1147454. Read the comment docs.

codecov bot commented Nov 13, 2017

Codecov Report

Merging #3456 into master will increase coverage by 1.2%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #3456     +/-   ##
=========================================
+ Coverage   33.83%   35.03%   +1.2%     
=========================================
  Files         253      283     +30     
  Lines        6741     8245   +1504     
  Branches     1223     1633    +410     
=========================================
+ Hits         2281     2889    +608     
- Misses       3761     4420    +659     
- Partials      699      936    +237
Impacted Files Coverage Δ
editor/header/ellipsis-menu/index.js 0% <ø> (ø) ⬆️
components/menu-items/menu-items-toggle.js 0% <0%> (ø)
editor/header/fixed-toolbar-toggle/index.js 0% <0%> (ø) ⬆️
editor/header/mode-switcher/index.js 0% <0%> (ø) ⬆️
components/menu-items/menu-items-group.js 0% <0%> (ø)
blocks/library/verse/index.js 28.57% <0%> (-8.93%) ⬇️
blocks/library/text-columns/index.js 25.92% <0%> (-7.41%) ⬇️
blocks/library/shortcode/index.js 44.44% <0%> (-5.56%) ⬇️
blocks/library/preformatted/index.js 44.44% <0%> (-5.56%) ⬇️
blocks/library/heading/index.js 23.68% <0%> (-4.89%) ⬇️
... and 80 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c1ccd85...1147454. Read the comment docs.

@youknowriad

This comment has been minimized.

Show comment
Hide comment
@youknowriad

youknowriad Nov 13, 2017

Contributor

Happy to update the PR to use the no-icon and check-selected pattern if you think it's a better alternative to explore.

Contributor

youknowriad commented Nov 13, 2017

Happy to update the PR to use the no-icon and check-selected pattern if you think it's a better alternative to explore.

@StaggerLeee

This comment has been minimized.

Show comment
Hide comment
@StaggerLeee

StaggerLeee Nov 13, 2017

I like first mock-up more.

Second one has the same problem as block descriptions. Move keyboard shortcuts information to some dedicated and hidden "Help" icon/place.

I do not know if you are aware Users will see it for years in the future.
It is like TV On/Off button has text above "This is a button to turn on appliance".

Make some orders and rules how you do all this.

Will User beginner need it ? Yes, one week max, really max, time.
Will User beginner see it for 5 next years without needing it ? Yes.
Remove it from view and bury it deep somewhere, already on default.

StaggerLeee commented Nov 13, 2017

I like first mock-up more.

Second one has the same problem as block descriptions. Move keyboard shortcuts information to some dedicated and hidden "Help" icon/place.

I do not know if you are aware Users will see it for years in the future.
It is like TV On/Off button has text above "This is a button to turn on appliance".

Make some orders and rules how you do all this.

Will User beginner need it ? Yes, one week max, really max, time.
Will User beginner see it for 5 next years without needing it ? Yes.
Remove it from view and bury it deep somewhere, already on default.

@youknowriad

This comment has been minimized.

Show comment
Hide comment
@youknowriad

youknowriad Nov 13, 2017

Contributor

While this PR is about the design and not the keyboard shorcuts, I tend to prefer the second design personally.

And even the shortcuts, it's very different from the "On/Off" because we have too many keyboard shortcuts and we can easily forget them. It's also a widely used pattern to show shortcuts next to menu items (Window Menus for example in several Operating systems)

screen shot 2017-11-13 at 15 45 30

Contributor

youknowriad commented Nov 13, 2017

While this PR is about the design and not the keyboard shorcuts, I tend to prefer the second design personally.

And even the shortcuts, it's very different from the "On/Off" because we have too many keyboard shortcuts and we can easily forget them. It's also a widely used pattern to show shortcuts next to menu items (Window Menus for example in several Operating systems)

screen shot 2017-11-13 at 15 45 30

@StaggerLeee

This comment has been minimized.

Show comment
Hide comment
@StaggerLeee

StaggerLeee Nov 13, 2017

You are not Windows.

People will hook there own things. If you do not make filters other core developers will, when they take over Gutenberg from you.

StaggerLeee commented Nov 13, 2017

You are not Windows.

People will hook there own things. If you do not make filters other core developers will, when they take over Gutenberg from you.

@karmatosed

This comment has been minimized.

Show comment
Hide comment
@karmatosed

karmatosed Nov 13, 2017

Member

I actually think having no icons and then the checkmark is more scalable. The weight of the current visual is a little strong. I can see what the goal is, but I think we've gone too far in the heavier UI direction. The feeling of the interface is instantly less clean and fresh. However, it can be resolved and sometimes we just need to see these things, so thank you for showing it.

2017-11-13 at 20 49

Also worth noting there are issues with contrast on the selected.

I like the idea with the toggles combining @jasmussen, at least to visually try. I do not think what we currently have is the best solution.

Before merge I would like the visual strength eased up on.

Member

karmatosed commented Nov 13, 2017

I actually think having no icons and then the checkmark is more scalable. The weight of the current visual is a little strong. I can see what the goal is, but I think we've gone too far in the heavier UI direction. The feeling of the interface is instantly less clean and fresh. However, it can be resolved and sometimes we just need to see these things, so thank you for showing it.

2017-11-13 at 20 49

Also worth noting there are issues with contrast on the selected.

I like the idea with the toggles combining @jasmussen, at least to visually try. I do not think what we currently have is the best solution.

Before merge I would like the visual strength eased up on.

@youknowriad

This comment has been minimized.

Show comment
Hide comment
@youknowriad

youknowriad Nov 14, 2017

Contributor

Agreed this feels heavy. I updated to use the lighter design

screen shot 2017-11-14 at 09 13 37

Contributor

youknowriad commented Nov 14, 2017

Agreed this feels heavy. I updated to use the lighter design

screen shot 2017-11-14 at 09 13 37

@jasmussen

This comment has been minimized.

Show comment
Hide comment
@jasmussen

jasmussen Nov 14, 2017

Contributor

I like it. I think we should probably get this in.

In a future iteration, perhaps when we get started on that plugin support, we can look at the concept outlined here, where instead of having two items in a group, one untoggling the other, we have a single item that's either checked or unchecked.

Contributor

jasmussen commented Nov 14, 2017

I like it. I think we should probably get this in.

In a future iteration, perhaps when we get started on that plugin support, we can look at the concept outlined here, where instead of having two items in a group, one untoggling the other, we have a single item that's either checked or unchecked.

@aduth

In this branch, when making a selection via keyboard, focus is moved elsewhere but the ellipsis menu stays open. This is unlike master where at least the menu is closed. I might expect that this would behave like a radio group, i.e. I can use arrow keys, and tab moves between groups. As implemented, tab moves between options within a group, and there is no behavior for arrow keys.

@youknowriad

This comment has been minimized.

Show comment
Hide comment
@youknowriad

youknowriad Nov 20, 2017

Contributor

Feedback addressed here. It should be good to go

Contributor

youknowriad commented Nov 20, 2017

Feedback addressed here. It should be good to go

@jasmussen

This comment has been minimized.

Show comment
Hide comment
@jasmussen

jasmussen Nov 20, 2017

Contributor

Looks good, probably good to ship.

I think we should look at how toggles work inside menus, and find some rules for it. I have looked at how Google Docs handles it, and they actually have two types of toggles inside menus, which should be confusing, but is actually very intuitive.

The two toggles are:

  1. Checkmark left of menu item acts as a checkbox, that is the single menu item toggles a checkmark when selected
  2. Multiple menu items act as a radio group where the whichever item selected gets a checkmark

This branch currently does the latter. And again, Google Docs does both, yet manages to stay pretty intuitive.

Google Docs checkbox behavior:

screen shot 2017-11-20 at 11 07 26

screen shot 2017-11-20 at 11 07 19

Google Docs radio group behavior:

screen shot 2017-11-20 at 11 08 05

screen shot 2017-11-20 at 11 08 31

The reason I'm asking is whether we should adopt both these patterns also. For example, like this:

screen shot 2017-11-20 at 11 03 35 1

The editor type would use the radio behavior, and the toolbar setting would use the checkbox behavior.

The alternative is to have both use the checkbox behavior, but that doesn't seem quite as intuitive to me:

screen shot 2017-11-20 at 10 54 07

The reason I think it's important to clarify this (perhaps separately, let me know if you'd like me to ticket), is that we'd love to get some extensions into this menu:

screen shot 2017-11-20 at 11 10 06

And in this case it'd be nice if the extensions could be either direct links (which can have icons), or checkbox toggles like spell-check or readability checks. Having two entries for every such extension (Enable spellchecking & Disable spellchecking items in a radio group behavior) would quickly make for an unwieldy menu.

Contributor

jasmussen commented Nov 20, 2017

Looks good, probably good to ship.

I think we should look at how toggles work inside menus, and find some rules for it. I have looked at how Google Docs handles it, and they actually have two types of toggles inside menus, which should be confusing, but is actually very intuitive.

The two toggles are:

  1. Checkmark left of menu item acts as a checkbox, that is the single menu item toggles a checkmark when selected
  2. Multiple menu items act as a radio group where the whichever item selected gets a checkmark

This branch currently does the latter. And again, Google Docs does both, yet manages to stay pretty intuitive.

Google Docs checkbox behavior:

screen shot 2017-11-20 at 11 07 26

screen shot 2017-11-20 at 11 07 19

Google Docs radio group behavior:

screen shot 2017-11-20 at 11 08 05

screen shot 2017-11-20 at 11 08 31

The reason I'm asking is whether we should adopt both these patterns also. For example, like this:

screen shot 2017-11-20 at 11 03 35 1

The editor type would use the radio behavior, and the toolbar setting would use the checkbox behavior.

The alternative is to have both use the checkbox behavior, but that doesn't seem quite as intuitive to me:

screen shot 2017-11-20 at 10 54 07

The reason I think it's important to clarify this (perhaps separately, let me know if you'd like me to ticket), is that we'd love to get some extensions into this menu:

screen shot 2017-11-20 at 11 10 06

And in this case it'd be nice if the extensions could be either direct links (which can have icons), or checkbox toggles like spell-check or readability checks. Having two entries for every such extension (Enable spellchecking & Disable spellchecking items in a radio group behavior) would quickly make for an unwieldy menu.

@youknowriad

This comment has been minimized.

Show comment
Hide comment
@youknowriad

youknowriad Nov 20, 2017

Contributor

Thanks for the explanations @jasmussen

I really like the second approach mixing both the "radio group" and the "checkbox" behavior for the Toolbar toggle. I updated this PR accordingly.

Contributor

youknowriad commented Nov 20, 2017

Thanks for the explanations @jasmussen

I really like the second approach mixing both the "radio group" and the "checkbox" behavior for the Toolbar toggle. I updated this PR accordingly.

@jasmussen

This comment has been minimized.

Show comment
Hide comment
@jasmussen

jasmussen Nov 20, 2017

Contributor

👍 👍 designwise from me. If Tammie doesn't object then ship.

Contributor

jasmussen commented Nov 20, 2017

👍 👍 designwise from me. If Tammie doesn't object then ship.

@afercia

This comment has been minimized.

Show comment
Hide comment
@afercia

afercia Nov 20, 2017

Contributor

See also #3566.

For some history about why in WordPress the word "HTML" was changed to "Text", see https://core.trac.wordpress.org/ticket/20993

Seems to me the argumentations there are still valid, so "Text" is better than "HTML". However, see also https://core.trac.wordpress.org/ticket/38061 which proposes to use "Code editor" / "Visual editor".

Contributor

afercia commented Nov 20, 2017

See also #3566.

For some history about why in WordPress the word "HTML" was changed to "Text", see https://core.trac.wordpress.org/ticket/20993

Seems to me the argumentations there are still valid, so "Text" is better than "HTML". However, see also https://core.trac.wordpress.org/ticket/38061 which proposes to use "Code editor" / "Visual editor".

@youknowriad

This comment has been minimized.

Show comment
Hide comment
@youknowriad

youknowriad Nov 20, 2017

Contributor

Renamed "HTML Editor" -> "Code Editor"

Contributor

youknowriad commented Nov 20, 2017

Renamed "HTML Editor" -> "Code Editor"

@youknowriad youknowriad merged commit 1a270df into master Nov 20, 2017

3 checks passed

codecov/project 35.03% (+1.2%) compared to c1ccd85
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@youknowriad youknowriad deleted the update/ellipsis-menu-design branch Nov 20, 2017

@jasmussen

This comment has been minimized.

Show comment
Hide comment
@jasmussen

jasmussen Nov 20, 2017

Contributor

I dig the new phrasing, "Code Editor". I also think it's good to merge early and test, so we can get a feel for it. Gutenberg is a perfect place to do that, because if for whatever unforeseen reason comes up that makes us have to revert, it's an easy change in this phase.

Contributor

jasmussen commented Nov 20, 2017

I dig the new phrasing, "Code Editor". I also think it's good to merge early and test, so we can get a feel for it. Gutenberg is a perfect place to do that, because if for whatever unforeseen reason comes up that makes us have to revert, it's an easy change in this phase.

@karmatosed

This comment has been minimized.

Show comment
Hide comment
@karmatosed

karmatosed Nov 20, 2017

Member

+1 for code editor, that makes sense as a name. This looks good to ship for me :) Thanks for the awesome work.

Member

karmatosed commented Nov 20, 2017

+1 for code editor, that makes sense as a name. This looks good to ship for me :) Thanks for the awesome work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment