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(textarea): add horizontal and vertical resize variants #2331
Conversation
PatternFly-Next preview: https://patternfly-next-pr-2331.surge.sh |
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.
I added a few questions that might pertain to form-control > textarea
or all form-control
s. I noticed that the resizable textareas can break out of their container. We probably want to prevent that.
|
||
&.pf-m-resize-horizontal { | ||
resize: horizontal; | ||
} |
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.
Can you add max-width: 100%
and a min-width
to this?
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.
@mceledonia should we add a min-width
to the textarea when you can resize it horizontally, to prevent sizing it down too narrow?
|
||
&.pf-m-resize-vertical { | ||
resize: vertical; | ||
} |
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.
Maybe a min-height
here?
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.
@mceledonia what about a min-height
when you can resize it vertically to prevent sizing it down too short?
@mcarrano would you mind verifying the behavior from our discussion this morning? |
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.
LGTM
Opened #2357 to address min/max width/height.
@mattnolting re: #2357 we discussed that this morning and decided not to impose a min/max-height/width beyond the changes in this PR. @mceledonia @mcarrano do you want to weigh in? |
@mcoker This looks great. @mattnolting we decided to keep this simple and not impose a max/min width or height. There does not seem to be any harm in letting the user control this other than constraining the resizing as implemented by this PR. |
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.
NICE!
🎉 This PR is included in version 2.34.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
fixes #2251