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

Full Site Editing: Introduce a template editing mode inside the post editor #26355

Merged
merged 21 commits into from Dec 3, 2020

Conversation

@youknowriad
Copy link
Contributor

@youknowriad youknowriad commented Oct 21, 2020

This PR explores the addition of a template editing mode right inside the "post editor". Allowing users to zoom-out to the template level.

Todo

  • Template resolution for new posts and pages (auto-drafts) tries to resolve the 404 template instead of the single one. I believe the root issue is that we rely on frontend resolution from Core which renders 404 for these posts. Not sure exactly how to solve that yet. I created a dedicate issue for this #27391
  • I noticed that sometimes (maybe because of auto-saves), the post gets saved with the content of the template inside post_content.
  • Post Content block should be usable inside the post editor in template mode which mean, I remove the check we had for recursions. We might have to find a different way to guard about the inclusion of such blocks inside the post content.
  • Add an e2e test.
@github-actions
Copy link

@github-actions github-actions bot commented Oct 21, 2020

Size Change: +1.61 kB (0%)

Total Size: 1.19 MB

Filename Size Change
build/annotations/index.js 3.8 kB +1 B
build/block-editor/index.js 128 kB -7 B (0%)
build/block-library/index.js 148 kB +103 B (0%)
build/blocks/index.js 48.1 kB -3 B (0%)
build/components/index.js 172 kB -6 B (0%)
build/components/style-rtl.css 15.3 kB -8 B (0%)
build/components/style.css 15.3 kB -8 B (0%)
build/compose/index.js 9.95 kB +1 B
build/core-data/index.js 15.2 kB +391 B (2%)
build/dom/index.js 4.95 kB -1 B
build/edit-navigation/index.js 11.1 kB +1 B
build/edit-post/index.js 306 kB +685 B (0%)
build/edit-post/style-rtl.css 6.49 kB +71 B (1%)
build/edit-post/style.css 6.47 kB +67 B (1%)
build/edit-site/index.js 24.1 kB -4 B (0%)
build/editor/index.js 43.6 kB +335 B (0%)
build/editor/style-rtl.css 3.85 kB -3 B (0%)
build/editor/style.css 3.85 kB -3 B (0%)
build/element/index.js 4.63 kB +2 B (0%)
build/format-library/index.js 6.86 kB +3 B (0%)
build/i18n/index.js 3.57 kB +1 B
build/keyboard-shortcuts/index.js 2.54 kB -1 B
build/keycodes/index.js 1.93 kB -1 B
build/list-reusable-blocks/index.js 3.1 kB +2 B (0%)
build/media-utils/index.js 5.31 kB -2 B (0%)
build/plugins/index.js 2.56 kB +2 B (0%)
build/reusable-blocks/index.js 2.92 kB -6 B (0%)
build/rich-text/index.js 13.4 kB -1 B
build/server-side-render/index.js 2.77 kB +2 B (0%)
build/shortcode/index.js 1.69 kB +1 B
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/api-fetch/index.js 3.42 kB 0 B
build/autop/index.js 2.84 kB 0 B
build/blob/index.js 665 B 0 B
build/block-directory/index.js 8.72 kB 0 B
build/block-directory/style-rtl.css 943 B 0 B
build/block-directory/style.css 942 B 0 B
build/block-editor/style-rtl.css 11.2 kB 0 B
build/block-editor/style.css 11.2 kB 0 B
build/block-library/editor-rtl.css 8.86 kB 0 B
build/block-library/editor.css 8.87 kB 0 B
build/block-library/style-rtl.css 8.27 kB 0 B
build/block-library/style.css 8.27 kB 0 B
build/block-library/theme-rtl.css 789 B 0 B
build/block-library/theme.css 790 B 0 B
build/block-serialization-default-parser/index.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.js 3.06 kB 0 B
build/data-controls/index.js 827 B 0 B
build/data/index.js 8.98 kB 0 B
build/date/index.js 11.2 kB 0 B
build/deprecated/index.js 768 B 0 B
build/dom-ready/index.js 571 B 0 B
build/edit-navigation/style-rtl.css 881 B 0 B
build/edit-navigation/style.css 885 B 0 B
build/edit-site/style-rtl.css 4.06 kB 0 B
build/edit-site/style.css 4.06 kB 0 B
build/edit-widgets/index.js 26.3 kB 0 B
build/edit-widgets/style-rtl.css 3.13 kB 0 B
build/edit-widgets/style.css 3.13 kB 0 B
build/editor/editor-styles-rtl.css 476 B 0 B
build/editor/editor-styles.css 478 B 0 B
build/escape-html/index.js 735 B 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 B 0 B
build/hooks/index.js 2.27 kB 0 B
build/html-entities/index.js 623 B 0 B
build/is-shallow-equal/index.js 698 B 0 B
build/list-reusable-blocks/style-rtl.css 476 B 0 B
build/list-reusable-blocks/style.css 476 B 0 B
build/notices/index.js 1.82 kB 0 B
build/nux/index.js 3.42 kB 0 B
build/nux/style-rtl.css 671 B 0 B
build/nux/style.css 668 B 0 B
build/primitives/index.js 1.43 kB 0 B
build/priority-queue/index.js 791 B 0 B
build/redux-routine/index.js 2.84 kB 0 B
build/token-list/index.js 1.27 kB 0 B
build/url/index.js 2.84 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.22 kB 0 B

compressed-size-action

@youknowriad youknowriad force-pushed the try/post-title-block branch from f174617 to 3cccd77 Nov 9, 2020
@youknowriad youknowriad changed the title Replace the post title component with the post title block Zoom-out to the template level in the post editor Nov 9, 2020
Copy link
Contributor

@mcsf mcsf left a comment

Nice one!

In terms of feature this is already interesting. It also highlights some of the current problems with FSE, like how long it takes for certain template blocks to load.

In terms of implementation, there are things we can revise, but I know this is still just a draft. :)

packages/edit-post/src/utils/find-template.js Outdated Show resolved Hide resolved
packages/edit-post/src/utils/find-template.js Outdated Show resolved Hide resolved
packages/block-library/src/post-content/edit.js Outdated Show resolved Hide resolved
packages/block-library/src/post-title/edit.js Outdated Show resolved Hide resolved
packages/block-library/src/post-title/edit.js Outdated Show resolved Hide resolved
packages/edit-post/src/editor.js Outdated Show resolved Hide resolved
@youknowriad
Copy link
Contributor Author

@youknowriad youknowriad commented Nov 24, 2020

@noahtallen @Addison-Stavlo and others. The last commit here c233dbf is I believe a good refactoring for the findTemplate adhoc function that is used right now in edit-site package. It might deserve to extract it from this PR to see how possible it is to use that new selector for edit-site as well.

@youknowriad
Copy link
Contributor Author

@youknowriad youknowriad commented Nov 24, 2020

I also noticed that trying to resolve a template for an "auto-draft" post or page results in the 404 template being returned. I'm not sure this is expected to me, It seems like a bug in how the template resolution work?

@gziolo gziolo self-requested a review Nov 25, 2020
@jasmussen
Copy link
Contributor

@jasmussen jasmussen commented Nov 25, 2020

This is a cool PR! This could be how most people will edit templates, or create new ones.

As for how this might work, I need to think about this some more, but because of the work that needs to be done to make this happen, I wanted to take a stab at mocking up my first instinct, which is that editing templates in this way could work a little bit like how "master slides" (the equivalent of templates) are edited in Keynote:

keynote

To translate that to the block editor:

  • You're editing a post
  • You press a button, "Edit [type] template", perhaps in the document sidebar
  • When pressed, you enter a template editing mode, which needs to be confirmed with a "Done" button

Following that instinct, here's my first draft. First, click to edit the template:

Block Editor

Now you're editing the template, and you have to press "Done" before you are allowed to publish:

Block Editor-1

I'm not feeling all of the ingredients, and I would think this could use some iteration. Notably I'm not sure it's clear enough that you switched "modes". However I think that the flow is right, and for the purposes of testing this out further in the PR, I think this might work as a V1.

One additional question, and perhaps also relevant in a larger FSE context: should the post content become locked / un-editable in this mode?

Let me know what you think.

@youknowriad
Copy link
Contributor Author

@youknowriad youknowriad commented Nov 26, 2020

@jasmussen I agree that this is a good starting point and you raise some good questions. Let's try some of it and see how it feels.

@youknowriad youknowriad force-pushed the try/post-title-block branch from 721937b to a9b9483 Nov 26, 2020
@youknowriad
Copy link
Contributor Author

@youknowriad youknowriad commented Nov 26, 2020

@jasmussen I implemented a first iteration of that flow, I'm certain that it's buggy but I'd appreciate some feedback :)

@jasmussen
Copy link
Contributor

@jasmussen jasmussen commented Nov 26, 2020

Alright, this is cool. This is what I see:

flow

As you can see, I can't actually exit that mode without making a change. But other than that, I feel like this is generally a good and relatively basic way to explore this feature. 👍 👍

Small nitpicks:

Screenshot 2020-11-26 at 10 43 47

It bugs me the link isn't the same blue as my "Modern" theme color :D I know that's not part of this PR, but it always stands out to me.

Screenshot 2020-11-26 at 10 47 46

Showing this snackbar was a good instinct!

I think we can reduce the text a bit. How about:

Editing template. Changes made here affect all posts and pages that include the template.

@youknowriad
Copy link
Contributor Author

@youknowriad youknowriad commented Nov 26, 2020

I think we may need a "cancel" link next to "done" instead of allowing to return from the save when there are no changes.

@youknowriad youknowriad force-pushed the try/post-title-block branch from 689dbf9 to d2bc4a3 Nov 26, 2020
@youknowriad youknowriad marked this pull request as ready for review Nov 26, 2020
@jasmussen
Copy link
Contributor

@jasmussen jasmussen commented Nov 26, 2020

I think we may need a "cancel" link next to "done" instead of allowing to return from the save when there are no changes.

In that case I'd change the "Done" label to say "Apply".

@youknowriad
Copy link
Contributor Author

@youknowriad youknowriad commented Dec 1, 2020

So it might be not related to this PR, but it's something with how templates are handled in general.
Edit: it looks like it's related to changes in this PR. I tried loading single template with the master branch and it works without any issues.

So the "draft" thing shouldn't happen. I had this early in the PR but now it solved. That said, the freezing when loading the template in the templates UI is also happening to me when editing the template in the site editor, it doesn't seem specific to this PR. I believe we should remove that listing entirely at some point, it was just an early thing where we didn't have any UI to access templates.

@jameskoster
Copy link
Contributor

@jameskoster jameskoster commented Dec 1, 2020

@jasmussen One small thought... I'm wondering if we could get away with a single button rather than the Apply/Cancel. Looking back at the Keynote example, couldn't we just have a single "Done" button? It would never be disabled, and behave differently based on whether there are changes or not:

  • No changes = returns you directly to the content editor
  • Changes = Save flow, then returns you to the content editor
@jasmussen
Copy link
Contributor

@jasmussen jasmussen commented Dec 2, 2020

I'm wondering if we could get away with a single button rather than the Apply/Cancel.

Yes I think that'd be ideal. This seems fine as a starting point, it lets us validate some of the ideas and test it out. For round 2, I'd love to see a single done button, and perhaps the locking of post content when editing templates, and maybe even bringing in this template tool from the site editor, in place of the centered text label currently there:

Screenshot 2020-12-02 at 08 42 37

Copy link
Member

@gziolo gziolo left a comment

Changes = Save flow, then returns you to the content editor

How about when I play with the template and I don't like changes applied, how do I discard them when there is no Cancel button?

);
return (
<>
<Button onClick={ () => setIsEditingTemplate( false ) } isTertiary>

This comment has been minimized.

@gziolo

gziolo Dec 2, 2020
Member

Should it else undo all unsaved changes applied to the template?

When the editor gets into a dirty state and I click Cancel then I see some strange dot icon applied on top of the Publish button.

This comment has been minimized.

@youknowriad

youknowriad Dec 2, 2020
Author Contributor

I'm not sure, I think it's something we should try and adapt.

@jasmussen
Copy link
Contributor

@jasmussen jasmussen commented Dec 2, 2020

How about when I play with the template and I don't like changes applied, how do I discard them when there is no Cancel button?

To me, the problem is that "review changes" has been collapsed into almost nothingness:

Screenshot 2020-12-02 at 10 39 25

I feel like this list should be expanded by default, redesigned if need be, but be a key part of the experience of editing templates, not something we bury:

Screenshot 2020-12-02 at 10 40 03

I don't see the point in this being hidden at all, to be honest. And when visible, you just uncheck your changes when you press Done.

@jameskoster
Copy link
Contributor

@jameskoster jameskoster commented Dec 2, 2020

Agreed @jasmussen.

Another option is to simply undo until you're back to the original state, which raises another question (probably beyond this PR) – is the undo action aware of this state change, as it is in Keynote? If not, should it be?

@jasmussen
Copy link
Contributor

@jasmussen jasmussen commented Dec 2, 2020

Another option is to simply undo until you're back to the original state, which raises another question (probably beyond this PR) – is the undo action aware of this state change, as it is in Keynote? If not, should it be?

Yep, undo is a very important ingredient.

One thing I will add though, which is why it's so important to show that list of changes by default, not collapsed, is that you may accidentally make a change without knowing you did, so you don't know to undo. By showing it in that expanded list, it makes you aware of it.

@youknowriad
Copy link
Contributor Author

@youknowriad youknowriad commented Dec 2, 2020

I'm going to open a separate PR for the "EntititiesSavedState" component and make it expanded by default.

@jasmussen
Copy link
Contributor

@jasmussen jasmussen commented Dec 2, 2020

Awesome. Followup question: does it need to be able to collapse at all? IMO: no.

@youknowriad
Copy link
Contributor Author

@youknowriad youknowriad commented Dec 2, 2020

@jasmussen I don't know, we can try whatever you like :P

@jameskoster
Copy link
Contributor

@jameskoster jameskoster commented Dec 3, 2020

@youknowriad would it be possible to add the site editing context (or at least the css classes) when you switch to template editing? That way we'll inherit the changes in #27271 which make interacting with template parts more transparent.

@gziolo
gziolo approved these changes Dec 3, 2020
Copy link
Member

@gziolo gziolo left a comment

I think code-wise we are good to go. There is still a ton of work ahead, but we need to split it into smaller chunks to make it possible to iterate quicker.

@youknowriad
Copy link
Contributor Author

@youknowriad youknowriad commented Dec 3, 2020

@jameskoster I didn't know about that change, looking at it now, and as implemented right now, it is an ad-hoc CSS. I believe this is better addressed with an explicit setting on the block-editor package. I like keeping the PRs small and focused. I shall explore that separately.

@youknowriad youknowriad merged commit a250e9e into master Dec 3, 2020
15 of 16 checks passed
15 of 16 checks passed
Admin - 1 Admin - 1
Details
Build Release Artifact
Details
Cancel Previous Runs
Details
Check Check
Details
build
Details
Compare performance with master
Details
pull-request-automation
Details
test (gutenberg-editor-gallery)
Details
test (gutenberg-editor-gallery)
Details
All
Details
JavaScript
Details
Admin - 2 Admin - 2
Details
PHP
Details
Admin - 3 Admin - 3
Details
Mobile
Details
Admin - 4 Admin - 4
Details
@youknowriad youknowriad deleted the try/post-title-block branch Dec 3, 2020
@youknowriad youknowriad changed the title Zoom-out to the template level in the post editor Full Site Editing: Introduce a template editing mode inside the post editor Dec 3, 2020
@github-actions github-actions bot added this to the Gutenberg 9.6 milestone Dec 3, 2020
vcanales added a commit to vcanales/gutenberg that referenced this pull request Dec 3, 2020
@youknowriad youknowriad mentioned this pull request Dec 18, 2020
2 of 11 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment