fix(InputGroupText): drop variants and fix missing padding because of wrong classes #8953
Conversation
|
Preview: https://patternfly-react-pr-8953.surge.sh A11y report: https://patternfly-react-pr-8953-a11y.surge.sh |
… wrong classes Follow the html docs on how input-group text component should look like. https://pf5.patternfly.org/components/input-group/#overview InputGroupText is meant to contain plain text. Any kind of variant setting is overkill, we should just provide users with a straightforward way to put a plain text inside an input group and have all the needed padding sets. Fixes patternfly#8952
KKoukiou
force-pushed
the
input-group-text
branch
from
3421e69
to
1a4c055
Compare
|
Screenshot of an application of this component on the slider documentation: Before: After: |
| export enum InputGroupTextVariant { | ||
| default = 'default', | ||
| plain = 'plain' | ||
| } |
There was a problem hiding this comment.
I'm not sure if we would want to remove this enum and the variant prop. Part of what the plain variant should be doing is removing that bottom border from pf-c-input-group__item. My comment below mentions it as well, but it looks like the cause of the padding issue might be the markup not matching Core. @srambach wdyt (including my other comment below)?
There was a problem hiding this comment.
I think we want to keep the option to make it plain or not, and I agree that the problem seems to be that the markup is slightly different as @thatblindgeye comments below.
| styles.inputGroupItem, | ||
| styles.inputGroupText, | ||
| variant === InputGroupTextVariant.plain && styles.modifiers.plain, | ||
| styles.modifiers.plain, | ||
| styles.modifiers.box, |
There was a problem hiding this comment.
This is causing differences in the markup compared to Core. In Core the markup is:
This PR the markup becomes:
|
So @thatblindgeye @srambach do we want to have the exact same structure in html like in core? I find this wrapping with |
|
@KKoukiou Good question. In this case it would be simpler, but I would say that keeping them separate means that if there are multiple |
|
@KKoukiou typically we match the structure core has. If we need to update the structure core should do so as well. |
|
HI Katerina. Maria contributed this PR #9074. |
Follow the html docs on how input-group text component should look like. https://pf5.patternfly.org/components/input-group/#overview
InputGroupText is meant to contain plain text. Any kind of variant setting is overkill, we should just provide users with a straightforward way to put a plain text inside an input group and have all the needed padding sets.
Fixes #8952