Use a DropdownMenu for menu selection in the navigation screen #25390

talldan · 2020-09-17T04:45:30Z

Description

Closes #25312.

Use a DropdownMenu component for choosing a menu.

Needs accessibility feedback ...

How has this been tested?

Visit the navigation screen
Make sure you have more than one menu
Try selecting menus

Screenshots

Types of changes

Bug fix (non-breaking change which fixes an issue)

Checklist:

My code is tested.
My code follows the WordPress code style.
My code follows the accessibility standards.
My code has proper inline documentation.
I've included developer documentation if appropriate.
I've updated all React Native files affected by any refactorings/renamings in this PR.

…ction

github-actions · 2020-09-17T04:53:31Z

Size Change: -27.6 kB (2%)

Total Size: 1.17 MB

Filename	Size	Change
`build/annotations/index.js`	3.52 kB	+1 B
`build/api-fetch/index.js`	3.35 kB	+19 B (0%)
`build/autop/index.js`	2.72 kB	-1 B
`build/block-directory/index.js`	8.61 kB	+205 B (2%)
`build/block-editor/index.js`	129 kB	+25 B (0%)
`build/block-editor/style-rtl.css`	11 kB	-127 B (1%)
`build/block-editor/style.css`	11 kB	-129 B (1%)
`build/block-library/editor-rtl.css`	8.6 kB	+10 B (0%)
`build/block-library/editor.css`	8.6 kB	+9 B (0%)
`build/block-library/index.js`	135 kB	+524 B (0%)
`build/block-library/style-rtl.css`	7.65 kB	+50 B (0%)
`build/block-library/style.css`	7.64 kB	+55 B (0%)
`build/block-serialization-default-parser/index.js`	1.78 kB	-1 B
`build/blocks/index.js`	47.5 kB	-52 B (0%)
`build/components/index.js`	168 kB	-33.2 kB (19%)	👏
`build/components/style-rtl.css`	15.4 kB	-69 B (0%)
`build/components/style.css`	15.4 kB	-69 B (0%)
`build/compose/index.js`	9.42 kB	-60 B (0%)
`build/core-data/index.js`	12 kB	+46 B (0%)
`build/data-controls/index.js`	1.27 kB	+1 B
`build/data/index.js`	8.42 kB	-6 B (0%)
`build/date/index.js`	31.9 kB	-3 B (0%)
`build/dom/index.js`	4.42 kB	-20 B (0%)
`build/edit-navigation/index.js`	10.8 kB	+384 B (3%)
`build/edit-navigation/style-rtl.css`	881 B	+13 B (1%)
`build/edit-navigation/style.css`	885 B	+14 B (1%)
`build/edit-post/index.js`	306 kB	+60 B (0%)
`build/edit-post/style-rtl.css`	6.25 kB	+15 B (0%)
`build/edit-post/style.css`	6.24 kB	+16 B (0%)
`build/edit-site/index.js`	20.4 kB	+1.25 kB (6%)	🔍
`build/edit-site/style-rtl.css`	3.78 kB	+647 B (17%)	⚠️
`build/edit-site/style.css`	3.78 kB	+648 B (17%)	⚠️
`build/edit-widgets/index.js`	18.7 kB	+1.88 kB (10%)	⚠️
`build/edit-widgets/style-rtl.css`	3.03 kB	+274 B (9%)	🔍
`build/edit-widgets/style.css`	3.03 kB	+275 B (9%)	🔍
`build/editor/index.js`	45.5 kB	+298 B (0%)
`build/editor/style-rtl.css`	3.83 kB	+28 B (0%)
`build/editor/style.css`	3.82 kB	+26 B (0%)
`build/element/index.js`	4.44 kB	-8 B (0%)
`build/escape-html/index.js`	733 B	-1 B
`build/format-library/index.js`	7.49 kB	-13 B (0%)
`build/html-entities/index.js`	621 B	-1 B
`build/i18n/index.js`	3.55 kB	+1 B
`build/is-shallow-equal/index.js`	709 B	-1 B
`build/list-reusable-blocks/index.js`	3.02 kB	+1 B
`build/media-utils/index.js`	5.12 kB	+21 B (0%)
`build/notices/index.js`	1.69 kB	+1 B
`build/nux/index.js`	3.27 kB	+17 B (0%)
`build/plugins/index.js`	2.44 kB	+4 B (0%)
`build/priority-queue/index.js`	789 B	-1 B
`build/rich-text/index.js`	13 kB	-673 B (5%)	✅
`build/server-side-render/index.js`	2.6 kB	+4 B (0%)
`build/shortcode/index.js`	1.7 kB	-1 B
`build/url/index.js`	4.06 kB	-3 B (0%)
`build/viewport/index.js`	1.74 kB	-1 B
`build/warning/index.js`	1.13 kB	-1 B

ℹ️ View Unchanged

Filename	Size	Change
`build/a11y/index.js`	1.14 kB	0 B
`build/blob/index.js`	620 B	0 B
`build/block-directory/style-rtl.css`	943 B	0 B
`build/block-directory/style.css`	942 B	0 B
`build/block-library/theme-rtl.css`	741 B	0 B
`build/block-library/theme.css`	741 B	0 B
`build/block-serialization-spec-parser/index.js`	3.1 kB	0 B
`build/deprecated/index.js`	772 B	0 B
`build/dom-ready/index.js`	568 B	0 B
`build/editor/editor-styles-rtl.css`	492 B	0 B
`build/editor/editor-styles.css`	493 B	0 B
`build/format-library/style-rtl.css`	547 B	0 B
`build/format-library/style.css`	548 B	0 B
`build/hooks/index.js`	1.74 kB	0 B
`build/keyboard-shortcuts/index.js`	2.39 kB	0 B
`build/keycodes/index.js`	1.85 kB	0 B
`build/list-reusable-blocks/style-rtl.css`	476 B	0 B
`build/list-reusable-blocks/style.css`	476 B	0 B
`build/nux/style-rtl.css`	671 B	0 B
`build/nux/style.css`	668 B	0 B
`build/primitives/index.js`	1.34 kB	0 B
`build/redux-routine/index.js`	2.85 kB	0 B
`build/token-list/index.js`	1.24 kB	0 B
`build/wordcount/index.js`	1.17 kB	0 B

_{compressed-size-action}

draganescu

On this PR when I create a new menu the selector does not move to the new menu created, it remains on the previously selected one.

…he selector

talldan · 2020-09-17T08:59:24Z

@draganescu Good catch. 07208b6 fixes that.

shaunandrews · 2020-09-17T21:58:48Z

I'm thinking it might be better to consolidate the interactions; All the other buttons here (Add new, Manage locations) are DropdownMenu's and maybe this one should be too. It would look something like this:

Posted this same comment over on #25312 (comment)

talldan · 2020-09-18T03:17:08Z

Thanks @shaunandrews, that's much nicer. Here's how it looks now:

draganescu

This works great and looks the same! 👏

afercia

From an accessibility perspective, a DropdownMenu looks good to me and I do see it brings in consistency. However, there's no indication of which menu is being edited. I'd like to propose to add a H2 heading, please see my comment on the issue: #25312 (comment)

Roughly:

talldan · 2020-09-21T05:20:51Z

Thanks for reviewing @afercia.

Is the position of an h2 (disregarding the fact it has to come after the h1 in the DOM) something that needs to be considered? Just wondering how much flexibility there will be in positioning it?

@shaunandrews would be great to get your advice on how to incorporate this into the screen.

afercia · 2020-09-21T14:48:13Z

Well, the visual/reading order should match the DOM order. Apart from that, headings don't necessarily need to look "big". They can be styled, as long as there's a visual hierarchy that matches the semantic hierarchy.

talldan · 2020-09-22T03:09:13Z

Here's what I've come up with, but would be good to get design feedback:

afercia · 2020-09-23T12:59:33Z

Thanks @talldan !

Question: is it possible to have a H2 heading also when there are no menus? I'd use a "Create menu" text for the heading or something along those lines. Otherwise, that page section won't have a heading at all (also the current H3 List View will skip a level):

shaunandrews · 2020-09-24T18:53:04Z

I think we can keep the elements together; No need to right-align the buttons:

If there are no menus, perhaps we simply state that in the heading:

talldan · 2020-09-30T03:48:01Z

Thanks @shaunandrews, I've made those changes:

The header can show three things:

'Editing: Menu name' (when a menu is selected)
'Create new menu' (when the user has no menus)
'Loading …' (when the page is loading menus. This last one won't be displayed if the menus are preloaded as per Navigation: Preload API requests #24642)

talldan · 2020-09-30T09:08:47Z

If there are no menus, perhaps we simply state that in the heading:

Oops, I missed this earlier this morning. Somehow I looked at it and thought this text was part of the screenshot 😄

I've now updated the header text to:

'Editing: Menu name' (when a menu is selected)
'No menus available' (when the user has no menus)
'Loading …' (when the page is loading menus. This last one won't be displayed if the menus are preloaded as per Navigation: Preload API requests #24642)

draganescu · 2020-10-08T12:59:20Z

@afercia could you re-review this PR? :) It seems it's current state reflects your requested changes. Thank you 🙏

outdated

talldan added 3 commits September 17, 2020 12:18

Add menu selection button

733ee30

Update copy

d789df0

Make the change menu button disabled when clicking it would have no a…

5c66bd4

…ction

talldan requested review from shaunandrews, afercia and tellthemachines September 17, 2020 04:45

talldan self-assigned this Sep 17, 2020

talldan requested review from kevin940726, adamziel and draganescu September 17, 2020 04:45

draganescu reviewed Sep 17, 2020

View reviewed changes

talldan added 2 commits September 17, 2020 16:57

Update selected menu when creating a menu or any change external to t…

07208b6

…he selector

Add explanatory comment

d6204d5

Switch to a dropdown menu for menu selector

8593bb0

talldan changed the title ~~Add a button for menu selection to the navigation screen~~ Use a DropdownMenu for menu selection in the navigation screen Sep 18, 2020

talldan removed Needs Copy Review Needs review of user-facing copy (language, phrasing) Needs Design Feedback Needs general design feedback. labels Sep 18, 2020

draganescu approved these changes Sep 18, 2020

View reviewed changes

afercia previously requested changes Sep 18, 2020

View reviewed changes

talldan added 2 commits September 22, 2020 10:56

Add header showing currently edited menu

f3e0492

Move dropdown menu back into main header file

67d8985

talldan added the Needs Design Feedback Needs general design feedback. label Sep 22, 2020

Align items left and add create menu header

66e8727

Change text to No menus available

fc42623

talldan removed the Needs Design Feedback Needs general design feedback. label Oct 12, 2020

talldan merged commit e8556c9 into master Oct 20, 2020

talldan deleted the add/button-for-menu-selection branch October 20, 2020 08:52

github-actions bot added this to the Gutenberg 9.3 milestone Oct 20, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use a DropdownMenu for menu selection in the navigation screen #25390

Use a DropdownMenu for menu selection in the navigation screen #25390

talldan commented Sep 17, 2020 •

edited

Loading

github-actions bot commented Sep 17, 2020 •

edited

Loading

draganescu left a comment

talldan commented Sep 17, 2020

shaunandrews commented Sep 17, 2020

talldan commented Sep 18, 2020

draganescu left a comment

afercia left a comment

talldan commented Sep 21, 2020 •

edited

Loading

afercia commented Sep 21, 2020

talldan commented Sep 22, 2020

afercia commented Sep 23, 2020

shaunandrews commented Sep 24, 2020

talldan commented Sep 30, 2020

talldan commented Sep 30, 2020

draganescu commented Oct 8, 2020

Use a DropdownMenu for menu selection in the navigation screen #25390

Use a DropdownMenu for menu selection in the navigation screen #25390

Conversation

talldan commented Sep 17, 2020 • edited Loading

Description

How has this been tested?

Screenshots

Types of changes

Checklist:

github-actions bot commented Sep 17, 2020 • edited Loading

draganescu left a comment

Choose a reason for hiding this comment

talldan commented Sep 17, 2020

shaunandrews commented Sep 17, 2020

talldan commented Sep 18, 2020

draganescu left a comment

Choose a reason for hiding this comment

afercia left a comment

Choose a reason for hiding this comment

talldan commented Sep 21, 2020 • edited Loading

afercia commented Sep 21, 2020

talldan commented Sep 22, 2020

afercia commented Sep 23, 2020

shaunandrews commented Sep 24, 2020

talldan commented Sep 30, 2020

talldan commented Sep 30, 2020

draganescu commented Oct 8, 2020

talldan commented Sep 17, 2020 •

edited

Loading

github-actions bot commented Sep 17, 2020 •

edited

Loading

talldan commented Sep 21, 2020 •

edited

Loading