-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Add template preview to the edit site template switcher #20958
Conversation
Size Change: +1.01 kB (0%) Total Size: 858 kB
ℹ️ View Unchanged
|
@@ -61,7 +61,7 @@ class InnerBlocks extends Component { | |||
// Set controlled blocks value from parent, if any. | |||
if ( __experimentalBlocks ) { | |||
__unstableMarkNextChangeAsNotPersistent(); | |||
replaceInnerBlocks( __experimentalBlocks ); | |||
replaceInnerBlocks( __experimentalBlocks, false ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The template part block was stealing the focus on the first render causing the popover to close. You can also verify this on master by loading the edit site page and notice that the template part block gets focused immediately on load. cc @aduth
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trying to recall what the original use-case is for focus to shift when replacing a block. I guess for non-nested blocks, it helps support cases like transforming a block from one to another (using /
slash insert to insert a quote -> type immediately in the quote). Struggling to think if it makes as much sense in the InnerBlocks case, or at least as a default.
The only other use in this function is in synchronizing the template. Did you determine if it makes sense there? I might imagine we'd have similar issues of stealing focus?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only other use in this function is in synchronizing the template. Did you determine if it makes sense there? I might imagine we'd have similar issues of stealing focus?
I think @jorgefilipecosta introduced an explicit prop about it at some point. I might think we can always default to "false" but I'm not 100% certain 😬
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In most cases when we add a block using a template for inner blocks the focus goes to the first focusable block. O added a flag because for some blocks people referred this behavior is not desirable e.g: for the media & text block.
@@ -154,7 +154,7 @@ function InserterBlockList( { | |||
} | |||
|
|||
const onHoverItem = ( item ) => { | |||
onHover(); | |||
onHover( item ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a bug in the inserter, already fixed master
Maybe this helps for the new/plus icon |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's keep this moving.
closes #20478
This PR adds a preview panel similar to the inserter one to the template switcher on the edit-site package.