-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Add a nested level when selecting templates or template parts #47777
Conversation
Size Change: -93 B (0%) Total Size: 1.33 MB
ℹ️ View Unchanged
|
Flaky tests detected in 85f8a69. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4162769894
|
Nice work, this has much clearer and more contextual edit buttons: I expect this will work well for Navigation as well. We are missing descriptive text in the drilldown sidebar on the template drilldown, and when you press Back from a particular template, it'd be nice to go back to the main Templates list, rather than the top level. But other than that, this feels promising. |
Nice! This feels pretty good to use :) Could we add a generic description for custom templates as a part of this PR? The panels for these templates look a bit empty. Perhaps something like:
It would be good to come up with something for template parts too, let me know if that's something we can include here and I'll noodle on that. In the future custom templates / template parts should be able to entertain custom descriptions but this should be fine to begin with. Could we add descriptions to the Templates / Templates Parts / Navigation panels too? If it's easy let's align the description with the site and panel title, and use $gray-600 for the color. Could we include the chevron indicator on the menu items? Ideally the small chevron variant (which we should use at the root level too). A couple of details that can be looked into separately:
|
I thought we could hide that menu in small screens entirely |
Do you mean hide the management view? We might temporarily, but that would prohibit bulk actions on mobile (once we've added them) so it's not a long-term solution. These views will also need to be accessible if they're ever going to fulfill a role outside of the site editor too. I was thinking we might do something like this: But it's certainly not something we need to handle in this PR. |
I noticed couple of other issues:
|
3218669
to
d653f92
Compare
What kind of design tweaks we can do while we wait for the navigator updates to land? |
Took it for a spin, it's working well. Here's a GIF going through the deeper nested drilldown menu: This is looking good. I don't think we necessarily need to block this PR from landing due to some of the small tweaks, but there are a couple of metrics we need to work out: I think we should use the 32px version of buttons for both the back button and the plus. I also wonder if we should show the plus below the list of templates, rather than fixed next to the title. With a black background and 24x24 footprint just like the in-canvas inserter. The heading is a bit big. I have it as a semibold 16px in some mockups I've explored: I know @jameskoster has some thoughs on this too, so it might not be the right moment to finesse the details quite yet, we can probably do that after the fact. Same with the deepest drilldown, we probably want to use the 32px button here: I find it a bit jarring that when I press "Back", I go to the top level instead of the higher level. I know there have been some component conversations here, but just for the record, here's what I'm seeing now:
|
Yeah, this is the navigator updates I'm talking about this comment #47777 (comment) we're making progress in #47883. Once it lands, I'll rebase this PR and it should solve the issue. For the button tweaks, they seem all kind of related, and hint to the introduction of a "new" button style in the sidebar, so maybe better to do in its own PR as well. |
Cool, do you need a green check? |
It helps but I'll have to wait for #47883 anyway :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This approach uxwise feels correct, and we'll explore polish separately.
Cool, thanks 🙏 I was a bit concerned about that, as skipping a level would be unexpected for all users and even more confusing for users with accessibility needs. Clarification for history: seems to me the skipping of the intermediate level occurs only when switching the editor to Edit mode. If I stay in browse mode, navigating back looks correct: Design ← Templates ← Front Page. There's a problem with the button aria-label though. Even if this gets fixed with #47883, I think the aria-label text is too complex. I do realize the good intent of providing more context but we need to find a compromise between non-sighted users and sighted users. For example, speech recognition software users would have a hard time figuring out the button accessible name is |
Personally I'd like to see a description added for all panels that do not include menus. The screenshot above is sub-optimal, there's nothing in the panel to clarify what I'm looking at except the name which can be a bit arbitrary. I'm happy to provide descriptions, or we can add placeholders and refine while polishing other details. The drilldown menu items are still missing a chevron. I suppose you can argue this is 'polish' but I'd say it has one foot in UX territory. The main outstanding issue: on the manage templates view clicking a template does nothing which feels like a pretty big regression, I'd consider this a blocker but happy to be overruled :) Imo we should either remove the link appearance, or make the links work (IE go directly to edit view). |
@ntsekouras Good call, that was a mistake. It should target 6.2 instead. |
@ntsekouras I'll follow up with a PR that updates these files |
Cherry-picked this PR to the wp/6.2 branch. |
…irect. Removes the server-side redirection if missing `postType` and `postId` query args when visiting Site Editor. Why? This redirect is no longer needed as the routing is now handled client side (via [55333]). References: * [WordPress/gutenberg#48023 Gutenberg PR 48023] * [WordPress/gutenberg#47777 Gutenberg PR 47777] Follow-up to [55333], [53413], [53093]. Props ntsekouras, youknowriad. Fixes #57716. git-svn-id: https://develop.svn.wordpress.org/trunk@55338 602fd350-edb4-49c9-b593-d223f7449a82
…irect. Removes the server-side redirection if missing `postType` and `postId` query args when visiting Site Editor. Why? This redirect is no longer needed as the routing is now handled client side (via [55333]). References: * [WordPress/gutenberg#48023 Gutenberg PR 48023] * [WordPress/gutenberg#47777 Gutenberg PR 47777] Follow-up to [55333], [53413], [53093]. Props ntsekouras, youknowriad. Fixes #57716. Built from https://develop.svn.wordpress.org/trunk@55338 git-svn-id: http://core.svn.wordpress.org/trunk@54871 1a063a9b-81f0-0310-95a4-ce76da25c4cd
…irect. Removes the server-side redirection if missing `postType` and `postId` query args when visiting Site Editor. Why? This redirect is no longer needed as the routing is now handled client side (via [55333]). References: * [WordPress/gutenberg#48023 Gutenberg PR 48023] * [WordPress/gutenberg#47777 Gutenberg PR 47777] Follow-up to [55333], [53413], [53093]. Props ntsekouras, youknowriad. Fixes #57716. Built from https://develop.svn.wordpress.org/trunk@55338 git-svn-id: https://core.svn.wordpress.org/trunk@54871 1a063a9b-81f0-0310-95a4-ce76da25c4cd
I don't think we should remove the nesting, it promotes consistency in terms of how to edit (click the edit button) and creates space for us to add more features in the future. It would be good to add descriptions for these sections though. |
…irect. Removes the server-side redirection if missing `postType` and `postId` query args when visiting Site Editor. Why? This redirect is no longer needed as the routing is now handled client side (via [55333]). References: * [WordPress/gutenberg#48023 Gutenberg PR 48023] * [WordPress/gutenberg#47777 Gutenberg PR 47777] Follow-up to [55333], [53413], [53093]. Props ntsekouras, youknowriad. Fixes #57716. Built from https://develop.svn.wordpress.org/trunk@55338 git-svn-id: http://core.svn.wordpress.org/trunk@54871 1a063a9b-81f0-0310-95a4-ce76da25c4cd
Noting that the change to this background, now applied also in 'view mode', broke the navigate region blue outline that is shown on focus: The logo container backgroudn now partially hides the blue outline. It happens also in 'edit mode', although I think that was already broken before this change. Not the first time the navigate regions focus style breaks, and I'd really love to see some form of visual regression testing in place into this project. WordCamp Europe would be a good opportunity to have some in person conversation about it. In the meantime, I'll creat ea new issue. |
Related to #36667
What?
This is a preparation PR for things like #44461 (comment) Basically in this PR we change the behavior of the site editor a bit to have a close relationship between what the sidebar shows and what's shown in the frame.
So when you click a "template" or "a template part" you navigate to a page where you have the target template in the frame and the sidebar also updates to show details about the template.
Notes
I've implemented this by making the "path" of the sidebar navigation as the "source of truth" from which all the rest (sidebar content and frame) adapts. The "path" is persited in the URL that way, you can come back to the same page.
There's an issue though: When you reload the page after selecting a template, the "back" link in the sidebar is broken, instead of navigating to the parent view (templates), it navigates to the root level one (design). I believe this is intrinsic to the design of the "Navigator" components and we need to adapt these in order to solve the issue. cc @ciampo
This PR is not ready entirely though as there's still a lot of design work remaining in these new sidebar screens.
Testing Instructions
1- Open the site editor
2- Navigate the site editor and select templates...