-
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
Rich text: log deprecation message for multiline prop #43613
Conversation
Size Change: +97 B (0%) Total Size: 1.24 MB
ℹ️ View Unchanged
|
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.
We should consider a timeline if we want to actually remove multiline machinery in the future. The sooner we can add a version:
option to the deprecated
call, the better. Aside from that, I'm good with this. :)
@@ -115,6 +115,14 @@ function RichTextWrapper( | |||
}, | |||
forwardedRef | |||
) { | |||
if ( multiline ) { | |||
deprecated( 'wp.blockEditor.RichText multiline prop', { | |||
since: '14.1', |
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.
Searching through the codebase, I'd say the convention is to use WP versions here. The only two instances of Gutenberg version numbers (12.6 and 12.7) are in deprecated
calls with the accompanying plugin: 'Gutenberg'
option.
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. Adjusted it to use WP version numbers and added a version prop.
if ( multiline ) { | ||
deprecated( 'wp.blockEditor.RichText multiline prop', { | ||
since: '6.1', | ||
version: '6.2', |
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.
I think 6.2 is way too close. Even though everything suggests that the multiline option was minimally used in the ecosystem, it has been part of a stable API for a long time.
For comparison, take the migration of classnames in 5.2 following the introduction of the block editor module. The classes were never even a public API, but we still gave it a year (which was very generous, IMO).
So I think 6.3 should be the absolute minimum.
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.
Alright :) I think it depends in how much it's communicated, but I agree that a bit longer is probably best.
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.
Looks good, thanks Ella!
Thank you! |
What?
Deprecates the
multiline
prop forRichText
. As of #42711, we're no longer using this in core.Why?
Handling multi line content in RichText is quite complex.
create
andtoTree
functions to convert between DOM/HTML and the rich text value.Deprecation would mean that after a WP release we can consider removing this code, which makes rich text easier to maintain and makes refactors easier to do.
For example, #29870 would be much more work if we have to take into account multi line values.
How?
This PR logs a simple message. We'd also have to write a dev not explaining how to change code to InnerBlocks. It's probably also a good idea to have a dedicated message in the email sent to plugin authors every release for extra awareness.
Testing Instructions
Edit a block to using RichText multiline and check the console.
Screenshots or screencast