feat(Text area): Add resize disable feature #11383
Conversation
tlabaj
requested review from
a team,
mfrances17 and
thatblindgeye
and removed request for
a team
|
Preview: https://patternfly-react-pr-11383.surge.sh A11y report: https://patternfly-react-pr-11383-a11y.surge.sh |
There was a problem hiding this comment.
This looks good, just non-blocking comment below. Could we also add a test that checks that no additional class is added for the new orientation value? Looks like we're also missing a test that checks for the pf-m-resize-both modifier class when orientation is both, but that can be a followup if need be.
| @@ -8,7 +8,8 @@ import { FormControlIcon } from '../FormControl/FormControlIcon'; | |||
| export enum TextAreResizeOrientation { | |||
| horizontal = 'horizontal', | |||
| vertical = 'vertical', | |||
| both = 'both' | |||
| both = 'both', | |||
| disabled = 'disabled' | |||
There was a problem hiding this comment.
Can't recall if there was a conversation regarding this, but was using "none" considered instead of "disabled", to match the CSS value that would normally be used? Not a blocker since "disabled" is still accurate, just wondering if people may think a modifier class will be applied like in other instances odf a disabled modifier.
There was a problem hiding this comment.
I actually debated between the two names. I started with none. I can change it back.
There was a problem hiding this comment.
👍 changes look good and tested fine, nice job.
i'm on the fence on whether we should add a specific example for it, since we do call out horizontal and vertical resize orientations explicitly. that makes it seem like we should have examples for all available options, especially none which i personally think will be a very popular use case (one could make a strong case that unresizable should really be the default). up to you though, not enough for me to not approve.
|
@tlabaj thanks for adding the example, looks good... merging |
What: Closes #11130