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 FSE Template resolution doesn't not work for auto-draft posts. #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.

@youknowriad youknowriad self-assigned this Oct 21, 2020
@youknowriad youknowriad added the [Type] Technical Prototype Offers a technical exploration into an idea as an example of what's possible label Oct 21, 2020
@github-actions
Copy link

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 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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/components/visual-editor/index.js Outdated Show resolved Hide resolved
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

@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

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?

@jasmussen
Copy link
Contributor

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

@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
Copy link
Contributor Author

@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

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

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

@jasmussen
Copy link
Contributor

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

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

@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

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
Copy link
Member

@gziolo gziolo Dec 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@jasmussen
Copy link
Contributor

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

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

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

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

@jasmussen
Copy link
Contributor

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

@youknowriad
Copy link
Contributor Author

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

@jameskoster
Copy link
Contributor

@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.

Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Experimental Experimental feature or API. [Type] Technical Prototype Offers a technical exploration into an idea as an example of what's possible
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants