PlainText: Add `multiline` and `isStylable` props #18238

Copons · 2019-11-01T15:15:54Z

Description

Add two new props to the PlainText component.
- multiline: keeps the previous behaviour of allowing line breaks.
  If not set, it prevents line breaks in two ways: it replaces \n\r characters with spaces, and on press ENTER it insert a new default block instead.
- isStylable: prepares the textarea to be styled with block attributes (e.g. color and background color).
  It also set the default background color of the textarea as transparent, to match the editor background color set by the theme.
Update the SiteTitle block to be a "stylable, single line" PlainText.
Update the Code, HTML, and Shortcode blocks (only ones that currently use PlainText) to be multiline PlainText, which is their original behaviour.

Notes

isStylable sounds very awkward, I know!
I also know that it currently is hard to see its point, but I didn't want to add too much stuff in the SiteTitle component yet, to avoid making this PR more confusing.
Though, its purpose can be tested by adding the .has-background.has-accent-background-color classes (in Twenty Twenty) to the textarea to simulate the background color block attribute.
I've chosen to add a backward-incompatible multiline instead of something like singleLine to keep the naming consistent with RichText etc. Though, I don't have a strong opinion on this, and I wouldn't be opposed to "invert" it.

How has this been tested?

Tested on the Gutenberg Docker with the Full Site Experiment enabled.
Checked if the new props caused regressions in other uses of the PlainText component.

Types of changes

Breaking change

Checklist:

My code is tested.
My code follows the WordPress code style.
My code follows the accessibility standards.
My code has proper inline documentation.
I've included developer documentation if appropriate.

…her components

youknowriad · 2019-11-02T10:45:44Z

packages/block-library/src/site-title/edit.js

 				placeholder={ __( 'Site Title' ) }
 				value={ title }
 				onChange={ setTitle }
-				allowedFormats={ [] }
+				isStylable
+				onKeyDown={ preventNewlines }


I think instead of onKeyDown here, we might want to align the API with how RichText works, I think it's onSplit or something like that cc @ellatrix

The difference here is that onSplit is an actual RichText method created for handling a block split, while in this case I'm just passing the native onKeyDown handler through to the textarea element.

This is another reason why I think it would be better to do this through (Rich)Text. The API's already there, and they would be the same.

packages/block-editor/src/components/plain-text/index.js

youknowriad · 2019-11-02T10:49:01Z

packages/block-editor/src/components/plain-text/index.js

+	onChange,
+	className,
+	multiline,
+	isStylable,


Should this be a prop, or should this be by default? I mean if users of PlainText don't want the styled behavior, they can always use TextAreaAutosize or just set styles on the wrapper? or do you think this behavior is too common to warrant a prop?

This is a good question.
Mainly, I wanted to affect as little as possible the other components using PlainText, to avoid introducing visual regressions that might be hard to catch.

At the same time, those other components currently use PlainText exactly like a normal textarea (plus the auto-resize, of course)... 🤔

ellatrix · 2019-11-02T10:51:12Z

multiline: keeps the previous behaviour of allowing line breaks.
If not set, it prevents line breaks in two ways: it replaces \n\r characters with spaces, and on press ENTER it insert a new default block instead.

Why not just use an input element?

youknowriad · 2019-11-02T10:56:02Z

Why not just use an input element?

you still want the autosize behavior. Not sure how easy/redundunt this would be with an input.

ellatrix · 2019-11-02T11:04:54Z

In that case, I'd recommend using RichText without any formatting, which has native autoresizing. Maybe we should consider wrapping it in another component for such use cases. The cool thing would be that you still have access to all other APIs such as splitting, autocomplete text... all without the text formatting.

youknowriad · 2019-11-02T11:21:36Z

Maybe we should consider wrapping it in another component for such use cases. The cool thing would be that you still have access to all other APIs such as splitting, autocomplete text... all without the text formatting.

That's a great argument, I feel we shouldn't use RichText though as semantically it's about "Rich Text", and using code, you can always pass content with formatted content and it will be rendered as formatted content. It definitely means there's a lot of refactoring needed to extract/share things between a base Text component and two specialized RichText and PlainText. This is though a bigger refactor than just expanding the capabilities of PlainText step by step.

Copons · 2019-11-04T12:00:16Z

Why not just use an input element?

you still want the autosize behavior. Not sure how easy/redundunt this would be with an input.

@ellatrix @youknowriad AFAIK there is no way to make an input element wrap the text inside.

I'd recommend using RichText without any formatting

Yes, I really agree with you, and I argued for this solution at length in this thread: #17207 (review)
(and also anecdotally it's what I did weeks ago for the SiteTitle block in WordPress.com)

Though, it is my understanding that the proposed direction though would be to keep things as semantically correct as possible, so I went ahead with a very light implementation of a resizable single-line PlainText, slightly inspired by PostTitle.

It definitely means there's a lot of refactoring needed to extract/share things between a base Text component and two specialized RichText and PlainText.

To be honest at this point I don't think there is a need for such thing.
Since RichText already solves all the problems of any type of text input, it might make more sense to rename rich-text into text, and replace PlainText with a direct import of TextareaAutosize... 😅

ellatrix · 2019-11-06T15:17:39Z

you can always pass content with formatted content and it will be rendered as formatted content.

Not necessarily. In a normal text version, we wouldn't allow any formatting.

It definitely means there's a lot of refactoring needed to extract/share things between a base Text component and two specialized RichText and PlainText. This is though a bigger refactor than just expanding the capabilities of PlainText step by step.

The way I see it, this wouldn't be a big refactor at all.

I'm not sure if I'll find the time right now, but I can try to spin up a PR today or tomorrow.

Copons · 2019-11-06T15:37:57Z

I'm not sure if I'll find the time right now, but I can try to spin up a PR today or tomorrow.

@ellatrix I'd be very happy to see that! Let me know if I can be of any help, or just add me as reviewer. 🙇

ellatrix · 2019-11-07T10:27:03Z

#18361 introduces a Text component, which is RichText under the hood, but doesn't allow any formatting. In other words, it behaves just like a textarea when rendering the value.

<Text value="some <b>tags</b>" />

will be editable as

some <b>tags</b>

(normally those tags would be interpreted as formatting)

noahtallen · 2020-01-25T00:03:36Z

@Copons should this PR be closed now?

Copons · 2020-01-27T11:47:40Z

@noahtallen Yes, let's see if the proper Text component gets further. 🙂

Copons added 6 commits November 1, 2019 14:55

Add an isStylable prop to the PlainText component

2282abc

Add a isSingleLine props to PlainText

96582e3

Make the SiteTitle a PlainText component

0d51e1a

Polish and docs

cead008

Rename isSingleLine into multiline to make it more consistent with ot…

bc140b3

…her components

Don't make the background transparent if a block attribute defines it

c000551

Copons added [Package] Block library /packages/block-library [Package] Block editor /packages/block-editor [Feature] Full Site Editing labels Nov 1, 2019

Copons requested a review from epiqueras November 1, 2019 15:15

Copons requested review from ajitbohra, ellatrix, jorgefilipecosta, Soean, talldan and youknowriad as code owners November 1, 2019 15:15

Copons self-assigned this Nov 1, 2019

youknowriad reviewed Nov 2, 2019

View reviewed changes

packages/block-editor/src/components/plain-text/index.js Outdated Show resolved Hide resolved

youknowriad reviewed Nov 2, 2019

View reviewed changes

Copons mentioned this pull request Nov 6, 2019

Full Site Editing: Add experimental SiteDescription block #18241

Closed

5 tasks

Default multiline = true

53b27df

ellatrix mentioned this pull request Nov 7, 2019

Editable Component #18361

Merged

5 tasks

Copons closed this Jan 27, 2020

aristath deleted the update/site-title-attributes branch November 10, 2020 14:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PlainText: Add `multiline` and `isStylable` props #18238

PlainText: Add `multiline` and `isStylable` props #18238

Copons commented Nov 1, 2019

youknowriad Nov 2, 2019

Copons Nov 4, 2019 •

edited

Loading

ellatrix Nov 6, 2019

youknowriad Nov 2, 2019

Copons Nov 4, 2019

ellatrix commented Nov 2, 2019

youknowriad commented Nov 2, 2019

ellatrix commented Nov 2, 2019

youknowriad commented Nov 2, 2019 •

edited

Loading

Copons commented Nov 4, 2019

ellatrix commented Nov 6, 2019

Copons commented Nov 6, 2019

ellatrix commented Nov 7, 2019 •

edited

Loading

noahtallen commented Jan 25, 2020

Copons commented Jan 27, 2020

PlainText: Add multiline and isStylable props #18238

PlainText: Add multiline and isStylable props #18238

Conversation

Copons commented Nov 1, 2019

Description

Notes

How has this been tested?

Types of changes

Checklist:

youknowriad Nov 2, 2019

Choose a reason for hiding this comment

Copons Nov 4, 2019 • edited Loading

Choose a reason for hiding this comment

ellatrix Nov 6, 2019

Choose a reason for hiding this comment

youknowriad Nov 2, 2019

Choose a reason for hiding this comment

Copons Nov 4, 2019

Choose a reason for hiding this comment

ellatrix commented Nov 2, 2019

youknowriad commented Nov 2, 2019

ellatrix commented Nov 2, 2019

youknowriad commented Nov 2, 2019 • edited Loading

Copons commented Nov 4, 2019

ellatrix commented Nov 6, 2019

Copons commented Nov 6, 2019

ellatrix commented Nov 7, 2019 • edited Loading

noahtallen commented Jan 25, 2020

Copons commented Jan 27, 2020

PlainText: Add `multiline` and `isStylable` props #18238

PlainText: Add `multiline` and `isStylable` props #18238

Copons Nov 4, 2019 •

edited

Loading

youknowriad commented Nov 2, 2019 •

edited

Loading

ellatrix commented Nov 7, 2019 •

edited

Loading