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

Site Editor (Experiment): Automatically open the sidebar to the appropriate menu #26964

Merged
merged 8 commits into from
Mar 24, 2021

Conversation

Copons
Copy link
Contributor

@Copons Copons commented Nov 13, 2020

Description

Fixes #26924

Clicking on the navigation toggle button will open the navigation sidebar to the appropriate menu, either Templates or Template Parts, depending on the template type that is being currently edited.

How has this been tested?

  • Open the Site Editor.
  • Click on the navigation toggle button (WP icon / site logo).
  • Make sure the sidebar opens to the "Templates" menu.
  • Navigate back to "Themes" and then into "Template Parts". Click a template part to focus on that.
  • Close and reopen the sidebar with the navigation toggle button.
  • Make sure the sidebar opens to the "Template Parts" menu.

Screenshots

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.
  • 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.

@github-actions
Copy link

github-actions bot commented Nov 13, 2020

Size Change: +8.46 kB (+1%)

Total Size: 1.41 MB

Filename Size Change
build/annotations/index.js 3.77 kB -2 B (0%)
build/api-fetch/index.js 3.4 kB -2 B (0%)
build/autop/index.js 2.82 kB -2 B (0%)
build/block-directory/index.js 8.63 kB -3 B (0%)
build/block-editor/index.js 127 kB +363 B (0%)
build/block-editor/style-rtl.css 12.4 kB +4 B (0%)
build/block-editor/style.css 12.4 kB +6 B (0%)
build/block-library/blocks/file/editor-rtl.css 175 B -24 B (-12%) 👏
build/block-library/blocks/file/editor.css 174 B -24 B (-12%) 👏
build/block-library/blocks/freeform/editor-rtl.css 2.45 kB -9 B (0%)
build/block-library/blocks/freeform/editor.css 2.45 kB -9 B (0%)
build/block-library/blocks/list/editor-rtl.css 0 B -65 B (removed) 🏆
build/block-library/blocks/list/editor.css 0 B -65 B (removed) 🏆
build/block-library/blocks/navigation-link/editor-rtl.css 634 B +8 B (+1%)
build/block-library/blocks/navigation-link/editor.css 635 B +8 B (+1%)
build/block-library/blocks/navigation-link/style-rtl.css 897 B +212 B (+31%) 🚨
build/block-library/blocks/navigation-link/style.css 895 B +213 B (+31%) 🚨
build/block-library/blocks/navigation/editor-rtl.css 1.12 kB +10 B (+1%)
build/block-library/blocks/navigation/editor.css 1.13 kB +11 B (+1%)
build/block-library/blocks/page-list/style-rtl.css 167 B -370 B (-69%) 🏆
build/block-library/blocks/page-list/style.css 167 B -369 B (-69%) 🏆
build/block-library/editor-rtl.css 9.44 kB -27 B (0%)
build/block-library/editor.css 9.45 kB -26 B (0%)
build/block-library/index.js 148 kB +656 B (0%)
build/block-library/style-rtl.css 8.95 kB +67 B (+1%)
build/block-library/style.css 8.95 kB +66 B (+1%)
build/block-serialization-default-parser/index.js 1.87 kB -2 B (0%)
build/blocks/index.js 48.3 kB +1 B (0%)
build/components/index.js 284 kB -11 B (0%)
build/components/style-rtl.css 16.2 kB +7 B (0%)
build/components/style.css 16.2 kB +8 B (0%)
build/compose/index.js 11.2 kB +62 B (+1%)
build/core-data/index.js 16.7 kB +2 B (0%)
build/customize-widgets/index.js 6.03 kB +46 B (+1%)
build/data-controls/index.js 830 B -1 B (0%)
build/data/index.js 8.87 kB +3 B (0%)
build/date/index.js 31.9 kB +21 B (0%)
build/dom/index.js 4.98 kB +8 B (0%)
build/edit-navigation/index.js 17 kB +5.09 kB (+43%) 🚨
build/edit-navigation/style-rtl.css 2.7 kB +1.29 kB (+91%) 🆘
build/edit-navigation/style.css 2.7 kB +1.29 kB (+92%) 🆘
build/edit-post/index.js 307 kB -323 B (0%)
build/edit-post/style-rtl.css 7.05 kB -56 B (-1%)
build/edit-post/style.css 7.04 kB -50 B (-1%)
build/edit-site/index.js 27.5 kB +308 B (+1%)
build/edit-site/style-rtl.css 4.51 kB -43 B (-1%)
build/edit-site/style.css 4.5 kB -42 B (-1%)
build/edit-widgets/index.js 20.2 kB +43 B (0%)
build/edit-widgets/style-rtl.css 3.15 kB -36 B (-1%)
build/edit-widgets/style.css 3.15 kB -39 B (-1%)
build/editor/index.js 41.9 kB +72 B (0%)
build/element/index.js 4.61 kB +5 B (0%)
build/format-library/index.js 6.75 kB +1 B (0%)
build/hooks/index.js 2.28 kB +3 B (0%)
build/keyboard-shortcuts/index.js 2.53 kB +2 B (0%)
build/keycodes/index.js 1.95 kB +1 B (0%)
build/list-reusable-blocks/index.js 3.19 kB +47 B (+1%)
build/media-utils/index.js 5.38 kB +39 B (+1%)
build/nux/index.js 3.4 kB -2 B (0%)
build/plugins/index.js 2.95 kB +60 B (+2%)
build/react-i18n/index.js 1.45 kB -1 B (0%)
build/redux-routine/index.js 2.84 kB +1 B (0%)
build/reusable-blocks/index.js 3.78 kB -1 B (0%)
build/rich-text/index.js 13.4 kB +41 B (0%)
build/token-list/index.js 1.27 kB -3 B (0%)
build/url/index.js 3.02 kB -4 B (0%)
build/viewport/index.js 1.86 kB -2 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/blob/index.js 664 B 0 B
build/block-directory/style-rtl.css 1 kB 0 B
build/block-directory/style.css 1.01 kB 0 B
build/block-library/blocks/archives/editor-rtl.css 61 B 0 B
build/block-library/blocks/archives/editor.css 60 B 0 B
build/block-library/blocks/audio/editor-rtl.css 58 B 0 B
build/block-library/blocks/audio/editor.css 58 B 0 B
build/block-library/blocks/audio/style-rtl.css 112 B 0 B
build/block-library/blocks/audio/style.css 112 B 0 B
build/block-library/blocks/block/editor-rtl.css 161 B 0 B
build/block-library/blocks/block/editor.css 161 B 0 B
build/block-library/blocks/button/editor-rtl.css 475 B 0 B
build/block-library/blocks/button/editor.css 474 B 0 B
build/block-library/blocks/button/style-rtl.css 479 B 0 B
build/block-library/blocks/button/style.css 479 B 0 B
build/block-library/blocks/buttons/editor-rtl.css 315 B 0 B
build/block-library/blocks/buttons/editor.css 315 B 0 B
build/block-library/blocks/buttons/style-rtl.css 364 B 0 B
build/block-library/blocks/buttons/style.css 363 B 0 B
build/block-library/blocks/calendar/style-rtl.css 208 B 0 B
build/block-library/blocks/calendar/style.css 208 B 0 B
build/block-library/blocks/categories/editor-rtl.css 84 B 0 B
build/block-library/blocks/categories/editor.css 83 B 0 B
build/block-library/blocks/categories/style-rtl.css 79 B 0 B
build/block-library/blocks/categories/style.css 79 B 0 B
build/block-library/blocks/code/style-rtl.css 90 B 0 B
build/block-library/blocks/code/style.css 90 B 0 B
build/block-library/blocks/columns/editor-rtl.css 190 B 0 B
build/block-library/blocks/columns/editor.css 190 B 0 B
build/block-library/blocks/columns/style-rtl.css 421 B 0 B
build/block-library/blocks/columns/style.css 421 B 0 B
build/block-library/blocks/cover/editor-rtl.css 605 B 0 B
build/block-library/blocks/cover/editor.css 605 B 0 B
build/block-library/blocks/cover/style-rtl.css 1.24 kB 0 B
build/block-library/blocks/cover/style.css 1.24 kB 0 B
build/block-library/blocks/embed/editor-rtl.css 486 B 0 B
build/block-library/blocks/embed/editor.css 486 B 0 B
build/block-library/blocks/embed/style-rtl.css 401 B 0 B
build/block-library/blocks/embed/style.css 400 B 0 B
build/block-library/blocks/file/style-rtl.css 248 B 0 B
build/block-library/blocks/file/style.css 248 B 0 B
build/block-library/blocks/gallery/editor-rtl.css 704 B 0 B
build/block-library/blocks/gallery/editor.css 705 B 0 B
build/block-library/blocks/gallery/style-rtl.css 1.11 kB 0 B
build/block-library/blocks/gallery/style.css 1.1 kB 0 B
build/block-library/blocks/group/editor-rtl.css 160 B 0 B
build/block-library/blocks/group/editor.css 160 B 0 B
build/block-library/blocks/group/style-rtl.css 57 B 0 B
build/block-library/blocks/group/style.css 57 B 0 B
build/block-library/blocks/heading/editor-rtl.css 129 B 0 B
build/block-library/blocks/heading/editor.css 129 B 0 B
build/block-library/blocks/heading/style-rtl.css 76 B 0 B
build/block-library/blocks/heading/style.css 76 B 0 B
build/block-library/blocks/html/editor-rtl.css 281 B 0 B
build/block-library/blocks/html/editor.css 281 B 0 B
build/block-library/blocks/image/editor-rtl.css 717 B 0 B
build/block-library/blocks/image/editor.css 716 B 0 B
build/block-library/blocks/image/style-rtl.css 476 B 0 B
build/block-library/blocks/image/style.css 478 B 0 B
build/block-library/blocks/latest-comments/editor-rtl.css 159 B 0 B
build/block-library/blocks/latest-comments/editor.css 158 B 0 B
build/block-library/blocks/latest-comments/style-rtl.css 269 B 0 B
build/block-library/blocks/latest-comments/style.css 269 B 0 B
build/block-library/blocks/latest-posts/editor-rtl.css 137 B 0 B
build/block-library/blocks/latest-posts/editor.css 137 B 0 B
build/block-library/blocks/latest-posts/style-rtl.css 523 B 0 B
build/block-library/blocks/latest-posts/style.css 522 B 0 B
build/block-library/blocks/list/style-rtl.css 63 B 0 B
build/block-library/blocks/list/style.css 63 B 0 B
build/block-library/blocks/media-text/editor-rtl.css 191 B 0 B
build/block-library/blocks/media-text/editor.css 191 B 0 B
build/block-library/blocks/media-text/style-rtl.css 535 B 0 B
build/block-library/blocks/media-text/style.css 532 B 0 B
build/block-library/blocks/more/editor-rtl.css 434 B 0 B
build/block-library/blocks/more/editor.css 434 B 0 B
build/block-library/blocks/navigation/style-rtl.css 204 B 0 B
build/block-library/blocks/navigation/style.css 205 B 0 B
build/block-library/blocks/nextpage/editor-rtl.css 395 B 0 B
build/block-library/blocks/nextpage/editor.css 395 B 0 B
build/block-library/blocks/page-list/editor-rtl.css 170 B 0 B
build/block-library/blocks/page-list/editor.css 170 B 0 B
build/block-library/blocks/paragraph/editor-rtl.css 157 B 0 B
build/block-library/blocks/paragraph/editor.css 157 B 0 B
build/block-library/blocks/paragraph/style-rtl.css 247 B 0 B
build/block-library/blocks/paragraph/style.css 248 B 0 B
build/block-library/blocks/post-author/editor-rtl.css 209 B 0 B
build/block-library/blocks/post-author/editor.css 209 B 0 B
build/block-library/blocks/post-author/style-rtl.css 183 B 0 B
build/block-library/blocks/post-author/style.css 184 B 0 B
build/block-library/blocks/post-comments-form/style-rtl.css 250 B 0 B
build/block-library/blocks/post-comments-form/style.css 250 B 0 B
build/block-library/blocks/post-content/editor-rtl.css 139 B 0 B
build/block-library/blocks/post-content/editor.css 139 B 0 B
build/block-library/blocks/post-excerpt/editor-rtl.css 73 B 0 B
build/block-library/blocks/post-excerpt/editor.css 73 B 0 B
build/block-library/blocks/post-featured-image/editor-rtl.css 338 B 0 B
build/block-library/blocks/post-featured-image/editor.css 338 B 0 B
build/block-library/blocks/post-featured-image/style-rtl.css 100 B 0 B
build/block-library/blocks/post-featured-image/style.css 100 B 0 B
build/block-library/blocks/preformatted/style-rtl.css 63 B 0 B
build/block-library/blocks/preformatted/style.css 63 B 0 B
build/block-library/blocks/pullquote/editor-rtl.css 183 B 0 B
build/block-library/blocks/pullquote/editor.css 183 B 0 B
build/block-library/blocks/pullquote/style-rtl.css 318 B 0 B
build/block-library/blocks/pullquote/style.css 318 B 0 B
build/block-library/blocks/query-loop/editor-rtl.css 83 B 0 B
build/block-library/blocks/query-loop/editor.css 82 B 0 B
build/block-library/blocks/query-loop/style-rtl.css 315 B 0 B
build/block-library/blocks/query-loop/style.css 317 B 0 B
build/block-library/blocks/query-pagination-numbers/editor-rtl.css 122 B 0 B
build/block-library/blocks/query-pagination-numbers/editor.css 121 B 0 B
build/block-library/blocks/query-pagination/editor-rtl.css 270 B 0 B
build/block-library/blocks/query-pagination/editor.css 262 B 0 B
build/block-library/blocks/query-pagination/style-rtl.css 168 B 0 B
build/block-library/blocks/query-pagination/style.css 168 B 0 B
build/block-library/blocks/query-title/editor-rtl.css 86 B 0 B
build/block-library/blocks/query-title/editor.css 86 B 0 B
build/block-library/blocks/query/editor-rtl.css 795 B 0 B
build/block-library/blocks/query/editor.css 794 B 0 B
build/block-library/blocks/quote/editor-rtl.css 61 B 0 B
build/block-library/blocks/quote/editor.css 61 B 0 B
build/block-library/blocks/quote/style-rtl.css 169 B 0 B
build/block-library/blocks/quote/style.css 169 B 0 B
build/block-library/blocks/rss/editor-rtl.css 201 B 0 B
build/block-library/blocks/rss/editor.css 202 B 0 B
build/block-library/blocks/rss/style-rtl.css 290 B 0 B
build/block-library/blocks/rss/style.css 290 B 0 B
build/block-library/blocks/search/editor-rtl.css 165 B 0 B
build/block-library/blocks/search/editor.css 165 B 0 B
build/block-library/blocks/search/style-rtl.css 342 B 0 B
build/block-library/blocks/search/style.css 344 B 0 B
build/block-library/blocks/separator/editor-rtl.css 99 B 0 B
build/block-library/blocks/separator/editor.css 99 B 0 B
build/block-library/blocks/separator/style-rtl.css 236 B 0 B
build/block-library/blocks/separator/style.css 236 B 0 B
build/block-library/blocks/shortcode/editor-rtl.css 512 B 0 B
build/block-library/blocks/shortcode/editor.css 512 B 0 B
build/block-library/blocks/site-logo/editor-rtl.css 201 B 0 B
build/block-library/blocks/site-logo/editor.css 201 B 0 B
build/block-library/blocks/site-logo/style-rtl.css 115 B 0 B
build/block-library/blocks/site-logo/style.css 115 B 0 B
build/block-library/blocks/social-link/editor-rtl.css 164 B 0 B
build/block-library/blocks/social-link/editor.css 165 B 0 B
build/block-library/blocks/social-links/editor-rtl.css 776 B 0 B
build/block-library/blocks/social-links/editor.css 776 B 0 B
build/block-library/blocks/social-links/style-rtl.css 1.32 kB 0 B
build/block-library/blocks/social-links/style.css 1.33 kB 0 B
build/block-library/blocks/spacer/editor-rtl.css 317 B 0 B
build/block-library/blocks/spacer/editor.css 317 B 0 B
build/block-library/blocks/spacer/style-rtl.css 48 B 0 B
build/block-library/blocks/spacer/style.css 48 B 0 B
build/block-library/blocks/table/editor-rtl.css 478 B 0 B
build/block-library/blocks/table/editor.css 478 B 0 B
build/block-library/blocks/table/style-rtl.css 402 B 0 B
build/block-library/blocks/table/style.css 402 B 0 B
build/block-library/blocks/tag-cloud/editor-rtl.css 118 B 0 B
build/block-library/blocks/tag-cloud/editor.css 118 B 0 B
build/block-library/blocks/tag-cloud/style-rtl.css 94 B 0 B
build/block-library/blocks/tag-cloud/style.css 94 B 0 B
build/block-library/blocks/template-part/editor-rtl.css 552 B 0 B
build/block-library/blocks/template-part/editor.css 551 B 0 B
build/block-library/blocks/term-description/editor-rtl.css 90 B 0 B
build/block-library/blocks/term-description/editor.css 90 B 0 B
build/block-library/blocks/text-columns/editor-rtl.css 95 B 0 B
build/block-library/blocks/text-columns/editor.css 95 B 0 B
build/block-library/blocks/text-columns/style-rtl.css 166 B 0 B
build/block-library/blocks/text-columns/style.css 166 B 0 B
build/block-library/blocks/verse/editor-rtl.css 50 B 0 B
build/block-library/blocks/verse/editor.css 50 B 0 B
build/block-library/blocks/verse/style-rtl.css 87 B 0 B
build/block-library/blocks/verse/style.css 87 B 0 B
build/block-library/blocks/video/editor-rtl.css 504 B 0 B
build/block-library/blocks/video/editor.css 503 B 0 B
build/block-library/blocks/video/style-rtl.css 187 B 0 B
build/block-library/blocks/video/style.css 187 B 0 B
build/block-library/common-rtl.css 1.1 kB 0 B
build/block-library/common.css 1.1 kB 0 B
build/block-library/reset-rtl.css 374 B 0 B
build/block-library/reset.css 376 B 0 B
build/block-library/theme-rtl.css 700 B 0 B
build/block-library/theme.css 701 B 0 B
build/block-serialization-spec-parser/index.js 3.06 kB 0 B
build/customize-widgets/style-rtl.css 378 B 0 B
build/customize-widgets/style.css 379 B 0 B
build/deprecated/index.js 787 B 0 B
build/dom-ready/index.js 577 B 0 B
build/editor/style-rtl.css 3.9 kB 0 B
build/editor/style.css 3.9 kB 0 B
build/escape-html/index.js 735 B 0 B
build/format-library/style-rtl.css 637 B 0 B
build/format-library/style.css 639 B 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 4.01 kB 0 B
build/is-shallow-equal/index.js 699 B 0 B
build/list-reusable-blocks/style-rtl.css 629 B 0 B
build/list-reusable-blocks/style.css 628 B 0 B
build/notices/index.js 1.85 kB 0 B
build/nux/style-rtl.css 731 B 0 B
build/nux/style.css 727 B 0 B
build/primitives/index.js 1.42 kB 0 B
build/priority-queue/index.js 791 B 0 B
build/reusable-blocks/style-rtl.css 225 B 0 B
build/reusable-blocks/style.css 225 B 0 B
build/server-side-render/index.js 2.58 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.22 kB 0 B

compressed-size-action

Copy link
Contributor

@Addison-Stavlo Addison-Stavlo left a comment

Choose a reason for hiding this comment

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

The code looks good and it functions like I would expect (I think?)... although it feels very weird and a bit heavy handed.

Im editing a Template, I want to open the sidebar to either select something else to edit or go back to the dashboard. It immediately opens to the 'Templates' menu. This would make sense if the menu had editing options for the entity I am working on, but its just a selection interface. Why assume the next thing I want to select is a different template to edit vs. a template part or going back to the dashboard? This doesn't make sense to me.

So now the panel always opens to a nested level. Even if I open it, go back one level, close it, and re-open it, it still opens to that nested level again. This is also awkward. The drill-down pattern is never used starting from the top-level anymore.

@jameskoster
Copy link
Contributor

jameskoster commented Nov 13, 2020

Why assume the next thing I want to select is a different template to edit vs. a template part or going back to the dashboard?

Presenting the Templates panel makes it easy to continue editing templates. Decidedly easier than having to navigate all the way back to the panel manually, especially when it might be nested 2 or 3 levels deep in the future (e.g. Theme > Templates > List).

It is much easier to drill up (to do something else entirely) than down because:

  1. The back button is always in the exact same location and behaves the exact same way.
  2. Zero cognitive load working out where something might live in the iA.

It therefore makes sense to present a user with adjacent tasks rather than dropping them at the root.

This is just my opinion, so I'd love to get thoughts from other designers.

@Addison-Stavlo
Copy link
Contributor

Addison-Stavlo commented Nov 13, 2020

Presenting the Templates panel makes it easy to continue editing templates.

True, but just because the editor initially loads with one entity type to edit does not imply that is what the user is necessarily there for. Similarly, just because a user is done editing a template does not imply the next thing they need to edit is also a template.

It is much easier to drill up (to do something else entirely) than down because

Good point.

easier than having to navigate all the way back to the panel manually, especially when it might be nested 2 or 3 levels deep in the future (e.g. Theme > Templates > List).

There is still the 'browse all templates' button that leads directly to that menu.

@MichaelArestad
Copy link
Contributor

MichaelArestad commented Nov 13, 2020

Presenting the Templates panel makes it easy to continue editing templates. Decidedly easier than having to navigate all the way back to the panel manually, especially when it might be nested 2 or 3 levels deep in the future (e.g. Theme > Templates > List).

It is much easier to drill up (to do something else entirely) than down

Right. I agree. Thinking about this in only the templates context is a little tricky because it's very possible that folks are going to want to head to their dashboard after modifying a template. That's not going to be the case for many other sections or even the current behavior of the Post editor. Currently, when you click the "W" logo in the post editor, you end up on the Posts view with the sidebar open to the Posts context.

A good example is how plugins may adapt this sidebar down the road. If I'm on a WooCommerce Orders page and want to navigate to WooCommerce's Settings page, I don't want to start at the highest level menu and keep drilling down. I want to click the "W" and then click WooCommer's Settings link in the sidebar. Does that make sense?

@Copons
Copy link
Contributor Author

Copons commented Nov 13, 2020

Throwing a random idea: what if we replaced the site title with the back to dashboard button?

Slice

@Addison-Stavlo
Copy link
Contributor

Addison-Stavlo commented Nov 13, 2020

Currently, when you click the "W" logo in the post editor, you end up on the Posts view with the sidebar open to the Posts context.

True, but you got to the post editor in the first place by choosing to edit posts. Currently, we are not choosing to edit any specific entity type when we select the site editor - it just loads in template context by default regardless of the users intention. I think this concept of starting on the 'appropriate menu' would make much more sense if we we actually had contextual entry points to select. Until that is the case, this is kind of funky.

If I'm on a WooCommerce Orders page and want to navigate to WooCommerce's Settings page, I don't want to start at the highest level menu and keep drilling down. I want to click the "W" and then click WooCommer's Settings link in the sidebar. Does that make sense?

Yeah, that makes a lot of sense 👍

@Addison-Stavlo
Copy link
Contributor

Going with the idea of automatically opening to the appropriate menu, how should this work on closing/re-opening the sidebar:

  • Users loads into editor (or selects a new entity to edit) - first time they open the sidebar it is on the 'appropriate menu' - "Menu A".
  • User interacts with the sidebar and explores other menus.
  • User closes the sidebar while on "Menu B", without ever selecting a new entity to edit.

Now, when the user re-opens the menu - Should it still be on "Menu B", or should it open back on "Menu A" ?

@jameskoster
Copy link
Contributor

jameskoster commented Nov 13, 2020

it just loads in template context by default

This may not always be the case – indeed I think it quite unlikely to be. I suspect that something like the Mosaic View would be a more practical destination for the Theme Editor link. On that note, this PR is also relevant to the conversation.

Now, when the user re-opens the menu - Should it still be on "Menu B", or should it open back on "Menu A" ?

That is a very good question, but not one that necessarily needs to be answered in the context of this PR. It feels like one piece of a more holistic discussion that needs to occur if/when this pattern is ever considered to replace the entire wp-admin navigation.

I must say that the nav panel "remembering" where you are sounds sensible in theory. But in practice I wonder if it may be more confusing due to the fact it relies on the user also remembering where they were for it to feel intuitive. I find my memory much less reliable than my computers, haha. There's absolutely no harm in trying it though!

@Addison-Stavlo
Copy link
Contributor

Addison-Stavlo commented Nov 13, 2020

This may not always be the case – indeed I think it quite unlikely to be.

I agree as well. However my point is this doesnt make much sense until that is the case. Furthermore, this implementation only supports the context of template and template parts, not pages, posts, etc. as those menus don't exist yet. So the implementation will need to be updated in the future to support what we really want to use it for anyways. That is easy to do, but considering the flow doesn't make sense without contextual entry points and that it would need to be updated in the future to support other menus anyways... does it really make sense to add this now vs. later?

That is a very good question, but not one that necessarily needs to be answered in the context of this PR. It feels like one piece of a more holistic discussion that needs to occur if/when this pattern is ever considered to replace the entire wp-admin navigation.

Im not sure what the wp-admin navigation has to do with it, maybe we have our wires crossed. The flow I am describing never leaves the site editor, just explores the navigation panel while inside of it. Either way, it's a simple implementation detail in this PR that could easily be adjusted in the future. Technically it just comes down to if we want to set the active menu a) every time we open the panel, or b) whenever the entityType context changes.

@jameskoster
Copy link
Contributor

does it really make sense to add this now vs. later?

This entire feature (theme/site editing) isn't even officially a beta yet, and I wouldn't consider it one until most of the pieces you describe exist. So I would say yes, if for no other reason than it crosses an item off the list.

Throwing a random idea: what if we replaced the site title with the back to dashboard button?

@Copons sorry I missed this earlier, the original plan was for the Site name to link to the frontend of the site, just like it does now in wp-admin.

@Addison-Stavlo
Copy link
Contributor

This entire feature (theme/site editing) isn't even officially a beta yet, and I wouldn't consider it one until most of the pieces you describe exist. So I would say yes, if for no other reason than it crosses an item off the list.

Fair enough. 👍

Base automatically changed from master to trunk March 1, 2021 15:44
@jameskoster
Copy link
Contributor

I think this is going to be quite important along with the sidebar menu organisation refinements we intend to make (#29150). I tried to build the PR and ran into some brick walls @Copons 🙏

@Copons
Copy link
Contributor Author

Copons commented Mar 19, 2021

Here you go @jameskoster, I've rebased this and it should work as intended now. 🙂

@jameskoster
Copy link
Contributor

Close @Copons :D When I open the sidebar while editing the Header template part I am taken to the "Template Parts" section, I should instead be taken to the "Headers" section within "Template Parts".

@Copons Copons force-pushed the update/fse-sidebar-initial-menu branch from e550189 to fd37406 Compare March 22, 2021 14:55
@Copons
Copy link
Contributor Author

Copons commented Mar 22, 2021

Close @Copons :D When I open the sidebar while editing the Header template part I am taken to the "Template Parts" section, I should instead be taken to the "Headers" section within "Template Parts".

@jameskoster I've updated the PR as requested (definitely missed the whole discussion around it 😅).
Now, template parts open the panel into their corresponding sub-menu (which, for TT1 is just "General").

Templates, on the other hand, will keep opening their main menu.
Let me know if this is not desirable, and if you'd like to have them open their corresponsing sub-menu instead.
I haven't tried it, but I think it should be possible without too many headaches.

@jameskoster
Copy link
Contributor

No worries! Template parts are working well.

Templates, on the other hand, will keep opening their main menu.

I believe the idea is to be consistent in both cases. So if I go to Templates > General > page-home, when I next open the menu I should be looking at the Templates > General panel and see the page-home template with the active state.

@Copons
Copy link
Contributor Author

Copons commented Mar 22, 2021

@jameskoster I've opened a separate PR to handle the templates, as there's (much) more work involved: #30098
It mostly build up on @david-szabo97 past work, so I've added you two to review while it's WIP. 🙂

@david-szabo97
Copy link
Member

@Copons This one is good to go once the tests are fixed 🚀

@Copons
Copy link
Contributor Author

Copons commented Mar 23, 2021

Thanks @david-szabo97!
I've updated the tests mocking the new selector (and removing a mocked selector no longer in use).

@david-szabo97
Copy link
Member

I did more throughout testing this time. Noticed a weird behavior. If you click any item in Content submenu then close-open the browsing sidebar, then it shows both menus.

Screen.Recording.2021-03-23.at.18.00.15.mov

@david-szabo97
Copy link
Member

Something is still going on 🤔

  1. Click on "Content -> Pages -> Any page"
  2. Close browsing sidebar
  3. Open browsing sidebar
  4. (Now Templates should be open) Click on "< Theme"
  5. Content is shown without a back button 😕
Screen.Recording.2021-03-24.at.9.56.55.mov

@Copons
Copy link
Contributor Author

Copons commented Mar 24, 2021

Blurgh!
@david-szabo97 thanks for the thorough test and apologies for failing to notice those behaviours in the first place.

I think those issues were happening before this PR, but either way.
The problem was that the content menu was handled by the internal state, which meant it wasn't reset to MENU_ROOT on close. So it could get stuck on a sub-menu, messing up how it interacted with the navigation menu.
My solution was to reset it to root on menu open.
This should hopefully sort out all the issues!

Copy link
Member

@david-szabo97 david-szabo97 left a comment

Choose a reason for hiding this comment

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

🍾 🚢

@Copons Copons merged commit c773f40 into trunk Mar 24, 2021
@Copons Copons deleted the update/fse-sidebar-initial-menu branch March 24, 2021 11:15
@github-actions github-actions bot added this to the Gutenberg 10.3 milestone Mar 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Template Navigation: Should open to the "Templates" panel when editing a template
5 participants