feat(TextInputGroup): added disabled styling #4484
Conversation
|
Preview: https://patternfly-pr-4484.surge.sh |
wise-king-sullyman
force-pushed
the
text-input-group-disabled-styling
branch
from
0dbb882
to
2047c2d
Compare
src/patternfly/components/TextInputGroup/examples/TextInputGroup.md
Outdated
Show resolved
Hide resolved
Co-authored-by: Michael Coker <35148959+mcoker@users.noreply.github.com>
- gave Color a default value - set pointer-events to none - disabled borders - added aria-label - added tabindex of -1 - added text-input-group--IsDisabled to template to enable quick changes of components from enabled to disabled
| {{#if text-input-group--attribute}} | ||
| {{{text-input-group--attribute}}} | ||
| {{/if}} | ||
| {{#if text-input-group--IsDisabled}} | ||
| aria-label="Disabled text input group example" |
There was a problem hiding this comment.
@jessiehuff can you confirm whether we need an aria-label on the outer container or just on the text input inside? I'm assuming the text input since this is designed to mimic the existing TextInput component.
There was a problem hiding this comment.
I'll go ahead and move this label to just the input itself.
There was a problem hiding this comment.
I agree, the aria-label makes the most sense on the text input like the TextInput component so that when the user gets to the input, it hears the label. That's where the screen reader would expect to interact with it. :)
src/patternfly/components/TextInputGroup/text-input-group-text-input.hbs
Outdated
Show resolved
Hide resolved
src/patternfly/components/TextInputGroup/examples/TextInputGroup.md
Outdated
Show resolved
Hide resolved
- added handlebars whitespace removal tilde - removed tabindex - moved aria-label to only the text input itself - removed pf-m-disabled border color setting - added automatic value input when hbs template IsDisabled - added contect to example id
wise-king-sullyman
requested review from
mcoker,
mattnolting,
srambach and
mcarrano
There was a problem hiding this comment.
@wise-king-sullyman the styling looks fine. But in comparing with https://www.patternfly.org/v4/components/text-input#disabled I notice that the standard disabled text field also has a cursor change, but the text input group does not.
|
@mcarrano I had originally used I can change back to using Button and Dropdown are a couple of examples where the cursor does not change when disabled. Ultimately this comes down to a decision between making things simpler from a developer perspective, or nicer from a user perspective. I'm happy to go in either direction you prefer but thought you may want this additional context. |
There was a problem hiding this comment.
@wise-king-sullyman Thanks for the explanation about why we should not change the cursor when the text input group is disabled. I agree to keep it as is.
src/patternfly/components/TextInputGroup/text-input-group-text-input.hbs
Outdated
Show resolved
Hide resolved
- reduced text input template IsDisabled conditional tests from two down to one - moved disabled border vars to top of block
src/patternfly/components/TextInputGroup/text-input-group-text-input.hbs
Outdated
Show resolved
Hide resolved
…erate lines Co-authored-by: Michael Coker <35148959+mcoker@users.noreply.github.com>
mcoker
changed the title
|
🎉 This PR is included in version 4.153.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Closes #4482