Fix deletion of custom block classes#8232
Merged
Merged
Conversation
7abf7c0 to
fd92be9
Compare
If you add a custom class to a block and then remove that custom class it would flag the block as invalid. The reason is that the custom class became part of the blocks ‘default’ attributes, and it would then add it back and this then the expected HTML wouldn’t match the current HTML. This change ensures the block contains the default class names + whatever classes are currently in the HTML, ignoring any custom ones in the block itself. It also handles the situation where the block's default class doesn't exist in the HTML The unit tests have been beefed up to cope with several edge case situations
fd92be9 to
5d986b5
Compare
Contributor
Author
aduth
approved these changes
Aug 3, 2018
aduth
left a comment
Member
There was a problem hiding this comment.
Thanks for the ping @johngodley . I agree this is a bug. I think there might be an easier approach here in retrieving the custom classes as simply the difference between innerHTML and the default block (i.e. a block serialized with a className attribute of undefined). I've done this in 1f2a7a1 . All tests still pass.
Let me know what you think. On my end, this is good to go 👍
Contributor
Author
|
That's great, thanks! It seemed like there should be a simpler way but I wasn't entirely sure what happened beyond this code. Merging. |
Contributor
|
It looks like this behavior regressed custom classes for dynamic blocks because they don't have any serialized content #9991 |
This file contains hidden or 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
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.
If you add a custom class to a block and then remove that custom class it will flag the block as invalid. This is because the custom class becomes part of the block attributes, and is then added back by the 'custom class name' filter. The expected HTML then doesn't match the current HTML, and a warning is triggered.
You can reproduce the problem as follows:
<blockquote>:<blockquote class="wp-block-quote newclass”><p>fdsfsdfsdd</p></blockquote>newclassThis may or may not be expected behaviour, but it seems like removing the custom class shouldn't trigger a warning, and so I've treated it as a bug.
This PR ensures the block contains the default class names + whatever classes are currently in the HTML, ignoring any custom ones in the block itself.
The unit tests have been beefed up to cope with several edge case situations
Fixes #8131
Note that I don't fully understand the context of this change, and I may totally have misunderstood the code. The change keeps the existing unit tests running while fixing the additional cases of removing classes. I'm not sure if this is the purpose of the code, so the fix may be misplaced.
How has this been tested?
Existing unit tests work, and additional unit tests have been added. Manual verification will show the problem in different situations:
<p class="test">paragraph</p>And then remove the class to trigger an invalid block warning
<blockquote class="wp-block-quote a b c"><p>fdsfsdfsdd</p></blockquote>Then remove class
bto trigger an invalid block warning. A variation of above, but worth testingTypes of changes
This is a bug fix that changes some core block HTML code and so could potentially be a breaking fix.
Checklist: