Add Textarea component by rezrah · Pull Request #1812 · primer/react

rezrah · 2022-01-20T15:13:41Z

Adds a Textarea component.

Try it here: Docs | Storybook

Screenshots

Merge checklist

…rea-form-component

changeset-bot · 2022-01-20T15:13:43Z

🦋 Changeset detected

Latest commit: 06acbaf

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@primer/react	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

github-actions · 2022-01-20T15:17:57Z

size-limit report 📦

Path	Size
dist/browser.esm.js	61.64 KB (+0.49% 🔺)
dist/browser.umd.js	62 KB (+0.46% 🔺)

mperrotti · 2022-01-20T16:23:42Z

+ SxProp
+
+const StyledTextArea = styled.textarea<TextAreaProps>`
+ min-height: 124px;


Where does "124px" come from?

It's the height that's set on the text area component in Figma. Took it from there.

I've now removed this min-height, so the user is forced to set their own. Was the textarea in Figma just an example, or intentional min-height @mperrotti?

If we end up using rows instead of height, I'd set the a default value for rows instead of specifying a min-height.
Otherwise, just make sure 124px perfectly fits {n} lines of text and doesn't cut off any lines of text in their vertical middle.

mperrotti · 2022-01-20T16:26:52Z

+} & TextareaHTMLAttributes<HTMLTextAreaElement> &
+ SxProp
+
+const StyledTextArea = styled.textarea<TextAreaProps>`


The Select and TextInput component both just use the private TextInputWrapper component to maintain consistent styling. I'd recommend we do that here too.

I did think about this, and decided against it as that component had styling that wasn't relevant for this component, like warning state, size variants etc. Do we want to inherit those here? It feels like we'd be creating a dependency here for the sake of convenience, but potentially more detrimental to the user as we're loading unnecessary css and subsequently delivering a less performant component.

You're right - the size variants aren't relevant. Most of the others are though:
Styling prop Used for TextArea?
disabled ✅
hasLeadingVisual ❌
hasTrailingVisual ❌
block ✅
contrast ✅
validationStatus ✅
size ❌

I still think we should DRY this by using TextInputWrapper or something that replaces it. It's unlikely somebody is going to load the CSS for a TextArea and not also have a component on the page that loads TextInputWrapper styles (e.g.: TextInput or Select). Then, we'd downloading nearly identical styles twice.

colebemis · 2022-01-20T18:36:19Z

+ description="Indicates to the user and assistive technologies that the field value is required"
+ />
+ <PropsTableRow
+ name="block"


I wonder if we should remove the block prop in favor of sx={{width: '100%'}} (in both TextArea and TextInput) cc @mperrotti

I'd be down with that if that's all block is doing

Feels like a very common use-case. Are we over-relying on sx here to handle scenarios many users would expect to be done via props?

I'm okay with a prop for this use case if we think it's pretty common. I'm not the biggest fan of block as a name for this though. What do y'all think?

I agree that block is a little unclear, but I'd prefer we change the naming for all components that use block if we choose to make it different here.

…rea-form-component

mperrotti

Everything is looking good, there's just that one styling sugggestion.

mperrotti · 2022-01-27T15:20:56Z

+} & TextareaHTMLAttributes<HTMLTextAreaElement> &
+ SxProp
+
+const StyledTextArea = styled.textarea<TextAreaProps>`


I still think we should DRY this by using TextInputWrapper or something that replaces it. It's unlikely somebody is going to load the CSS for a TextArea and not also have a component on the page that loads TextInputWrapper styles (e.g.: TextInput or Select). Then, we'd downloading nearly identical styles twice.

…rea-form-component

…textarea

rezrah · 2022-01-31T14:07:44Z

+const textInputBasePadding = '12px'
+export const textInputHorizPadding = textInputBasePadding
+
+export const TextInputBaseWrapper = styled.span<StyledBaseWrapperProps>`


In order to reuse TextInputWrapper, I've abstracted the base styles into TextInputBaseWrapper, which apply universally to all types of text input, including textarea.

rezrah · 2022-01-31T14:09:36Z

@@ -1 +1 @@
-export type FormValidationStatus = 'error' | 'success'
+export type FormValidationStatus = 'error' | 'success' | 'warning' // TODO: remove warning as a breaking change. Added for backwards compatibility with TextArea


Not ideal, but this avoids a breaking change to TextInput by leaving warning in play. I'd recommend we use the upcoming form validation work to remove warning altogether and defer the breaking change.

mperrotti

Two small comments about styling, but otherwise LGTM! 🎉

colebemis · 2022-01-31T16:55:16Z

My strong opinion weakly held is that we should call this component Textarea instead of TextArea. Since this component is a simple wrapper around the HTML <textarea> element, <Textarea> seems more intuitive than <TextArea>.

mperrotti · 2022-01-31T16:59:01Z

FWIW, I also prefer Textarea, but it's also my strong opinion weakly held

…rea-form-component

rezrah · 2022-02-01T15:09:44Z

@mperrotti @colebemis FYI, I've renamed everything to Textarea. Seems there's ecosystem alignment for this too, so happy to make that change.

rezrah added 2 commits January 20, 2022 15:09

feat: add TextArea component
e14195b

Merge branch 'main' of github.com:primer/react into feature/add-texta…
ae8204d
…rea-form-component

rezrah requested review from a team and colebemis January 20, 2022 15:13

chore: fix lint errors
e823162

mperrotti reviewed Jan 20, 2022
View reviewed changes

colebemis reviewed Jan 20, 2022
View reviewed changes

siddharthkp reviewed Jan 21, 2022
View reviewed changes

Comment threaddocs/content/TextArea.mdx Outdated

siddharthkp reviewed Jan 21, 2022
View reviewed changes

Comment threaddocs/content/TextArea.mdx

siddharthkp reviewed Jan 21, 2022
View reviewed changes

Comment threaddocs/content/TextArea.mdx

rezrah added 9 commits January 21, 2022 11:20

fix: typos and content
c5169c6

chore: update docs
3e8f084

fix: tests
fbdfa7c

chore: remove width and height
470e110

chore: fix typo in docs
36ba42a

docs: add ref row to TextArea
345fabb

Merge branch 'main' of github.com:primer/react into feature/add-texta…
90d0ff0
…rea-form-component

chore: fix typo in docs
7a4f8e4

chore: remove defaultValue
1b2f772

mperrotti reviewed Jan 27, 2022
View reviewed changes

rezrah added 3 commits January 27, 2022 15:51

Merge branch 'main' of github.com:primer/react into feature/add-texta…
fd048cb
…rea-form-component

feat: refactor TextInputWrapper to separate base styles and reuse in …
150e441
…textarea

fix: tests
165190b

rezrah commented Jan 31, 2022
View reviewed changes

rezrah requested a review from mperrotti January 31, 2022 14:09

mperrotti approved these changes Jan 31, 2022
View reviewed changes

Comment threadsrc/TextArea.tsx Outdated
Comment threadsrc/TextArea.tsx

colebemis added the react label Jan 31, 2022

colebemis assigned mperrotti and rezrah Jan 31, 2022

rezrah added 4 commits February 1, 2022 13:36

Merge branch 'main' of github.com:primer/react into feature/add-texta…
e0566d6
…rea-form-component

feat: rename TextArea to Textarea
c13284a

fix: move not-allowed to base wrapper
71673e1

fix: rename file properly
06acbaf

rezrah changed the title ~~Add TextArea component~~Add Textarea componentFeb 1, 2022

rezrah merged commit 97bf7c6 into mainFeb 1, 2022

rezrah deleted the feature/add-textarea-form-component branch February 1, 2022 15:11

primer-css mentioned this pull request Feb 1, 2022
Release Tracking #1825
Merged

Styling prop	Used for TextArea?
disabled	✅
hasLeadingVisual	❌
hasTrailingVisual	❌
block	✅
contrast	✅
validationStatus	✅
size	❌

		@@ -1 +1 @@
		export type FormValidationStatus = 'error' \| 'success'
		export type FormValidationStatus = 'error' \| 'success' \| 'warning' // TODO: remove warning as a breaking change. Added for backwards compatibility with TextArea

Uh oh!

Conversation

rezrah commented Jan 20, 2022• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Screenshots

Merge checklist

Uh oh!

changeset-botBot commented Jan 20, 2022• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

github-actionsBot commented Jan 20, 2022• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

size-limit report 📦

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mperrotti left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mperrotti left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

colebemis commented Jan 31, 2022

Uh oh!

mperrotti commented Jan 31, 2022

Uh oh!

rezrah commented Feb 1, 2022

Uh oh!

Reviewers

rezrah commented Jan 20, 2022•
edited
Loading

changeset-botBot commented Jan 20, 2022•
edited
Loading

github-actionsBot commented Jan 20, 2022•
edited
Loading

mperrotti left a comment •
edited
Loading