Skip to content
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

Navigation: Add delete nav menu button #35981

Merged
merged 3 commits into from
Nov 1, 2021

Conversation

talldan
Copy link
Contributor

@talldan talldan commented Oct 27, 2021

Description

Adds a button for deleting 'wp_navigation' posts to the nav block's block inspector:
Screenshot 2021-10-27 at 4 03 03 pm

Deleting has a confirm step:
Screenshot 2021-10-27 at 4 05 40 pm

There's a bug in the multi-entity saving flow that needs to be shipped, preferably before this is shipped. When deleting an entity, it shows in the saving panel as an 'undefined' change 😬 : Screenshot 2021-10-27 at 4 04 27 pm

This should preferably be filtered out, deleting entities is immediate and doesn't need to be saved.

How has this been tested?

  1. Add a nav block
  2. Create a new menu
  3. Click delete
  4. Confirm deletion

Types of changes

New feature (non-breaking change which adds functionality)

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • 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 (please manually search all *.native.js files for terms that need renaming or removal).

@talldan talldan added [Type] Enhancement A suggestion for improvement. [Block] Navigation Affects the Navigation Block labels Oct 27, 2021
@talldan talldan self-assigned this Oct 27, 2021
@github-actions
Copy link

github-actions bot commented Oct 27, 2021

Size Change: +271 B (0%)

Total Size: 1.08 MB

Filename Size Change
build/block-library/blocks/navigation/editor-rtl.css 1.83 kB +18 B (+1%)
build/block-library/blocks/navigation/editor.css 1.83 kB +18 B (+1%)
build/block-library/editor-rtl.css 9.79 kB +14 B (0%)
build/block-library/editor.css 9.79 kB +14 B (0%)
build/block-library/index.min.js 155 kB +207 B (0%)
ℹ️ View Unchanged
Filename Size
build/a11y/index.min.js 931 B
build/admin-manifest/index.min.js 1.09 kB
build/annotations/index.min.js 2.7 kB
build/api-fetch/index.min.js 2.21 kB
build/autop/index.min.js 2.08 kB
build/blob/index.min.js 459 B
build/block-directory/index.min.js 6.2 kB
build/block-directory/style-rtl.css 1.01 kB
build/block-directory/style.css 1.01 kB
build/block-editor/default-editor-styles-rtl.css 378 B
build/block-editor/default-editor-styles.css 378 B
build/block-editor/index.min.js 135 kB
build/block-editor/style-rtl.css 14.1 kB
build/block-editor/style.css 14 kB
build/block-library/blocks/archives/editor-rtl.css 61 B
build/block-library/blocks/archives/editor.css 60 B
build/block-library/blocks/archives/style-rtl.css 65 B
build/block-library/blocks/archives/style.css 65 B
build/block-library/blocks/audio/editor-rtl.css 58 B
build/block-library/blocks/audio/editor.css 58 B
build/block-library/blocks/audio/style-rtl.css 111 B
build/block-library/blocks/audio/style.css 111 B
build/block-library/blocks/audio/theme-rtl.css 125 B
build/block-library/blocks/audio/theme.css 125 B
build/block-library/blocks/block/editor-rtl.css 161 B
build/block-library/blocks/block/editor.css 161 B
build/block-library/blocks/button/editor-rtl.css 470 B
build/block-library/blocks/button/editor.css 470 B
build/block-library/blocks/button/style-rtl.css 560 B
build/block-library/blocks/button/style.css 560 B
build/block-library/blocks/buttons/editor-rtl.css 309 B
build/block-library/blocks/buttons/editor.css 309 B
build/block-library/blocks/buttons/style-rtl.css 317 B
build/block-library/blocks/buttons/style.css 317 B
build/block-library/blocks/calendar/style-rtl.css 207 B
build/block-library/blocks/calendar/style.css 207 B
build/block-library/blocks/categories/editor-rtl.css 84 B
build/block-library/blocks/categories/editor.css 83 B
build/block-library/blocks/categories/style-rtl.css 79 B
build/block-library/blocks/categories/style.css 79 B
build/block-library/blocks/code/style-rtl.css 90 B
build/block-library/blocks/code/style.css 90 B
build/block-library/blocks/code/theme-rtl.css 131 B
build/block-library/blocks/code/theme.css 131 B
build/block-library/blocks/columns/editor-rtl.css 206 B
build/block-library/blocks/columns/editor.css 205 B
build/block-library/blocks/columns/style-rtl.css 497 B
build/block-library/blocks/columns/style.css 496 B
build/block-library/blocks/cover/editor-rtl.css 546 B
build/block-library/blocks/cover/editor.css 547 B
build/block-library/blocks/cover/style-rtl.css 1.17 kB
build/block-library/blocks/cover/style.css 1.17 kB
build/block-library/blocks/embed/editor-rtl.css 488 B
build/block-library/blocks/embed/editor.css 488 B
build/block-library/blocks/embed/style-rtl.css 417 B
build/block-library/blocks/embed/style.css 417 B
build/block-library/blocks/embed/theme-rtl.css 124 B
build/block-library/blocks/embed/theme.css 124 B
build/block-library/blocks/file/editor-rtl.css 300 B
build/block-library/blocks/file/editor.css 300 B
build/block-library/blocks/file/style-rtl.css 255 B
build/block-library/blocks/file/style.css 255 B
build/block-library/blocks/file/view.min.js 322 B
build/block-library/blocks/freeform/editor-rtl.css 2.44 kB
build/block-library/blocks/freeform/editor.css 2.44 kB
build/block-library/blocks/gallery/editor-rtl.css 977 B
build/block-library/blocks/gallery/editor.css 982 B
build/block-library/blocks/gallery/style-rtl.css 1.6 kB
build/block-library/blocks/gallery/style.css 1.59 kB
build/block-library/blocks/gallery/theme-rtl.css 122 B
build/block-library/blocks/gallery/theme.css 122 B
build/block-library/blocks/group/editor-rtl.css 159 B
build/block-library/blocks/group/editor.css 159 B
build/block-library/blocks/group/style-rtl.css 57 B
build/block-library/blocks/group/style.css 57 B
build/block-library/blocks/group/theme-rtl.css 78 B
build/block-library/blocks/group/theme.css 78 B
build/block-library/blocks/heading/style-rtl.css 114 B
build/block-library/blocks/heading/style.css 114 B
build/block-library/blocks/home-link/style-rtl.css 247 B
build/block-library/blocks/home-link/style.css 247 B
build/block-library/blocks/html/editor-rtl.css 332 B
build/block-library/blocks/html/editor.css 333 B
build/block-library/blocks/image/editor-rtl.css 731 B
build/block-library/blocks/image/editor.css 730 B
build/block-library/blocks/image/style-rtl.css 502 B
build/block-library/blocks/image/style.css 505 B
build/block-library/blocks/image/theme-rtl.css 124 B
build/block-library/blocks/image/theme.css 124 B
build/block-library/blocks/latest-comments/style-rtl.css 284 B
build/block-library/blocks/latest-comments/style.css 284 B
build/block-library/blocks/latest-posts/editor-rtl.css 137 B
build/block-library/blocks/latest-posts/editor.css 137 B
build/block-library/blocks/latest-posts/style-rtl.css 528 B
build/block-library/blocks/latest-posts/style.css 527 B
build/block-library/blocks/list/style-rtl.css 94 B
build/block-library/blocks/list/style.css 94 B
build/block-library/blocks/media-text/editor-rtl.css 266 B
build/block-library/blocks/media-text/editor.css 263 B
build/block-library/blocks/media-text/style-rtl.css 493 B
build/block-library/blocks/media-text/style.css 490 B
build/block-library/blocks/more/editor-rtl.css 431 B
build/block-library/blocks/more/editor.css 431 B
build/block-library/blocks/navigation-link/editor-rtl.css 642 B
build/block-library/blocks/navigation-link/editor.css 642 B
build/block-library/blocks/navigation-link/style-rtl.css 94 B
build/block-library/blocks/navigation-link/style.css 94 B
build/block-library/blocks/navigation-submenu/editor-rtl.css 299 B
build/block-library/blocks/navigation-submenu/editor.css 299 B
build/block-library/blocks/navigation-submenu/style-rtl.css 195 B
build/block-library/blocks/navigation-submenu/style.css 195 B
build/block-library/blocks/navigation-submenu/view.min.js 343 B
build/block-library/blocks/navigation/style-rtl.css 1.71 kB
build/block-library/blocks/navigation/style.css 1.7 kB
build/block-library/blocks/navigation/view.min.js 2.74 kB
build/block-library/blocks/nextpage/editor-rtl.css 395 B
build/block-library/blocks/nextpage/editor.css 395 B
build/block-library/blocks/page-list/editor-rtl.css 377 B
build/block-library/blocks/page-list/editor.css 377 B
build/block-library/blocks/page-list/style-rtl.css 198 B
build/block-library/blocks/page-list/style.css 198 B
build/block-library/blocks/paragraph/editor-rtl.css 157 B
build/block-library/blocks/paragraph/editor.css 157 B
build/block-library/blocks/paragraph/style-rtl.css 273 B
build/block-library/blocks/paragraph/style.css 273 B
build/block-library/blocks/post-author/style-rtl.css 175 B
build/block-library/blocks/post-author/style.css 176 B
build/block-library/blocks/post-comments-form/style-rtl.css 347 B
build/block-library/blocks/post-comments-form/style.css 347 B
build/block-library/blocks/post-comments/style-rtl.css 492 B
build/block-library/blocks/post-comments/style.css 493 B
build/block-library/blocks/post-content/style-rtl.css 56 B
build/block-library/blocks/post-content/style.css 56 B
build/block-library/blocks/post-excerpt/editor-rtl.css 73 B
build/block-library/blocks/post-excerpt/editor.css 73 B
build/block-library/blocks/post-excerpt/style-rtl.css 69 B
build/block-library/blocks/post-excerpt/style.css 69 B
build/block-library/blocks/post-featured-image/editor-rtl.css 396 B
build/block-library/blocks/post-featured-image/editor.css 397 B
build/block-library/blocks/post-featured-image/style-rtl.css 156 B
build/block-library/blocks/post-featured-image/style.css 156 B
build/block-library/blocks/post-template/editor-rtl.css 99 B
build/block-library/blocks/post-template/editor.css 98 B
build/block-library/blocks/post-template/style-rtl.css 391 B
build/block-library/blocks/post-template/style.css 392 B
build/block-library/blocks/post-terms/style-rtl.css 73 B
build/block-library/blocks/post-terms/style.css 73 B
build/block-library/blocks/post-title/style-rtl.css 60 B
build/block-library/blocks/post-title/style.css 60 B
build/block-library/blocks/preformatted/style-rtl.css 103 B
build/block-library/blocks/preformatted/style.css 103 B
build/block-library/blocks/pullquote/editor-rtl.css 198 B
build/block-library/blocks/pullquote/editor.css 198 B
build/block-library/blocks/pullquote/style-rtl.css 378 B
build/block-library/blocks/pullquote/style.css 378 B
build/block-library/blocks/pullquote/theme-rtl.css 167 B
build/block-library/blocks/pullquote/theme.css 167 B
build/block-library/blocks/query-pagination-numbers/editor-rtl.css 122 B
build/block-library/blocks/query-pagination-numbers/editor.css 121 B
build/block-library/blocks/query-pagination/editor-rtl.css 262 B
build/block-library/blocks/query-pagination/editor.css 255 B
build/block-library/blocks/query-pagination/style-rtl.css 234 B
build/block-library/blocks/query-pagination/style.css 231 B
build/block-library/blocks/query/editor-rtl.css 131 B
build/block-library/blocks/query/editor.css 132 B
build/block-library/blocks/quote/style-rtl.css 187 B
build/block-library/blocks/quote/style.css 187 B
build/block-library/blocks/quote/theme-rtl.css 223 B
build/block-library/blocks/quote/theme.css 226 B
build/block-library/blocks/rss/editor-rtl.css 202 B
build/block-library/blocks/rss/editor.css 204 B
build/block-library/blocks/rss/style-rtl.css 289 B
build/block-library/blocks/rss/style.css 288 B
build/block-library/blocks/search/editor-rtl.css 165 B
build/block-library/blocks/search/editor.css 165 B
build/block-library/blocks/search/style-rtl.css 397 B
build/block-library/blocks/search/style.css 398 B
build/block-library/blocks/search/theme-rtl.css 64 B
build/block-library/blocks/search/theme.css 64 B
build/block-library/blocks/separator/editor-rtl.css 99 B
build/block-library/blocks/separator/editor.css 99 B
build/block-library/blocks/separator/style-rtl.css 250 B
build/block-library/blocks/separator/style.css 250 B
build/block-library/blocks/separator/theme-rtl.css 172 B
build/block-library/blocks/separator/theme.css 172 B
build/block-library/blocks/shortcode/editor-rtl.css 474 B
build/block-library/blocks/shortcode/editor.css 474 B
build/block-library/blocks/site-logo/editor-rtl.css 770 B
build/block-library/blocks/site-logo/editor.css 770 B
build/block-library/blocks/site-logo/style-rtl.css 165 B
build/block-library/blocks/site-logo/style.css 165 B
build/block-library/blocks/site-tagline/editor-rtl.css 86 B
build/block-library/blocks/site-tagline/editor.css 86 B
build/block-library/blocks/site-title/editor-rtl.css 84 B
build/block-library/blocks/site-title/editor.css 84 B
build/block-library/blocks/social-link/editor-rtl.css 177 B
build/block-library/blocks/social-link/editor.css 177 B
build/block-library/blocks/social-links/editor-rtl.css 824 B
build/block-library/blocks/social-links/editor.css 823 B
build/block-library/blocks/social-links/style-rtl.css 1.32 kB
build/block-library/blocks/social-links/style.css 1.32 kB
build/block-library/blocks/spacer/editor-rtl.css 307 B
build/block-library/blocks/spacer/editor.css 307 B
build/block-library/blocks/spacer/style-rtl.css 48 B
build/block-library/blocks/spacer/style.css 48 B
build/block-library/blocks/table/editor-rtl.css 471 B
build/block-library/blocks/table/editor.css 472 B
build/block-library/blocks/table/style-rtl.css 481 B
build/block-library/blocks/table/style.css 481 B
build/block-library/blocks/table/theme-rtl.css 188 B
build/block-library/blocks/table/theme.css 188 B
build/block-library/blocks/tag-cloud/style-rtl.css 146 B
build/block-library/blocks/tag-cloud/style.css 146 B
build/block-library/blocks/template-part/editor-rtl.css 560 B
build/block-library/blocks/template-part/editor.css 559 B
build/block-library/blocks/template-part/theme-rtl.css 101 B
build/block-library/blocks/template-part/theme.css 101 B
build/block-library/blocks/text-columns/editor-rtl.css 95 B
build/block-library/blocks/text-columns/editor.css 95 B
build/block-library/blocks/text-columns/style-rtl.css 166 B
build/block-library/blocks/text-columns/style.css 166 B
build/block-library/blocks/verse/style-rtl.css 87 B
build/block-library/blocks/verse/style.css 87 B
build/block-library/blocks/video/editor-rtl.css 571 B
build/block-library/blocks/video/editor.css 572 B
build/block-library/blocks/video/style-rtl.css 173 B
build/block-library/blocks/video/style.css 173 B
build/block-library/blocks/video/theme-rtl.css 124 B
build/block-library/blocks/video/theme.css 124 B
build/block-library/common-rtl.css 815 B
build/block-library/common.css 812 B
build/block-library/reset-rtl.css 474 B
build/block-library/reset.css 474 B
build/block-library/style-rtl.css 10.5 kB
build/block-library/style.css 10.6 kB
build/block-library/theme-rtl.css 668 B
build/block-library/theme.css 673 B
build/block-serialization-default-parser/index.min.js 1.09 kB
build/block-serialization-spec-parser/index.min.js 2.79 kB
build/blocks/index.min.js 46 kB
build/components/index.min.js 212 kB
build/components/style-rtl.css 15.4 kB
build/components/style.css 15.4 kB
build/compose/index.min.js 10.9 kB
build/core-data/index.min.js 12.6 kB
build/customize-widgets/index.min.js 11.2 kB
build/customize-widgets/style-rtl.css 1.5 kB
build/customize-widgets/style.css 1.49 kB
build/data-controls/index.min.js 614 B
build/data/index.min.js 7.1 kB
build/date/index.min.js 31.5 kB
build/deprecated/index.min.js 428 B
build/dom-ready/index.min.js 304 B
build/dom/index.min.js 4.44 kB
build/edit-navigation/index.min.js 15.8 kB
build/edit-navigation/style-rtl.css 3.76 kB
build/edit-navigation/style.css 3.76 kB
build/edit-post/classic-rtl.css 492 B
build/edit-post/classic.css 494 B
build/edit-post/index.min.js 29.4 kB
build/edit-post/style-rtl.css 7.12 kB
build/edit-post/style.css 7.12 kB
build/edit-site/index.min.js 30.7 kB
build/edit-site/style-rtl.css 5.79 kB
build/edit-site/style.css 5.79 kB
build/edit-widgets/index.min.js 16.3 kB
build/edit-widgets/style-rtl.css 4.17 kB
build/edit-widgets/style.css 4.18 kB
build/editor/index.min.js 37.7 kB
build/editor/style-rtl.css 3.78 kB
build/editor/style.css 3.77 kB
build/element/index.min.js 3.21 kB
build/escape-html/index.min.js 517 B
build/format-library/index.min.js 6.34 kB
build/format-library/style-rtl.css 571 B
build/format-library/style.css 571 B
build/hooks/index.min.js 1.55 kB
build/html-entities/index.min.js 424 B
build/i18n/index.min.js 3.6 kB
build/is-shallow-equal/index.min.js 501 B
build/keyboard-shortcuts/index.min.js 1.72 kB
build/keycodes/index.min.js 1.3 kB
build/list-reusable-blocks/index.min.js 1.85 kB
build/list-reusable-blocks/style-rtl.css 838 B
build/list-reusable-blocks/style.css 838 B
build/media-utils/index.min.js 2.92 kB
build/notices/index.min.js 845 B
build/nux/index.min.js 2.03 kB
build/nux/style-rtl.css 747 B
build/nux/style.css 743 B
build/plugins/index.min.js 1.83 kB
build/primitives/index.min.js 921 B
build/priority-queue/index.min.js 582 B
build/react-i18n/index.min.js 671 B
build/redux-routine/index.min.js 2.63 kB
build/reusable-blocks/index.min.js 2.19 kB
build/reusable-blocks/style-rtl.css 256 B
build/reusable-blocks/style.css 256 B
build/rich-text/index.min.js 10.7 kB
build/server-side-render/index.min.js 1.52 kB
build/shortcode/index.min.js 1.48 kB
build/token-list/index.min.js 562 B
build/url/index.min.js 1.74 kB
build/viewport/index.min.js 1.02 kB
build/warning/index.min.js 248 B
build/widgets/index.min.js 7.11 kB
build/widgets/style-rtl.css 1.16 kB
build/widgets/style.css 1.16 kB
build/wordcount/index.min.js 1.04 kB

compressed-size-action

@talldan
Copy link
Contributor Author

talldan commented Oct 28, 2021

This should now be working as expected. The fix for entities has been separated out into #36027, but is also included here for testing.

@talldan talldan force-pushed the add/delete-nav-menu-button-to-nav-block branch from 92a4d3e to 25199fa Compare October 28, 2021 09:21
@talldan talldan force-pushed the add/delete-nav-menu-button-to-nav-block branch from 25199fa to 7bebfcd Compare October 29, 2021 05:06
@talldan talldan force-pushed the add/delete-nav-menu-button-to-nav-block branch from 7bebfcd to 718c855 Compare October 29, 2021 07:39
@talldan talldan marked this pull request as ready for review October 29, 2021 07:48
@talldan
Copy link
Contributor Author

talldan commented Oct 29, 2021

The issue with the entity saving process has a temporary fix now, so this is ready for review. It would be good to get in for 5.9.

@jasmussen
Copy link
Contributor

Thank you for the PR. The ability to delete an automatically or manually created menu is nice:
navigation

I do have some concern about the prominence of the panel, and saving feature itself. I had hoped that changing the storage system would have no visible effect on the user experience.

The balance to find here is tricky. If we autosave menus as your other PR does, it eliminates a big point of friction to using the block and gets us closer to the "no visible effect" experience, but there would still be a proliferation of patterns. Adding a delete button would give a way to manage that, but the less we'd need to manage, the better. One solution idea of managing prominence could be to put a delete button directly in the menu selection dropdown, appearing when hovering a non-selected navigation:
delete

Another option is to restore the CPT management screen and add a link as we do for Reusable blocks:
Screenshot 2021-10-29 at 10 07 46

@talldan
Copy link
Contributor Author

talldan commented Oct 29, 2021

@jasmussen There's an issue for restoring the admin page, but it isn't straightforward (#36036).

If that doesn't land, users will need a way to delete menus. I did wonder about a separate modal for managing menus as another idea.

One solution idea of managing prominence could be to put a delete button directly in the menu selection dropdown, appearing when hovering a non-selected navigation:

I don't think this would be accessible. Using the keyboard, this menu would need to work as a grid, which unfortunately breaks the accessible menu concept.

Any other thoughts? I don't a huge amount of time to iterate on this, so the simpler solutions are better. Could we lower the prominence of the button somehow?

@jasmussen
Copy link
Contributor

I think it's unfortunate that the inspector panel pushes down the visual appearance controls that are so important for the navigation block. I think a tool to delete menus might get buried if put under the "Advanced" collapsed-by-default panel, and my best other instinct, as noted, would be to have an item in the ellipsis menu next to Reusable blocks. If that needs to open a modal, perhaps that's a way forward.

I know you feel these mockups might be a mouthful, but part of what they do by embracing the multi entity saving flow is to both reduce the weight of having to think about saving, but also in doing it only when you're done with the page, minimizing the potential proliferation of saved menus, reducing the need to delete them.

Is there anything we can do to simplify that flow, but keep the principle the same? The placeholder plus selection dropdown is already in a good place per Isabels fast work, we don't need the snackbar, we can omit the "Unsaved" button from the toolbar until the menu has been saved, and we don't need the ability to rename menus in the multi entity saving flow. In my mind that means all that's left is to remove the modal to name the menu, and have the saving happen only as part of the multi entity saving step. Do you think that would be feasible?

@talldan
Copy link
Contributor Author

talldan commented Oct 29, 2021

I think it's unfortunate that the inspector panel pushes down the visual appearance controls that are so important for the navigation block. I think a tool to delete menus might get buried if put under the "Advanced" collapsed-by-default panel, and my best other instinct, as noted, would be to have an item in the ellipsis menu next to Reusable blocks. If that needs to open a modal, perhaps that's a way forward.

I'd be concerned 'Advanced' is too buried. Do you have the same feeling about the 'menu name' part of the panel Maybe we need a more concise version of the whole thing that doesn't push the other options down so far. Something like this?
Screenshot 2021-10-29 at 5 09 08 pm

Screenshot isn't this clear, but this would be a row in the inspector underneath the block details. The icon on the right could open a dropdown that would have 'rename' and 'delete' options.

To be honest, some of these smaller details have been hard to focus on while we're changing the big picture. But if we can come up with feasible suggestions for them in the extremely limited time we have, there's no reason we can't improve them.

@jasmussen
Copy link
Contributor

To be honest, some of these smaller details have been hard to focus on while we're changing the big picture. But if we can come up with feasible suggestions for them in the extremely limited time we have, there's no reason we can't improve them.

I keep coming back to the solution existing in the existing save buttons:
Screenshot 2021-10-29 at 11 20 18

As someone using the block editor, I shouldn't have to worry about my content being saved, or even about multiple different ways to save things. This is ideally complexity we handle for you. And so embracing that existing flow seems like it should be able to reduce the work we have on our table.

@talldan
Copy link
Contributor Author

talldan commented Oct 29, 2021

I don't really understand how saving helps a user delete a menu?

@jasmussen
Copy link
Contributor

Saving only once when you're done editing the page means you end up with fewer overall, reducing the urgency for a delete button, at the very least it reduces the need for it to be so prominent.

@talldan
Copy link
Contributor Author

talldan commented Oct 29, 2021

Saving only once when you're done editing the page means you end up with fewer overall, reducing the urgency for a delete button, at the very least it reduces the need for it to be so prominent.

Right, but it doesn't seem like it removes the need.

@jasmussen
Copy link
Contributor

No no, but it means it might be fine to make it a tiny button at the end of the menu, maybe?

mock

@talldan
Copy link
Contributor Author

talldan commented Oct 29, 2021

My feeling is that renaming and deleting a menu aren't actions that are only for advanced users. They are, whether we like it or not, something many users will want/need to do. For example, people make typos. Or they accidentally create something and then realise they don't need it.

Maybe we don't quite give the features the priority they have right now, but I do think they need to be accessible, not too hidden away.

Copy link
Contributor

@draganescu draganescu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way I see it is that we're working on new foundational features without a UX designed. So we approximate. And when we approximate we get feedback as complex flows that reroute all work to new horizons instead of allowing us to make the next step. That means we can postpone the realisation of these designs to future iterations. Future iterations thay may very well be next week ;D

This is how the entire project moves, I don't see how this particular thing has to be different.

In this case, we can't work with independent menus that can't be deleted. Hopefully once the editing in isolation will crystalize the need to delete from the block will vanish or will be moveable to a secondary place. For now adding a button is a good stepping stone allowing us to figure out the lay of the land in the UX of managing reusable menus.

Maybe in the end we realise that the block theme system doesn't encuorage users to create useles superfluous menus, because of the direct manipulation UX. Then also the delete can be relegated to some accordion at the end of the inspector. But we don't know.

I'd go ahead with this PR so we can have something to move around later but also something to work with now.

@talldan
Copy link
Contributor Author

talldan commented Nov 1, 2021

I'll merge this one. If there's an alternative design that's relatively quick to achieve and doesn't hide the feature away too much, then I'm happy to look at it.

@talldan talldan merged commit 0b609cf into trunk Nov 1, 2021
@talldan talldan deleted the add/delete-nav-menu-button-to-nav-block branch November 1, 2021 08:20
@github-actions github-actions bot added this to the Gutenberg 11.9 milestone Nov 1, 2021
@andrewserong andrewserong changed the title Add delete nav menu button to nav block Navigation: Add delete nav menu button Nov 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Navigation Affects the Navigation Block [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants