Skip to content
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

feat(TextareaField): add TextArea base component and TextareaField #1493

Merged
merged 1 commit into from
Feb 22, 2023

Conversation

booc0mtaco
Copy link
Contributor

@booc0mtaco booc0mtaco commented Feb 10, 2023

Summary:

  • create TextArea component
  • for now exporting, but we would want to treat this similar to how we do the input base component
  • will sync with latest design before merging

Test Plan:

  • add snapshots and base tests
  • verify no other snapshots change

TODO

  • create proper exportable component (implementing label, error message, counting when maxlength used, etc.)
  • brief discussion on how to (re-)organize such base components (atoms that map directly to HTML tags)

@codecov
Copy link

codecov bot commented Feb 10, 2023

Codecov Report

Merging #1493 (1fa0d5d) into next (5abd719) will increase coverage by 0.02%.
The diff coverage is 93.75%.

@@            Coverage Diff             @@
##             next    #1493      +/-   ##
==========================================
+ Coverage   91.87%   91.90%   +0.02%     
==========================================
  Files         272      277       +5     
  Lines        4113     4177      +64     
  Branches      732      755      +23     
==========================================
+ Hits         3779     3839      +60     
- Misses        309      313       +4     
  Partials       25       25              
Impacted Files Coverage Δ
src/components/TextareaField/TextareaField.tsx 91.42% <91.42%> (ø)
src/components/TextArea/TextArea.tsx 92.30% <92.30%> (ø)
src/components/TextArea/index.ts 100.00% <100.00%> (ø)
...components/TextareaField/TextareaField.stories.tsx 100.00% <100.00%> (ø)
src/components/TextareaField/index.ts 100.00% <100.00%> (ø)
src/index.ts 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@github-actions
Copy link

github-actions bot commented Feb 10, 2023

size-limit report 📦

Path Size
components 118.06 KB (+1.59% 🔺)
styles 3.1 KB (0%)

@booc0mtaco
Copy link
Contributor Author

booc0mtaco commented Feb 10, 2023

Notes to self:

  • structure should mimic RadioInput and CheckboxInput
  • fix up the a11y problems from test
  • review with Ryan once designs are finalized

@booc0mtaco booc0mtaco force-pushed the eds-817-create-textarea-atom branch 7 times, most recently from d4451f9 to 867c56a Compare February 17, 2023 23:05
@booc0mtaco booc0mtaco changed the title feat(TextArea): add TextArea base component feat(TextareaField): add TextArea base component and TextareaField Feb 21, 2023
@booc0mtaco booc0mtaco force-pushed the eds-817-create-textarea-atom branch 2 times, most recently from 6b39903 to bd4100f Compare February 21, 2023 23:18
@booc0mtaco booc0mtaco requested a review from a team February 21, 2023 23:23
@booc0mtaco
Copy link
Contributor Author

@jinlee93 we can talk thru the states when an input is required and empty. b/c we don't apply .error on empty/required, the UI shows no problem. if we were to add :invalid to the inputStyles mixin to match .error, any empty fields would be styled as an error when required is true in chromatic

I was thinking to try and use :invalid across our inputs, but will save that for another time. I added a TODO for this in the PR for consideration at some point

@booc0mtaco booc0mtaco force-pushed the eds-817-create-textarea-atom branch 2 times, most recently from c8f1563 to 4a0c7d2 Compare February 22, 2023 17:50
- create TextArea base component
- for now exporting, but we would want to treat this similar to how we do the input base component
- will sync with latest design before merging
- create TextareaField component (augmenting with label, a11y changes, etc.)
- specify examples for updating height using rows
- handle disabled on base component
args: {
rows: 10,
},
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about having an interactive story to reflect the ongoing character count, like:
image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not a bad idea. I was planning to create a separate ticket for the text count thing, as there are a few behaviors i want to discuss.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jinlee93 created ticket and tagged you on that one!

@@ -133,13 +133,15 @@
* Input error state
*/
&.error {
/* TODO: should this color be applied when a field is in an :invalid state */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make input have an invalid state manually, or is it interpreted from fields such as the disabled attribute?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:invalid is determined by the browser based on other attributes, and the value in the field, so it's not really an attribute unto itself. for <textarea>s :invalid applies if the value in the field is outside the bounds of what minlength and maxlength says is allowed (and in the case of maxlength only after some user action triggers the check, which is weird)

For <input>s, :invalid applies if you have a required field, and it becomes empty OR if the pattern is violated, along with the above.

When i added &:invalid to the selector list on line 135, it changed some TextField snapshots to an error state b/c the fields didn't have any default value. this means that blank forms might start out with red outlines if marked as required, which we don't want

@import '../../design-tokens/mixins.css';

/*------------------------------------*\
    # TEXTAREA
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: is textarea one word or two words loll 🤔 (doesn't actually need to change)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jinlee93 lol, treating it as one word since MDN does. which may mean the base component name should lower-case the A :)

Copy link
Contributor

@jinlee93 jinlee93 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@booc0mtaco booc0mtaco merged commit f2ba31d into next Feb 22, 2023
@booc0mtaco booc0mtaco deleted the eds-817-create-textarea-atom branch February 22, 2023 21:47
@booc0mtaco booc0mtaco mentioned this pull request Mar 2, 2023
booc0mtaco added a commit that referenced this pull request Mar 2, 2023
## [10.0.0](v9.1.0...v10.0.0) (2023-03-02)


### ⚠ BREAKING CHANGES

* remove data-bootstrap-override EDS-850

### Features

* **Avatar:** add avatar component ([#1510](#1510)) ([bc21f85](bc21f85))
* **slider:** create slider ([#1503](#1503)) ([e7ced34](e7ced34))
* **TextareaField:** add TextArea base component and TextareaField ([#1493](#1493)) ([f2ba31d](f2ba31d))


### Bug Fixes

* remove data-bootstrap-override EDS-850 ([3b5d59b](3b5d59b))
* remove old, outdated, unnecessary snapshot ([fb63a34](fb63a34))
* **Select:** sync label design with form fields ([#1506](#1506)) ([efe9330](efe9330))
* update deps ([9a80e7f](9a80e7f))
* upgrade axe-core from 4.4.3 to 4.6.3 ([af3f9c5](af3f9c5))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants