-
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 a control per block to reset pattern overrides #57907
Conversation
attribute1 instanceof RichTextData && | ||
attribute2 instanceof RichTextData | ||
) { | ||
return attribute1.toString() === attribute2.toString(); |
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.
A bit of a gotcha here that if the attributes are different RichTextData
instances then ===
won't be true even though we expect them to be most of the time. 🤔
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.
I can't really think of another way, so this looks ok to me.
Size Change: +401 B (0%) Total Size: 1.69 MB
ℹ️ View Unchanged
|
d8e58d6
to
71f815a
Compare
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core SVNCore Committers: Use this line as a base for the props when committing in SVN:
GitHub Merge commitsIf you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Flaky tests detected in 71f815a. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7775121664
|
71f815a
to
a5eb4fe
Compare
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.
At the moment it's not quite working for me, the reset button always appears greyed out even after making edits:
I rebased and pushed a commit for an issue where the button wasn't showing at all (code needed to be updated for the new block bindings format), so it could be something similar.
I think it'd be good if the button could be moved to the end of the toolbar rather than the start, it's quite prominent at the moment.
attribute1 instanceof RichTextData && | ||
attribute2 instanceof RichTextData | ||
) { | ||
return attribute1.toString() === attribute2.toString(); |
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.
I can't really think of another way, so this looks ok to me.
a5eb4fe
to
a2abc1b
Compare
return blocks.find( ( block ) => { | ||
if ( block.attributes.metadata?.id === id ) { | ||
return block; | ||
} | ||
|
||
return recursivelyFindBlockWithId( block.innerBlocks, id ); | ||
} ); |
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.
@kevin940726 I don't think this does what's expected. blocks.find
will only ever return a top-level block since it gets the value from the current array that the callback returns truthy
for.
If you try a pattern where an overriden paragraph is nested in a group it doesn't work as expected.
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.
Oops! I don't know what I was thinking 😅. Fixed in 6ec77d7. Hopefully I'm not missing something obvious again! 🤞
6ec77d7
to
7421a50
Compare
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 testing well for me now, including with deeply nested overridden blocks
What?
Part of #53705 and #57751. Continued from #57845.
Add a control per block to reset pattern overrides.
Why?
As mentioned in #57751.
How?
Add a control and conditionally render it.
Testing Instructions
Follow the instructions in #53705 (comment) to create a pattern with overrides and add it to a post.
Screenshots or screencast
Block without overrides
Block with overrides