-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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: Site editor template mosaic view #33770
Conversation
Size Change: +3 kB (0%) Total Size: 1.04 MB
ℹ️ View Unchanged
|
baceed9
to
4bffa01
Compare
The previews are working for me, I guess you should update the description. |
Tries playing with this quickly and I have a few questions here:
|
Also, so we want a mosaic view for template parts as well? |
Some potential bugs:
|
4bffa01
to
74ac879
Compare
Description updated 👍 |
I guess it makes sense that browse templates opens the mosaic view. I'm working on an update to do that. Edited: It is implemented. |
Hi @youknowriad, Edited: It is implemented. |
I think the plan was to have the sidebar and the mosaic view. But, this is a good UX question, maybe @jasmussen or @mtias have some insights on what we should do. |
It was not part of the mockups so I assume it is not something we want for now. |
Hi @youknowriad, I tested this on Firefox and everything works without any error. What version of firefox are you using? Would you be able to share the stacktrace of the error? |
6187740
to
ef1ed8e
Compare
This was independent of this PR and was already fixed in trunk. |
6ab9bb5
to
20b196f
Compare
@@ -79,6 +90,8 @@ export default function Header( { | |||
listViewShortcut: getShortcutRepresentation( | |||
'core/edit-site/toggle-list-view' | |||
), | |||
editorMode: getEditorMode(), |
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.
I'm interested in whether it makes sense to conceptualize this as a "zoom" level that could range from focusing on a content to template to full template to mosaic.
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.
Hi @mtias, to me it seems a zoom concept makes sense. For the mosaic view, it seems a good fit but we would need to know how the zoom is going to be used for example to focus the content.
On the side editor, we can edit the content of a specific post but it seems we can not focus on the content and we always see the full site. Do you think we should implement a feature to focus the content of a post on the site editor (making it look almost equal to the post editor)?
@jorgefilipecosta What do you need to move forward here? What is the MVP to land this (as I'm sure this will require iterations). Maybe we could get some reviews from folks that worked on the site editor before @Copons @Addison-Stavlo @Mamaduka |
20b196f
to
1294b29
Compare
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 is really cool, thanks for working on this! The code looks good from a high level.
A few notes/Qs below in comments, beyond those a couple other things on my mind:
-
I would expect to be able to 'dismiss' the mosaic view and be back to where I left off in the editor, but I cannot find any way to go back/dismiss. If I open this and then determine it is not where I want to be, it seems that the only way to get back to the state I was previously at is to select the template I was previously editing from the mosaic view. However, there is also no indication of which template the editor was set to, therefore being able to 'go back' depends on me knowing the specific template I was on which may be a tall order for some users that find their way to this view.
-
I have no indications that clicking the preview will result in any action. I think the previews themselves need some hover styles for the pointer and border, to help indicate that a click will trigger some sort of action (loading that template in the editor and dismissing mosaic view).
-
It feels odd and bit fragmented that we have 2 systems for browsing templates, but the different entry points for browsing templates take us to different systems by default. I feel that "browse all templates" from the document dropdown in the header and "templates >" from the navigation sidebar should bring the user to the same place to create a cohesive experience. Whether that landing point is the nav sidebar view with a clear option to open mosaic view or the mosaic view itself if it serves to replace these bottom levels of the drill-down menu.
-
Somewhat related to the above (and you already noted this as a missing functionality in the description). Since these different entry points to browse all templates take us to different interfaces for doing so, those interfaces should really offer the same functionality. As it stands now, the nav sidebar is the only place where we can create a new template that does not yet exist via the "+" button at the templates level. If the "browse all templates" button isn't taking us to the nav panel and instead opens mosaic view, I would think mosaic view should also allow this functionality.
packages/edit-site/src/components/navigation-sidebar/navigation-panel/index.js
Outdated
Show resolved
Hide resolved
...ages/edit-site/src/components/navigation-sidebar/navigation-panel/content-navigation-item.js
Show resolved
Hide resolved
Hi @jasmussen, that was a bug and it is fixed 👍 |
The close button to exit the mosaic view was implemented. |
Hi @paaljoachim, thank you for your tests, and for sharing a video 👍
Yes, I think we can merge both PR's, the way to preview styles does not need to match the way we preview templates. |
Hi @jasmussen, |
Lots of good points, I'll dive in as best I can tomorrow my time. Just wanted to note that for any icons we need and that are missing from the library, I'm always happy to help. Though at this time my instinct is that we won't need an icon for every type, and that we can have a more generic "template" icon that applies to most of them. Home and blog pages are likely going to be the most important ones there. |
Good progress! This is what I see (and yes, it appears I need to fix some of my templates! 🙈 ): A few small things. Can we apply an The focus style should have a 2px border radius ($radius-block-ui), and be 1.5px thick. This is what we've done for focus styles elsewhere:
Two things, I don't actually think we need any hover style at all on these. And if need be, we can make the focus style be outset a bit from the main item. How does that sound? |
Maybe I am mis-interpereting, but why would we not at least have a pointer cursor style on the actual preview with the click action? Every other clickable interface in the editor has a pointer cursor, from blocks in the inserters, to the buttons in the toolbar, the inline inserter button, input dropdowns, etc. |
Sorry, I was imprecise. Definitely need a pointer cursor — I just don't know that we need the blue border on hover. |
I've pushed some style adjustments. There are few changes I think we can consider in terms of functionality for this PR:
There are a number of other things to get right such as providing a way to create a template from this view, searching, and enabling folks to personalise the number of columns on display. But since this PR already feels pretty big, we can probably do those separately? |
Yeah, I think the idea was to leave those as a follow-up since this has already gotten so large.
Suppose you are user that opens this interface for the first time or by accident. Once it loads, you immediately know that you don't want to do anything here and want to go back to where you were previously. However, you also don't remember exactly which template you were editing, so selecting it out of the list is not an option. Without some sort of back button/dismissal, how do you propose the user gets back to their normal flow in a reasonable manner? |
@Addison-Stavlo In flows that are primarily two-way having an escape hatch like this makes a lot of sense (the "<- Back to notifications" button on github is a good example :D). But I'm not sure that our design should account for accidental / uncommon flows. Otherwise we'd need back buttons on every unique view 🙈 Thinking about it this way, perhaps it makes more sense to support a native navigational pattern than 100% of users will be familiar with: the browser back button. So if you're editing template A, then view all templates, clicking the back button will take you back to editing template A.
I'll see if I can figure out how to do that with css grid and push an update. |
I think this feels a bit better. It may still be worth seeing an iteration with same-height templates though. I moved the description to the popover which tidies things up a bit and will stop templates with long descriptions disrupting the layout too much. Obviously some styling polish is still required, as is the removal the bulk action functionality. |
Im really liking the look of the bottom-aligned templates. Note - kayboard nav for a11y is a bit rough in the current iteration. I can't seem to get access to anything in the interface other than the ellipsis menus.
I think if we wanted to do something like that, we would need to make that a consistent behavior throughout all of Gutenberg's flows. That may be quite the task. In the meantime, "x" and "<" icons are widely used throughout the plugin to close, dismiss, and go back and don't take up much real estate on the screen. |
Closing the PR as there is no work currently happening on it, and we are not sure if we are going to have a mosaic view or not. |
Fixes: #20477
This PR implements the site editor mosaic view.
It implements most of the functionality described in the mockups shared by @jameskoster and tries to follow the design proposal.
Functionality:
I did not add yet all the functionality in mockups because in this PR because it is already big. Missing functionality is the following:
This PR adds the following high-level changes:
How has this been tested?
Although the preview component itself is not working in the trunk and I'm debugging the issue/regression in parallel this PR itself could be reviewed, and I did the following tests :