Block Editor: Add controlled inner blocks focus styles. #19929
Conversation
epiqueras
requested review from
ellatrix,
talldan and
youknowriad
as code owners
| ...props | ||
| } = this.props; | ||
| const { templateInProcess } = this.state; | ||
|
|
||
| const classes = classnames( 'block-editor-inner-blocks', { | ||
| 'has-overlay': enableClickThrough && hasOverlay, | ||
| 'is-capturing-toolbar': captureToolbars, | ||
| 'is-controlled': onInput, | ||
| 'is-selected': isSelected, |
There was a problem hiding this comment.
Could we add these to BlockList? I'm trying to move the existing classes in #19849.
There was a problem hiding this comment.
That's because this particular wrapper will eventually be opt-out: #19910.
There was a problem hiding this comment.
I've moved this functionality to block-list-block now. Does that change make sense??
|
This feels incredible similar to the current Spotlight mode, but with different styling. Maybe we should try fading all the other elements away, rather than introducing the dark overlay.
|
|
Feel free to tweak the shadow values. |
|
You folks move so fast, it's wonderful to see! This "focus mode"/isolation mode is in principle not a bad idea. Variations of it have been explored in a number of mockups, and the goal, nearly always is to:
The 2nd one is probably the most important, and it affords the opportunity to add an "Are you sure" dialog when you exit editing the template part, and a spot to explain that — hey, this change affects every page! As such, the idea, the implementation, both feel like they are veering in the right direction. Kudos for the energy. I wonder though, whether this is the right time to implement it or not. It’s not a bad idea, but it might be one that should be postponed until we know we need it. The G2 interface (#18667) is likely going to simplify a number of things, we might even consider retiring "spotlight mode" entirely, opening up alternate treatments to this, as Shaun alludes to. More importantly, we might all think it's going to be important to make it clear to users that they are editing a global part, the fact is we don't actually know for sure yet that this is a problem. By distinguishing between a template part and any other block, we may in fact add confusion: why is this block highlighted but not that block? — and since we are already looking at batching multiple template part edits together in an on-publish prompt, we already have an opportunity to inform the users of what they are doing. To summarize: this may be the right idea and the right solution, but because we don't know that for sure yet, maybe we should wait! |
Try editing a template part, it's very hard to tell if you are editing it or the blocks right after it without opening the block navigator.
That can easily be explained somewhere like the breadcrumb or where the button to exit the code view appears. |
|
Any updates on this from design? |
|
Here's a quick mockup in Figma:
|
|
That last one from @shaunandrews is spot on! I suggest that be the direction we go. All my efforts were leading in that direction as well. Let's just maintain the same amount of padding above the text within the selected template-part that initially shows in the unselected view. Once the template-part is focused, the padding above the text gets a little tight. Adding the margin for the block toolbar with the "pull away" effect is totally worth keeping. The breadcrumb indicator is unobtrusively informative as well. Really good work! 😍 |
epiqueras
force-pushed
the
add/controlled-inner-blocks-focus-styles
branch
from
7e270ff to
821c25c
Compare
|
Size Change: +261 B (0%) Total Size: 857 kB
ℹ️ View Unchanged
|
|
|
|
G2 makes it look slick @youknowriad 👏 |
noahtallen
force-pushed
the
add/controlled-inner-blocks-focus-styles
branch
from
ff4f22c to
b31e3db
Compare
|
I was looking at picking this up again today. Should the "focus mode" styles be consistent with what we're doing here? |
| @@ -121,6 +121,7 @@ function BlockListBlock( { | |||
| 'is-focus-mode': isFocusMode, | |||
| 'has-child-selected': isAncestorOfSelectedBlock, | |||
| 'is-block-collapsed': isAligned, | |||
| 'is-focus-container': name === 'core/template-part', | |||
There was a problem hiding this comment.
We need to create this class in a more extensible way. Before, with InnerBlocks, we added our custom styles if InnerBlocks had the onInput prop and if it or one of its children was selected.
|
This is what it looks like right now. It feels a little choppy because selecting blocks in the editor jerks everything around a bit (and because my gif software is low framerate!)
|
Yes this is an excellent point, and I think we should be changing the style of the effect in this PR to match that of what focus mode does. |
|
Cool! That makes it easy to stay consistent if we did ever want to update focus mode to be similar to the mockup above. |
|
I'm thinking we should probably shelve this for now in favor of just using the usual block focus styles. |
|
Yeah, that sounds reasonable. I'll close this PR out since the styles basically just set a massive background shadow over the rest of the page. (And since it's quite stale at this point!) I investigated triggering the focus styles just for the template part block probably 4 weeks ago but didn't make any progress on it. (There was some oddity with detecting when just the template part block was selected that I didn't figure out.) A new PR could be opened once someone wants to pick this up again 👍 |
Closes #19886
Screenshots
Types of Changes
New Feature: Controlled inner blocks like template parts and post content now have a distinct style when selected.
Checklist: