-
Notifications
You must be signed in to change notification settings - Fork 56
feat(component): add Text Area component #3215
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
base: ouds/main
Are you sure you want to change the base?
Conversation
❌ Deploy Preview for boosted failed. Why did it fail? →
|
louismaximepiton
left a comment
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.
General remarks:
- The text-area scroll looks a bit broken on FF when it's little, I don't know if there is anything better to do here.
- Wondering if the flash when we resize is questionable from a11y pov. I think we can avoid this anyway
- We can probably do it later, but we might want a live example of the behavior of error text + helper text
- We can probably do it later, but we might flatten the CSS to have less complexity in selectors
- We can probably do it later, but I don't like having several places in one component that handles the same CSS var, I find hard to debug, but it's maybe only me on this.
TBH, I don't wanna go too deep in the review because it needs to land quite fast and it works well, but here are the implementation upgrades I could see so far.
scss/forms/_text-input.scss
Outdated
| box-shadow: none; | ||
|
|
||
| @supports (field-sizing: content) { | ||
| resize: none; |
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.
After reflection, maybe it's not a good pratice to remove functionality to people-, maybe they should have th eright to resize on every browser
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.
Let's discuss it
scss/forms/_text-input.scss
Outdated
| } | ||
|
|
||
| // Invalid text inputs | ||
| &.was-validated:has(.text-area-field:invalid), |
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.
The .was-validated is not expected on the .text-area or any of its children but on the <form> element itself. I think it doesn't work in this case.
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.
To be discussed if we keep .was-validated
louismaximepiton
left a comment
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.
Few questions to answer still, but it's good for me
MaxLardenois
left a comment
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.
Just a question I add while copying stuff for control items ;)
scss/forms/_text-input.scss
Outdated
| } | ||
|
|
||
| // Hover state styles | ||
| &:hover:has(.text-area-field:not(:focus):not(:disabled):not(:read-only):not(.is-invalid)) { |
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.
You can add a list in the :not() instead of repeating them (I know it wont make the selector small)
nilloq
left a comment
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.
Updates following @vprothais review
|
Ok on technical side, can be merged when ok a11y and design |
Types of change
Related issues
Closes #2919
Description
Implment new Text Area component
Checklists
Checklist (for Core Team only)
Progression (for Core Team only)
ouds/mainfollowing conventional commitLive previews