-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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 transforms #39718
Quote v2: add transforms #39718
Conversation
Size Change: +442 B (0%) Total Size: 1.22 MB
ℹ️ View Unchanged
|
bb4a8f1
to
c4af3aa
Compare
@mtias What should happen to the pull quote block? Should that we a style variation of the quote block? Or why is that block still around? |
@oandregal Are e2e tests ran with the new quote block or when do we make that switch? It would be good to know what tests are passing and failing |
Hm, the raw transform is also not working for me 🤔 |
c4af3aa
to
ca50ee5
Compare
@ellatrix I'd like to keep this PR focused on the transforms so PR's size & concern is manageable :) The things I've seen that we need to do outside the transforms are listed here at the moment: interaction of the attribution button, deprecations, etc.
The tests would be enabled when #25892 is updated to make v2 the default. I can do that after this lands. |
I've just rebased to get #39729 in which fixes issues with filling the |
@ellatrix What html input are you using? |
ok, the only issue I see now is the focus on the |
Thoughts for the prefix transform
Given all this, I'd suggest we investigate this issue separately and do not block this PR by this issue. @ellatrix thoughts? |
The <figure>
<blockquote>
<p>Paragraph.</p>
<ul>
<li>list</li>
</ul>
<code>rm -rf</code>
</blockquote>
<figcaption>Author</figcaption>
</figure> It doesn't with the same code but without the |
I'm still seeing the paste issue when copying the content here: https://codepen.io/iseulde/pen/eYyvJbd There's no quote and the two paragraphs in the quote disappear. When I remove the image, it works as expected indeed. Let's try to figure this out separately then. |
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.
Let's merge 😄
This reverts commit 9c2650a.
9756835
to
f101a6c
Compare
Part of #15486
Related #25892
What?
This PR adds transforms to the Quote v2.
How?
*
transform.*
transform.>
prefix<blockquote><p /><cite /></blockquote>
<figure><blockquote/><figcaption /></figure>
. It ignores the cite from the old markup.*
)*
transform.*
transform.*
)Testing Instructions
Content for testing the `raw` transform
TODO
Merge Quote v2: update how attributes are registered #39729 (it updates how the block is registered and fixes the issues withattribution
).Update transforms to consider other attributes (anchor, align, etc). Specially when coming from pullquote.>
prefix transformation, investigate if we can make the focus to start in the inner content instead of in the wrapper. Before, the paragraph was focused and users could start typing. To be addressed in a follow-up.raw
transform:attribution
fromfigcaption
not working.<img src=""/>
element, it doesn't transfom it to a quote