Heading: Add block classname deprecation #46138
Merged
Conversation
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
aaronrobertshaw
added
[Type] Bug
|
Size Change: +95 B (0%) Total Size: 1.32 MB
ℹ️ View Unchanged
|
aaronrobertshaw
force-pushed
the
add/heading-classname-deprecation
branch
from
ab037fa to
68ac06b
Compare
Contributor
|
Thank you! I haven't gotten a chance to review yet, but this looks great. As a side note...
@dmsnell is this something that your block API work would solve in the general case? |
andrewserong
approved these changes
Nov 30, 2022
Contributor
There was a problem hiding this comment.
Thanks for following up with this PR @aaronrobertshaw!
✅ The added deprecation appears to work as expected and the errors are no longer present in the console
❓ The order of the deprecations in deprecated.js is now ascending in structure, whereas it looks like other deprecated.js files in the repo are descending (with the latest version coming first in the file). E.g. Navigation, and Cover. Should we do the same here? (Though I see the docs have it ascending 🤔)
It looks like the deprecations are correctly named here (e.g. v1 is the earliest of the deprecations, and v5 is the latest, and the const deprecated statement sets them in the correct order), so this question is only about the order of the functions in the file, which doesn't make a difference in the run-time.
Other than that, and given that Adam confirmed we'd be sticking to storing the heading classname in post content for now (#42122 (comment)) this LGTM! ✨
Contributor
Author
|
Thanks for reviewing this @andrewserong 👍
I'd say there are a couple of reasons I went with this order:
As you note, the order in which the deprecations are defined makes no difference, only the order they appear in the I'll merge this now to eliminate the block validation errors on trunk. If there is consensus on the order deprecations are defined, we can update this and the docs or all the other block deprecations in a follow-up. |
Contributor
Sounds good, and I agree that it feels more natural to place the latest deprecation at the end of the file, and it should be pretty clear for the next person based on the naming of each of the deprecation versions. Thanks for landing this one! |
mpkelly
pushed a commit
to mpkelly/gutenberg
that referenced
this pull request
Dec 7, 2022
bph
added
the
Needs Dev Note
adamziel
added
has dev note
Contributor
|
Collective dev note in #42122 |
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.
Related:
What?
Adds missing deprecation for Heading block after adding of block classname via supports.
Why?
Without the deprecation old heading blocks throw block validation errors.
How?
Testing Instructions
npm run test:unit test/integration/full-content/full-content.test.jsExample deprecated heading content:
NOTE: It might be easiest to review the code changes by commit. The first commit (bc5fc1c) includes the new deprecation and the updated fixtures. The second commit (68ac06b) refactors the deprecations into the currently recommended format.