Force HTML blocks to be in preview mode when in a reusable block #5298
Conversation
When a HTML block is made reusable, and the reusable block is not being edited, we should not display the HTML.
noisysocks
added
[Type] Enhancement
| @@ -48,7 +48,7 @@ export const settings = { | |||
|
|
|||
| edit: withState( { | |||
| preview: false, | |||
| } )( ( { attributes, setAttributes, setState, isSelected, toggleSelection, preview } ) => ( | |||
| } )( ( { attributes, setAttributes, setState, isSelected, toggleSelection, preview, isReadOnly } ) => ( | |||
There was a problem hiding this comment.
In an effort to avoid extra block APIs, I'm wondering if we could avoid this new prop.
How about:
- Always making the preview mode as the default for the HTML block
- When the HTML block is selected, switch to code mode unless we explicitely switched to HTML mode (local state)
cc @jasmussen @aduth ?
There was a problem hiding this comment.
What about just setting preview={ true }?
There was a problem hiding this comment.
Actually, why we don't just use the block save function instead of edit (for all blocks) when we're not editing it?
There was a problem hiding this comment.
That wouldn't work in many cases, also much harder to keep excesive re-renders in check.
There was a problem hiding this comment.
- Always making the preview mode as the default for the HTML block
- When the HTML block is selected, switch to code mode unless we explicitely switched to HTML mode (local state)
If @jasmussen is happy with the HTML block behaving in the way you describe, then I think this is the way to go 🙂
What about just setting
preview={ true }?
withState would override the prop that we pass in.
Actually, why we don't just use the block save function instead of edit (for all blocks) when we're not editing it?
Also, using save doesn't work because, in the majority of cases, the editor styles do not work with frontend markup.
There was a problem hiding this comment.
I don't think we should default to preview in blocks that are editable, because it breaks the expectation — you might fear that clicking on a gallery would switch to HTML and there is no clear indication of when that would happen. Having a reusable block default to preview is fine because the nature of the block is that it is not generally editable unless you opt in into that.
There was a problem hiding this comment.
That makes a lot of sense @mtias.
With that UI change ruled out, I don't see a way to avoid passing a prop.
Just an idea: would making the prop a more abstract concept help ease our concerns about expanding the API? For example, we could make this a prop that tells the child block information about the parent block:
<BlockEdit parent={ { name: 'core/block', isEditing: false } } ... />
There was a problem hiding this comment.
This is made awkward by the fact that we seem to only care to target a single block here. If we're introducing a concept of "read only", why shouldn't then every block have to define the behavior of what read-only means?
Obviously this would be a huge pain, but I think special treatment of a few select blocks introduces non-ideal inconsistency, both from an implementation perspective and for developers.
A few pieces of related work:
- The reusable block type already tries to disable its children's inputs via a new
Disabledcomponent (Blocks: Fix Reusable Blocks keyboard navigable (Introduce Disabled component) #5223). - We already display the
saveform as a preview for invalid blocks (source)
There was a problem hiding this comment.
Using save for a reusable block might make sense in general—at least worth trying. Dynamic blocks won't work, though.
There was a problem hiding this comment.
Using
savefor a reusable block might make sense in general—at least worth trying.
I explored this while first implementing reusable blocks. It causes a lot of visual inconsistencies due to the editor styles not working with the markup that is generated by save.
Is it expected that save should work in this way? If so, then perhaps our path forward is to do this and fix the editor styles for each and every block.
The reusable block type already tries to disable its children's inputs via a new Disabled component (#5223).
Could we have the HTML block detect that it's contained within a <Disabled>? Maybe container blocks like core/block and core/columns could define a context that blocks within it can detect? I suspect that we may encounter needs for such an API in the future, e.g. to style a block differently when it is a nested block.
|
Will try a different approach here. |
Description
Closes #4875.
When a HTML block is made reusable, and the reusable block is not being edited, we should not display the HTML.
How to test