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

FSE: Remove template block settings and UI #35115

Merged
merged 6 commits into from
Aug 5, 2019

Conversation

noahtallen
Copy link
Contributor

@noahtallen noahtallen commented Aug 3, 2019

The goal of this PR is to improve the UX of FSE by removing a lot of visual clutter caused by our container blocks. With this PR enabled, you'll see virtually nothing related to them while clicking around! @shaunandrews was definitely right on here.

Here's what it looks like: (lol my gif software speeds it up a ton)
2019-08-02 18 03 43

Changes proposed in this Pull Request

  • Disable all block settings for the post content block
  • Disable anchor setting for template block (custom classname is still needed for @Copons work to make the theme classname available on the top level block)
  • Cleanup the CSS for hiding all of the UI related to the post content & template blocks. @apeatling had done a ton of work for the post content block per this, but it turns out that Gutenberg v6.2 introduces some new dashed borders to indicate structure, and we needed to get the CSS ready for that. I also extended this CSS to the template block.
  • Make sure that it's impossible to even see the block setting panel for the template block.

Hiding template settings
Diving into that last item, I used a little hack that's available in core. The SiteTitle block actually deselects itself when you click into it (you'll notice you see no block settings when clicking the site title block because of that). So I also applied that to the template block. When you click on it, we just dispatch an action deselecting it. This means that you don't see the settings panel for it. And it's totally transparent, it visually just looks like not much happens when you click on it.

Trying to hide post content settings
I tried to extend this approach to the post content block, but it turns out that when the post content block is deselected, you can't select or edit or do anything to the inner blocks. So that was a no go. This caveat is still there in v6.2, but it turns out they're reverting that behavior: WordPress/gutenberg#16888. I'm not sure there's a ton more we can do unless we want to use a css hack to display:none; that bad boy. It will be really nice if WordPress/gutenberg#7469 gets implemented. Then all of this will be resolved. :)

What does this mean, then? You can still see information related to the post content block in two places.

  1. The breadcrumb thingy for blocks shows the parent block in its path.
  2. If you select in the area around the post content, it's possible to select the block and see its name in the settings panel. Note that there are no settings that can be changed here, though. Additionally, you have to click twice to begin editing the post content.

Screen Shot 2019-08-02 at 6 04 16 PM

Why not fix those now?
I think it would be high effort and low impact, so I figured I'd get a PR up for the rest of it for now. :)

Testing instructions

  1. Run FSE on your local environment. Brownie points if you can test this with Gutenberg v6.2!
  2. Open the page editor. Click all over the place. Make sure that you see no block UI related to our container blocks other than the mention above.
  3. Click into the header and footer areas. Open the block settings sidebar and make sure you see "no block selected"
  4. Click and hover around the elements in post content. Make sure you see no outlines of the larger post content block.
  5. Make sure that the "edit header/footer" buttons still open the template editor.

This goes a long way towards fixing #34751, but it may not be completely fixed. Need to do some more testing on that.
Resolves #35094.

@noahtallen noahtallen added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Goal] Full Site Editing labels Aug 3, 2019
@noahtallen noahtallen requested review from apeatling and a team August 3, 2019 01:16
@noahtallen noahtallen requested a review from a team as a code owner August 3, 2019 01:16
@noahtallen noahtallen self-assigned this Aug 3, 2019
@matticbot
Copy link
Contributor

- Disables all configuration options for post content
- Disables anchor for FSE templates. Enables custom class name so we can
  inject the correct classname for the header/footer
This means that you can't see any reference to their settings!

Note that we cannot do this same patch for the post content
block because once the block has been disabled, you can't
edit the inner blocks. Will have to find a different patch for that.
@noahtallen noahtallen force-pushed the fix/remove-template-block-controls branch from fe2f04c to 20e8c2f Compare August 3, 2019 01:25
@matticbot
Copy link
Contributor

matticbot commented Aug 3, 2019

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

Copy link
Contributor

@glendaviesnz glendaviesnz left a comment

Choose a reason for hiding this comment

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

👍 This all works for me, and tested on gutenberg 6.1 & 6.2 and didn't notice any issues.

@Copons
Copy link
Contributor

Copons commented Aug 5, 2019

Works fine for me too, but I've got an additional suggestion.
What about renaming the Content Slot block into just Content and updating the description (#)?
As it is right now, it sounds more technical than descriptive, and would probably be confusing for the end user to see the page blocks being treated as children of a nondescript "content slot".

Screenshot 2019-08-05 at 13 24 59

What do you say if we went with something like this?

- title: __( 'Content Slot' ),
- description: __( 'Placeholder for a post or a page.' ),
+ title: __( 'Content' ),
+ description: __( 'The page content.' ),

(suggestions appreciated for the description, though 🙂 )

@noahtallen
Copy link
Contributor Author

That's an awesome idea! I think that people won't be confused by that nomenclature if it pops up in a couple of places.

@noahtallen
Copy link
Contributor Author

Here's what that looks like:
Screen Shot 2019-08-05 at 9 12 27 AM

Since users can see this in a few places, we want to make
sure they aren't confused when it does pop up.
@noahtallen noahtallen requested a review from a team as a code owner August 5, 2019 16:15
@apeatling
Copy link
Member

Tested all of this last night and worked great for me.

@noahtallen noahtallen merged commit 0492add into master Aug 5, 2019
@noahtallen noahtallen deleted the fix/remove-template-block-controls branch August 5, 2019 16:57
@matticbot matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Aug 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FSE: Content Slot and Template block options are visible
5 participants