-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Quote v2: add innerblocks support #39704
Conversation
1c79969
to
fc9fae9
Compare
Size Change: +313 B (0%) Total Size: 1.21 MB
ℹ️ View Unchanged
|
"keywords": [ "blockquote", "cite" ], | ||
"textdomain": "default", | ||
"attributes": { | ||
"attribution": { |
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.
Why not just reuse the same attribute name as the existing quote "citation"?
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.
(reasoning being to make the least possible breaking changes)
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 original discussion #25892 (comment)
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 problem is that it's a breaking change for folks using quotes in "CPT templates". That said, I think the problem will still exist for folks using the "value" attribute which now becomes inner blocks. So would be good to provide a "migration" for these templates. We don't have "migrations" at the framework level but maybe we can start adhoc:
- Leave the old and new attributes
- if we detect a block with the old attributes, use a
useEffect
or something to migrate the attributes.
The ideal scenario would be to implement "migrations" in addition to "deprecations".
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.
Even if we maintain the name, the selector has changed from cite
to figcaption
, so it's going to be a breaking change for anyway.
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.
For the selector change, the deprecate handles it for us but we don't have a migration mechanism when the starting point is a block object, not markup.
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.
Oh, thanks for the clarification.
29dc699
to
c9e4e17
Compare
Co-authored-by: Ella van Durpe <4710635+ellatrix@users.noreply.github.com>
Co-authored-by: Dennis Snell <dennis.snell@automattic.com> Co-authored-by: Ari Stathopoulos <aristath@gmail.com>
fc9fae9
to
7de5a5f
Compare
@youknowriad @ntsekouras your feeback was addressed. Do you think we can land this to continue iterating? There's two changes that can be worked in parallel when this lands (see issue description for more details): bring the transformations + iterate on the attribution behavior (focus, etc). |
Using a different |
Part of #15486
Related #25892
Depends on #39703
What?
This PR makes the Quote v2 support inner blocks.
How?
The markup for this block behaves this way: without an attribution a quote is a simple
<blockquote />
but with an attribution we follow the HTML spec with<figure><blockquote /><ficaption /></figure>
.I've added as co-authors all the people that had contributed to the original PR.
Testing Instructions
quote-block-markup.mp4
Follow-ups
As per the conversations at #25892 we want the attribution to behave like this:
By defering this to a follow-up we can unblock changes to bring transformations in parallel.