RichText: replace deprecated multiline prop with simple multiple instances #54310
Conversation
This file contains 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
ellatrix
added
[Feature] Rich Text
|
Size Change: +141 B (0%) Total Size: 1.52 MB
ℹ️ View Unchanged
|
ellatrix
force-pushed
the
remove/rich-text-multiline
branch
from
19141ec
to
9d82981
Compare
ellatrix
force-pushed
the
remove/rich-text-multiline
branch
from
9d82981
to
67c320c
Compare
ellatrix
changed the title
|
Flaky tests detected in 6adc72b. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6125506189
|
ellatrix
requested review from
fluiddot,
dcalhoun and
ajitbohra
as code owners
|
I like where this is going. So basically this replaces the There are obviously some major differences here:
That said, there's necessarily changes in behavior here for third-party blocks, I would love to be able to know if people are still using this and test these blocks to know if these blocks are still behaving ok. Do you think we should try to get this in 6.4 or leave a bit more time in the plugin to solidify it? |
|
@youknowriad The prop is deprecated, with removal scheduled for 6.3, so my goal with this PR is to have a decent fallback instead of just removing it where paragraphs would now be treated at formats, or to fall back to a preview. I wanted something more gentle, so this solution is pretty good, because you can still edit, merge and split paragraphs and so on. But it should absolutely not be seen as a replacement. I want people to move away from this and use inner blocks. I don't want a 1:1 mapping otherwise there's no incentive for people to move away. I suspect the remaining usage of multiline is very low, so I'd love to try it for 6.4. What's the point in waiting? If we get this in now, there's plenty of time for feedback. What do you think? |
I'm all for merging but I just want to ensure that we understand how this is going to impact folks. I'd love if we can somehow get a sense of usage and just test these plugins. Maybe the 6.4 lifecycle is sufficient for this though I don't know. |
|
I can't find much, I tried here: https://github.com/search?q=org%3Awp-plugins+RichText+multiline&type=code&p=1 There's a small existing test for multiline, but I'll add some more e2e tests for this fallback. |
youknowriad
approved these changes
Sep 11, 2023
fluiddot
added
the
Mobile App - Automation
|
Heads up that I opened the PR wordpress-mobile/gutenberg-mobile#6181 to run the full testing suite of the native mobile version. It will help us to detect any potential breakages to that version. |
|
Thanks @fluiddot, I'll keep an eye. |
youknowriad
added
the
Needs Dev Note
|
I added some e2e tests for a basic block registering with RichText multiline, testing typing, split and merge. |
ellatrix
requested review from
ajlende,
ryanwelcher,
juanmaguitar,
fabiankaegy and
ndiego
as code owners
|
Let's merge. I'm pretty confident that we can address issues if people who are still using it find it too rough. In my testing the change is pretty gentle though. |
|
Mobile tests are also passing. |
|
That's a really nice simplification! 🚀 |
10 tasks
This was referenced Oct 17, 2023
6 tasks
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.
What?
RichText's deprecated (!) multiline prop is a huge maintenance burden. It makes the rich text value much more complex and prevents us from doing a refactor to improve performance, refactor splitting/merging etc. It also makes the internal logic much more difficult to understand.
But we can't simply remove it and break people's blocks. There need to be some sort of fallback that works decently. The idea: a separate component that is multiple rich text instances with merge/split interop.
Yes, it's not a 1:1 mapping. Some things like paste will work differently, it will now be inline. But we can polish this if needed. This is still a deprecated prop after all, and block authors should be encouraged to move away from it and use InnerBlocks.
Why?
How?
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast