Add back button for isolated template part editor #34732
Conversation
kevin940726
added
the
[Feature] Template Editing Mode
|
Size Change: +5.17 kB (0%) Total Size: 1.07 MB
ℹ️ View Unchanged
|
| } | ||
|
|
||
| return ( | ||
| <Button |
There was a problem hiding this comment.
We probably should auto focus on this element when navigating to the isolated editor?
There was a problem hiding this comment.
Hm. Could we focus .edit-site-visual-editor? That way pressing Tab once focuses the close button. This matches how other modal interfaces work in the block editor e.g. opening Preferences.
cc. @tellthemachines who is smarter than I am at this kind of stuff.
There was a problem hiding this comment.
I'm not 100% sure what's going on here, but this is what I'm seeing:
When tabbing through the page, starting from the header, the back button is focusable in the normal tab order as long as nothing inside the editor is selected. This means that if I tab to the button, and tab once more, landing on the first block in the editor area, I'm unable to Shift+Tab back to the button. In addition, the shortcut to navigate between landmark areas (Ctrl+`) doesn't seem to work in the site editor, so there's no way to get back to that button.
I'd expect to be able to Shift+Tab back to the button from the editor.
There was a problem hiding this comment.
Hmm, I couldn't figure it out either. It seems like pressing Shift + Tab goes back to the editor header in site editor. No idea if it's a browser thing or it's handled somewhere else 🤔. It's not that normal since the editor is in an iframe. Anyone has any idea how to fix this?
| { sprintf( | ||
| /* translators: 1: Template name. 2: Template type name ("template" or "template part"). */ | ||
| __( 'Back to "%1$s" %2$s' ), | ||
| previousTemplateTitle, | ||
| getTemplateTypeName( previousTemplateType ) | ||
| ) } |
There was a problem hiding this comment.
We probably don't want "template part" since it's a technical term? Should we just remove this entirely and just say Back to "index" or Back to "header"?
There was a problem hiding this comment.
c.c. @jameskoster because you mentioned it in #34679 (review) 🙂
There was a problem hiding this comment.
I'm not sure that we need to display the name here. Just a "<- Back" label (like in the post editor) is probably adequate?
There was a problem hiding this comment.
It was in the design mockup though, but also works for me if we decide to remove it.
I guess it would have some benefits when we have nested template parts (if ever).
kevin940726
requested review from
talldan,
noisysocks,
tellthemachines and
jameskoster
There was a problem hiding this comment.
To me it feels a little peculiar to see the back button when opening a template part via the navigation sidebar. That particular action seems more like the commencement of a new flow, and so the back buttons feels contextually misplaced.
I would however expect to see the back button when accessing the isolated template part view from the "Edit template part" menu item we're working on in #34679.
Related to this, we should probably consider utilising the other "go back" design pattern from the post editor:
back.mp4
Although I acknowledge that we may prefer to do that separately :)
There was a problem hiding this comment.
Code looks really good, nice work. A lot of it will change after you address @jameskoster's feedback so not sure my review is very helpful 😀
| } | ||
|
|
||
| return ( | ||
| <Button |
There was a problem hiding this comment.
Hm. Could we focus .edit-site-visual-editor? That way pressing Tab once focuses the close button. This matches how other modal interfaces work in the block editor e.g. opening Preferences.
cc. @tellthemachines who is smarter than I am at this kind of stuff.
| @@ -87,15 +96,19 @@ export default function BlockEditor( { setIsInserterOpen } ) { | |||
| <BlockInspector /> | |||
| </SidebarInspectorFill> | |||
| <BlockTools | |||
| className="edit-site-visual-editor" | |||
| className={ classnames( 'edit-site-visual-editor', { | |||
| 'is-template-part': isTemplatePart, | |||
There was a problem hiding this comment.
Right, so per @jameskoster's feedback (I was confused about this too) we probably should show the black border when there is a is-isolated class instead of just whenever a template part is being edited.
Yeah, that makes a lot of sense! Quoting this from the PR body to bring this up again since I feel like it's important (and I'm curious):
Anyone knows why we aren't making the isolated (zoom) mode a native navigation on |
I can't comment on the technical side. I'm also not sure if this has ever been discussed from the design side either. Perhaps Joen knows? @jasmussen when a user moves from editing something like a post, to a template or template part, should the browser back button take them back to the original post? |
|
Personally if I saw a button labelled ← Back then I'd expect the browser back button to also work. If it were Ⅹ Close then I would not. |
For any navigation you make to what feels like a different interface or document, I'd expect the browser back button to go back to where you were, ideally. However I want to pose the question: that in-canvas back button, do we want it at all? It's never quite clear what it goes back to, and the button has become synonymous with "go left", even to the point of the icon pointing there, which is rarely accurate. Why should editing a template part be different from editing a template in that vein? |
|
Re-introducing myself with #33926, I think there's an aspect to isolated template part editing which is important, namely the UX for flowing in and out of it. In a sense you can already do that today with the W menu, by choosing a template part and editing it then and there. But from what I read, template part isolation mode needs more context to the parent template than W navigation provides. If you are meant to go back to editing the Index template, that has to be where you came from — which is only technically and in a browser history sense the case with this PR. Here's an older and very obsolete mockup showing how flowing in and out of template mode could be a much more literal thing, featuring animation and scaling:
In the above context, I'd expect to be able to click outside the template part, on the scrimmed area, to exit that isolation mode. A more likely flow would be between these two:
How you get from left to right and back again is suggested here:
Notably, the context between template and isolated template part is kept, since you enter the mode from either of the above mechanisms, making it more obvious where you go when you go "back". In this vein, perhaps a close button actually would make sense? |
|
Thanks for your input @jasmussen, and for digging up the old mockups. I agree that the back button feels part of a larger discussion. From a more holistic viewpoint I too have reservations. I believe the zooming design can work very well for moving in and out of document / file management, and maybe for things like pattern selection. But zooming to edit entities within a document is something I still struggle to picture a bit. I worry it could get a little nebulous; zooming out of a post to a template, then into a template part like a header, then into a navigation menu, then back in to the original post. All the while being careful not to conflate pages and templates. As an alternative I've been playing with a simpler approach, somewhat inspired by the spotlight mode we have. In the video below I'm editing a page, but am able to easily move between other entities like the header and the menu without losing context. This dynamic could work exactly the same when editing a template. spotlight.mp4Access to edit the contents of secondary entities is gated behind an "Edit" button. This could be augmented with a double-click action, or any other ways we can think of. I do think the added friction is helpful though because it helps indicate when you're editing something different. I know there's not really anything new here. We've explored these things before. But I wonder if something like this could serve as an interim? It circumvents the awkward back button and lays a rough foundation for potential zoom behaviours in the future. |
noisysocks
added
[Type] Enhancement
It does run the risk of being too fancy and ultimately confusing, so it definitely hinges on the right implementation to work. On the flipside, animation and zooming if done just right can help establish context such as point of origin, destination, and focus by virtue of motion alone. Even without motion, because it will be turned off for people with the prefers-reduced-motion option, it feels important to keep the canvas context, which your prototype does excellently, and it doesn't seem like it would preclude adding motion at a later point, would you agree? In that vein it seems like a great starting point. |
|
Yes, agreed on all points. Getting the zoom animation right will be a task all on its own, and may take a few iterations to get it feeling right. So laying some foundations for the spirit of the interaction now – in a way that avoids the pesky "<- Back" button – makes sense to me. There are a couple of pieces missing to make the prototype I shared above work as intended, most notably the overlay + edit button in the toolbar when selecting entities like template parts. Perhaps we can add that to #34679, what do you think @kevin940726? |
|
Had a chat with Matias today. In the interest of getting the isolated view to a good spot let's push forward with back button for now since that aligns with the page -> template edit flow in the post editor. We can circle back to explore that interaction in more detail later. So going back to the original feedback, can we try making the back button appear only when you access the isolated view from either the 'edit template part menu button' or the list view? Not from the sidebar menu. As we aren't 100% sure what the interaction will look like for moving back and forth between content/template/template part in the long term, we can probably leave the browser back button support out for now as well. |
kevin940726
force-pushed
the
add/isolated-template-part-editor-back-button
branch
from
5db3be6
to
34ce49b
Compare
Apologies if this is still in progress, but to clarify: I think we should always show the background (as described in the tracking issue). It's just the "<- Back" button that should appear situationally. We can also probably have the button simply read "<- Back" and omit the template name. In some situations displaying the template name can be more confusing. For example "<- Back to Page" could be interpreted in a couple of different ways, and things will get worse when we consider custom templates where folks might use a name that matches the content. This would also align with the back button in the post editor. |
kevin940726
deleted the
add/isolated-template-part-editor-back-button
branch
Description
Related to #33926. Addresses the following:
Added a "edit-post"-style back button for isolated template part editor based on the design in #29148 (comment), modified to include a dark background as seen in "edit-post" template editor. See the attached screencast below.
The back button is only visible when navigating to an "isolated template part editor" from another editor. It's not there in template editor, nor when landing directly in the isolated template part editor (as there isn't anywhere to go back to).
I noticed that we're not using browser's history to navigating between editors, I'm not sure if it's an oversight or an intentional behavior. Hence, we can't just use the native
history.back()for this feature.How has this been tested?
Screenshots
Kapture.2021-09-24.at.10.39.34.mp4
Types of changes
New feature
Checklist:
*.native.jsfiles for terms that need renaming or removal).