Site Editor: Add 'Show template' toggle when editing pages #52674
Conversation
noisysocks
added
Needs Design
|
Size Change: +417 B (0%) Total Size: 1.62 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 0724b6d. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6244853194
|
|
I know this is a proposal, so just commenting on the concept of having an icon (any icon) in the Document tools toolbar to toggle between editing a page/post and editing that page/post's template. This is all totally subjective. I'm finding it hard to articulate why I'm not sold on it yet. Though it seems like the right place for such a button, given that its siblings are mostly related to document editing, the toolbar does contain a number of tools already, it might be just that I don't like clutter 🤷🏻 Or could be the icon itself that's throwing me. Maybe a lock/unlock set would tap my brain in the right way and it's just as simple as finding the right signage. With this toggle button, we'd have three different controls to toggle between modes (counting the notification), whose functions require the user to expend a few cognitive cycles to either read their descriptions or learn what they do. This is not a bad thing. The editor is full of such things. I mention this with the risk of bringing up a potentially tired pattern, but I can't help but wonder if the editor couldn't just show me straight up what I can and cannot edit in the canvas. Example: 
|
|
Did y'all consider an inverted implementation of the button, IE use the
Edit: From discussions elsewhere it seems like this isn't strictly about toggling the template. It's more about hiding parts of the template that distract from writing (headers, footers, sidebars, etc.). It can still be useful for certain meta data (title, featured image) to be editable on the canvas though. |
We didn't spend much time thinking about which way the toggle should work as it's easy to swap 😀 Agree the |
noisysocks
force-pushed
the
try/show-template-toggle
branch
2 times, most recently
from
6610070
to
a45f499
Compare
noisysocks
changed the title
noisysocks
requested review from
ajitbohra,
ryanwelcher,
juanmaguitar,
fabiankaegy and
ndiego
as code owners
|
I updated this so that Show template is a toggle that appears in the sidebar. This is a lot less prominent which makes me feel comfortable shipping it in the plugin as a way to gather feedback and iterate. I also refactored things so that switching between modes is fast. I'm pretty happy with it, so will mark it as ready for review, though maybe @SaxonF wants to push a few design tweaks. |
noisysocks
changed the title
noisysocks
added
Needs Design Feedback
noisysocks
force-pushed
the
try/show-template-toggle
branch
from
a45f499
to
22734d4
Compare
|
Given the changes to the template section of the sidebar (#51477) we might do something like this: 
It has similar prominence to the current state of the PR. |
|
I'll put this on my list to revive. Thanks for the updated designs @jameskoster 🍺 |
ramonjd
force-pushed
the
try/show-template-toggle
branch
from
22734d4
to
837c6b2
Compare
ramonjd
force-pushed
the
try/show-template-toggle
branch
from
c63eb8b
to
0724b6d
Compare
There was a problem hiding this comment.
Thanks again for all the rounds of follow-up @ramonjd, this is testing great for me now! The resetting when going to the browse mode is working nicely, with the value persisting when I go back into the editor. This feels like a good balance to me, so that folks going out and in to a page have their value persisted, but if they reload the page they can easily get back to the template preview, too 👍
2023-09-20.16.48.02.mp4
I noted one other (very) edge case, which I've added in a comment — feel free to disregard that one for this PR, I think we're well and truly in polishing territory which could be looked at in a follow-up if need be. And of course we can see if we get feedback from folks using the feature during the 6.4 beta, too. For now this PR is feeling good and solid to me and ready to land 🎉
Great work again getting this PR to a thorough state and adding in all the tests. LGTM! ✨
| return [ | ||
| createBlock( | ||
| 'core/group', | ||
| { | ||
| layout: { type: 'constrained' }, | ||
| style: { | ||
| spacing: { | ||
| margin: { | ||
| top: '4em', // Mimics the post editor. | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| flattenBlocks( blocks, ( block ) => { | ||
| if ( PAGE_CONTENT_BLOCK_TYPES[ block.name ] ) { | ||
| return createBlock( block.name ); | ||
| } | ||
| } ) | ||
| ), | ||
| ]; |
There was a problem hiding this comment.
Another nit: if no page content blocks are found, then we return an empty group block. This means that if a page's template has no content blocks at all, switching off "Template preview" results in an empty page, because the return value is not an empty array:
This is getting very edge-casey territory, and you'd only see this if you go to a page that uses a template that has no content blocks and only after switching off Template preview, so not a blocker to landing for me!
There was a problem hiding this comment.
Great find! I think we should follow up with a fix for this.
I'll make a note and get a PR up.
🙇🏻
|
@andrewserong Thanks for sticking with me on this one. I owe you 💯 🍹 |
|
Also found a bug. When exiting the site editor in edit mode with the template hidden, we need to reset |
andrewserong
added
the
Backport to Gutenberg RC
ramonjd
removed
the
Backport to Gutenberg RC
fullofcaffeine
added
Backport to Gutenberg RC
* Site Editor: First pass at adding a Show Template toggle when editing pages * Bit of clean up * Use a toggle in sidebar * Make toggling 'Show template' on/off faster * Use blocks in same order as template * Tidy up code * Add unit tests * Add doc comment * Formatting Remove unused component import Doc update * With icon and updated copy * Use only the check icon and set min width of dropdown to minimize the effect of the dropdown width toggling between narrow and wide * This commit tries to get around a peculiarity of __experimentalGetGlobalBlocksByName, which doesn't seem to be able to find the post-content block in a new block tree (if it even exists in state). We're sidestepping the issue, but with fewer selectors, so overall hopefully it's more sustainable. * Removing the filter iteration layer and adding it to the flattenBlock helper function Checking for disallowed blocks as well, e.g., core/query * Extract usePageContentBlocks and write basic tests Moving PAGE_CONTENT_BLOCK_TYPES to utils/constants and creating a map * Updates selectors to include canvas mode so we can show any hidden templates in focus mode * Moving getPageContentFocusType and setPageContentFocusType selector/action to locked, private methods In the use-page-content-blocks.js test, add innerblocks to the query test block --------- Co-authored-by: ramon <ramonjd@gmail.com>
| }, | ||
| }, | ||
| }, | ||
| flattenBlocks( blocks, ( block ) => { |
There was a problem hiding this comment.
I find it weird that we're trying to guess the blocks from the content of the template rather than just fixing "core/post-title" + "core/post-content" (like the post editor).
There was a problem hiding this comment.
Could you help me understand that statement? Is there something more reliable that could be done here?
I imagine it was approached this way to cover the (possibly unlikely) scenario that there are 2 x site titles or whatever in a template.
Plus there is a bit of guessing required since we don't know how deep or where these blocks are located in a template in advance.
There was a problem hiding this comment.
rather than just fixing "core/post-title" + "core/post-content" (like the post editor).
By grabbing the real blocks we ensure that the order of the content blocks matches the template so that it more closely resembles the site. I suppose the main value at the moment is that it determines whether or not a featured image is shown, and the order of the title, featured image, and content blocks. While reviewing, I thought it added a nice level of polish.
There was a problem hiding this comment.
By grabbing the real blocks we ensure that the order of the content blocks matches the template so that it more closely resembles the site
Is that the role of this mode though? I mean, it felt like this mode is more of a "post editor" mode to me where we don't really care about the template, we're just filling the content and title.
There was a problem hiding this comment.
I mean, it felt like this mode is more of a "post editor" mode to me where we don't really care about the template, we're just filling the content and title.
We could do it this way.
I'd probably want to confer on the UX with some design folks.
The thinking was that, because we're in the site editor, it would be a more intuitive, seamless experience to have the page content reflect the page template.
So for instance, if I edit the Pages template to have the post title appear underneath the post content, (just because I can) then when I edit that page in the site editor, the page corresponds with the new template structure. I can see my changes.
It might be confusing to have the page content seemingly 'revert' in template preview mode just seconds after I've edited the template, whereas the post editor doesn't have any of the extra context of templates etc so the experience is more "static", if that makes sense.
That's the argument for.
Against, it'd be easier to refactor the code to just make it match the post editor behavior, e.g., just show the required inner blocks...
[
createBlock( 'core/post-title' ),
createBlock( 'core/post-content' ),
]Another angle is to remove the template preview mode altogether, I'd be for this. I understand there's a "great unification" going on, where the site editor will be jack of all trades, but I don't know how much value hiding the template in this way adds. Maybe to some folks.
There was a problem hiding this comment.
I don't know how much value hiding the template in this way adds. Maybe to some folks.
I'm not sure how much value it has in its current form, I but I like the principle behind the idea of a more focused writing-mode like experience akin to the post editor, that folks can use from within the site editor. However it's exposed, I think it's important that folks can still set a featured image while editing their post content, whether or not that's in the sidebar or on the editor canvas. One of the neat things about having it in the canvas is that it gets exposed in the list view for free, so it's easier to find / interact with.
There was a problem hiding this comment.
I'm in a middle of a big refactor to use the EntityProvider and would have appreciated an e2e test to ensure I didn't break this feature. #56000
I'll add one. Thanks! |
It also adds an aria label to the edit template button to indicate its state (pressed or not pressed).
What?
First pass at adding a Show template toggle to the site editor when editing a page. Toggling this hides the blocks belonging to the template entirely so that you're left with a minimal interface that mimics the post editor.
Why?
Allows users to focus on their content when desired. Moreover, it is small a step towards one day unifying the post editor and site editor. See item 3 in #41717.
How?
We re-use the approach that the site editor has for editing a
wp_navigationCPT but instead of creating a fakecore/navigationblock we create a fakecore/groupblock containing acore/post-title,core/post-contentandcore/post-featured-image`. These blocks are shown in the same order they appear in the template and do not appear if the template doesn't have it.Testing Instructions
Run some tests:
Screenshots or screencast
2023-09-14.14.33.26.mp4