-
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
RichText: Switch from disableEditing to standard html readonly attribute #60327
Conversation
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Size Change: +1.26 kB (0%) Total Size: 1.72 MB
ℹ️ View Unchanged
|
Flaky tests detected in 262fe03. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/8502126181
|
@@ -111,7 +111,7 @@ export function RichTextWrapper( | |||
__unstableDisableFormats: disableFormats, | |||
disableLineBreaks, | |||
__unstableAllowPrefixTransformations, | |||
disableEditing, | |||
readonly, |
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.
Should it be readOnly
instead? O
uppercase.
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.
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.
Thanks, fixed.
return ( | ||
<PrivateRichText ref={ ref } { ...props } disableEditing={ false } /> | ||
); | ||
return <PrivateRichText ref={ ref } { ...props } readonly={ false } />; |
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.
When do you think it could get public?
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.
@ellatrix what are your thoughts on this? It seems like a reasonably low-risk thing to add to the public API. Should we leave it private for a plugin release or two and aim to have it public for 6.6?
@retrofox I assume it is not much use to you until it is in a WP core release as many woo users won't be using the GB plugin, so no panic to have it public before 6.6?
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.
@glendaviesnz @retrofox Are you stil interested in making the readOnly
attribute public? Does Woo have a use case for it?
It's been 8 months since it's been added, 6.6 and 6.7 releases passed by and now we're working on 6.8.
…ute (WordPress#60327) Co-authored-by: glendaviesnz <glendaviesnz@git.wordpress.org> Co-authored-by: ellatrix <ellatrix@git.wordpress.org> Co-authored-by: retrofox <retrofox@git.wordpress.org>
What?
Implements suggestion here to switch to using a standard html attrib to flag RichText field as read only.
Why?
This is a more standard way of indicating this, and will make a better public API if we move this from private.
How?
Renames prop from
disableEditing
toreadonly
Testing Instructions