-
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: use nested blocks #25892
Quote: use nested blocks #25892
Conversation
I'm using this snippet for testing block deprecations: <!-- wp:paragraph -->
<p>Quote with single paragraph and cite:</p>
<!-- /wp:paragraph -->
<!-- wp:quote -->
<blockquote class="wp-block-quote"><!-- wp:paragraph -->
<p>Single paragraph.</p>
<!-- /wp:paragraph --><cite>Some cite.</cite></blockquote>
<!-- /wp:quote -->
<!-- wp:paragraph -->
<p>Quote with multiple paragraph and cite:</p>
<!-- /wp:paragraph -->
<!-- wp:quote -->
<blockquote class="wp-block-quote"><!-- wp:paragraph -->
<p>First paragraph.</p>
<!-- /wp:paragraph -->
<!-- wp:paragraph -->
<p>Second paragraph.</p>
<!-- /wp:paragraph --><cite>Some cite.</cite></blockquote>
<!-- /wp:quote -->
<!-- wp:paragraph -->
<p>Quote without cite:</p>
<!-- /wp:paragraph -->
<!-- wp:quote -->
<blockquote class="wp-block-quote"><!-- wp:paragraph -->
<p>Single paragraph.</p>
<!-- /wp:paragraph --></blockquote>
<!-- /wp:quote -->
<!-- wp:paragraph -->
<p>Quote without content:</p>
<!-- /wp:paragraph -->
<!-- wp:quote -->
<blockquote class="wp-block-quote"><cite>Some cite.</cite></blockquote>
<!-- /wp:quote -->
<!-- wp:paragraph -->
<p>Quote with single paragraph and cite (style large):</p>
<!-- /wp:paragraph -->
<!-- wp:quote {"className":"is-style-large"} -->
<blockquote class="wp-block-quote is-style-large"><!-- wp:paragraph -->
<p>Single paragraph.</p>
<!-- /wp:paragraph --><cite>Some cite.</cite></blockquote>
<!-- /wp:quote --> |
Size Change: -911 B (0%) Total Size: 1.25 MB
ℹ️ View Unchanged
|
Thanks for working on this! |
You'll also need to adjust the raw transform (paste) to parse as inner blocks instead. |
88e4a03
to
8a89f96
Compare
I think we need to.
You should also check the other |
Keep in mind that we should probably not be adding styles to the Quote block that specifically target other blocks via CSS classes, but rather target the underlying HTML tags. Remember that plugins can add their own heading, paragraph, and other textual content blocks. A selector for |
8a89f96
to
544940e
Compare
6198011
to
74bb68a
Compare
Doing a status update of latest changes in the PR. At current this PR looks like this: This looks good! It is very helpful having nested blocks! Ps. I just want to mention another block that could also used nested blocks. The List block: #6394 |
@enriquesanchez the allowed blocks are the ones suggested here: paragraph, headings, list, code. My understanding is that we want cite to have a stable place within the quote and be always shown, so it's not a block but a field of the blockquote. |
A very common use case will be to cite content that includes images, so I'd suggest including those in the allowed list. What's the primary motivator for limiting content? The quote element itself allows a wide range of flow content. https://developer.mozilla.org/en-US/docs/Web/Guide/HTML/Content_categories#Flow_content |
@@ -98,14 +100,15 @@ describe( 'Quote', () => { | |||
await insertBlock( 'Quote' ); | |||
await page.keyboard.press( 'ArrowRight' ); | |||
await page.keyboard.type( 'cite' ); | |||
await transformBlockTo( 'Paragraph' ); | |||
await transformBlockTo( 'Unwrap' ); |
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 test is failing because after ArrowRight
you can't type a cite
. See #25892. We need to either fix this keyboard interaction or adapt the test.
await transformBlockTo( 'Heading' ); | ||
expect( await getEditedPostContent() ).toMatchSnapshot(); | ||
} ); | ||
|
||
it( 'can be converted to a pullquote', async () => { |
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 test is failing because after ArrowRight
you can't type a cite
. See #25892. We need to either fix this keyboard interaction or adapt the test.
Can we move the cite to be a button in the Quote toolbar, which once you click adds and focuses the cite? If the cite is left empty, when the block is unselected, or another block is focused within the block, the cite element should disappear (no placeholder). I think that'd help with the "Go to cite after adding content" so we can push this through. |
8b9e4b3
to
ccdce3a
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.
Fixed all the remaining tests. Looks good now.
I've marked this as "needs dev note" for authors to be aware of the changes. I haven't done a before/after review of the markup (it should be the same in principle) but a couple of things that stand out to highlight are:
|
What?
Fixes #15486.
This PR enables the quote block to use inner blocks, meaning that it can have any block within beyond the paragraph.
Why?
Allowing users to add any kind of content to quote is a much requested feature that we've been tracking at #15486
We've tried this before #6520 but the interaction wasn't a great model for this block at the time.
How
We've made the block support inner blocks without any other changes (no changes to markup, etc).
Note this PR has gone through multiple iterations and multiple people have contributed to it. The last push was to merge it in small PRs behind a flag so it didn't interfere with the existing quote block until we were ready to enable the v2 version:
This is a comparison of the keyboard interactions between v1 and v2:
Testing
>
).TODO