Parser: Update validateBlock to use fixedBlock #62178
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tjcafferkey
added
[Type] Bug
|
Size Change: +118 B (+0.01%) Total Size: 1.74 MB
ℹ️ View Unchanged
|
|
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 If 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. |
gziolo
added
[Feature] Parsing
ockham
added
Backport to WP 6.6 Beta/RC
|
Adding labels to mark as potential candidate for WP 6.6. |
ockham
force-pushed
the
fix/apply-block-validation-to-fixed-block
branch
from
4af4d61
to
2fa8116
Compare
|
It would have been helpful to have testing instructions or testing coverage for this path, but based on the original code, the fix looks correct. |
If that was an issue with refactoring then we definitely should add a test that will prevent future regressions. |
|
Thank you, @tjcafferkey! The quick test shows that the new unit test also passes on |
youknowriad
reviewed
Jun 4, 2024
| }, | ||
| save: ( { attributes } ) => ( | ||
| // eslint-disable-next-line react/no-unknown-property | ||
| <div class={ attributes.className }> |
There was a problem hiding this comment.
If I'm not wrong there's no need to use attributes.className here as it should be done automatically no?
There was a problem hiding this comment.
I thought so too, but apparently not.
|
This change is logical indeed. Makes me wonder how useful the built-in fixes are (This bug is around for some time now) |
Huh, @Mamaduka it looks like it's passing on The test would fail on |
|
@tjcafferkey I guess that means that this PR actually doesn't do anything but clarify things. It's good anyway but at least we know that the fixes are useful and used :P |
youknowriad
added
[Type] Code Quality
|
@youknowriad ha yes. do you think I should update |
|
@tjcafferkey I think so, assuming there's no perf impact or anything, it's better to avoid mutating function arguments like that. |
youknowriad
approved these changes
Jun 4, 2024
Mamaduka
approved these changes
Jun 4, 2024
|
Flaky tests detected in 246eb14. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/9369233307
|
patil-vipul
pushed a commit
to patil-vipul/gutenberg
that referenced
this pull request
Jun 17, 2024
* Run validateBlock against fixedBlock * Unit test for block validation applying customClassNames * Add another custom class to the unit test * Avoid mutating blockAttributes
dmsnell
reviewed
Jun 27, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #62176
What?
Updated the second
validateBlockexecution to run againstfixedBlockafter we have attempted to apply fixes if it fails validation for the first time.Why?
It appears as though we are revalidating the original
unvalidatedBlockinstead offixedBlockduring the second execution ofvalidateBlock().How?
Updated the variable being passed to
validateBlockTesting Instructions
Testing Instructions for Keyboard
Screenshots or screencast