fix(form-control): addition of vars for MUI theming#7291
fix(form-control): addition of vars for MUI theming#7291mcoker merged 4 commits intopatternfly:mainfrom
Conversation
|
Preview: https://patternfly-pr-7291.surge.sh A11y report: https://patternfly-pr-7291-a11y.surge.sh |
There was a problem hiding this comment.
Left a few comments to review.
This is what it looks like currently if I customize the padding vars - note that the right padding doesn't apply to the right of the utilities element (select toggle, status icons, manually passed icons) and instead applies to the inner form element (the actual input, select, textarea)
This means you would set __[element]--PaddingInlineEnd to theme the space between the input and utilities/icons, and set __utilities--PaddingInlineEnd to theme the right padding of the component, though you would have to set __utilities--PaddingInlineEnd globally, since there isn't a var per form control type.
I would probably expect these vars to change the right padding of the utilities element, too - meaning they change the padding inside of the border for the component. Like this:
Then if you wanted to set the gap between the form element and the utilities/icons, you'd have to set --pf-v6-c-form-control--ColumnGap, though same issue as above where we don't have a var per form control type so it's a global change.
Though that requirement wasn't clear in the original request, so I'm curious what others think. wdyt @sg00dwin @srambach @jenny-s51?
|
|
||
| > select { | ||
| --#{$form-control}--PaddingInlineEnd: var(--#{$form-control}__select--PaddingInlineEnd); | ||
| @at-root .#{$form-control}:has(select) { |
There was a problem hiding this comment.
can this be rewritten as the suggestion below to be more consistent with how the other selectors are written?
| @at-root .#{$form-control}:has(select) { | |
| &:has(select) { |
| > :is(input, textarea) { | ||
| padding-inline-end: var(--#{$form-control}--m-success--PaddingInlineEnd); | ||
| } |
There was a problem hiding this comment.
I think you can't use --#{$form-control}--PaddingInlineEnd: var(--#{$form-control}--m-success--PaddingInlineEnd); here because --#{$form-control}--m-success--PaddingInlineEnd references --#{$form-control}--PaddingInlineEnd. You can probably fix that if you update it to reference --#{$form-control}--PaddingInlineEnd--base instead in this var
Same goes for other properties you may have set the actual property for instead of the CSS var.
| // select padding | ||
| --#{$form-control}__select--PaddingBlockStart: var(--#{$form-control}--PaddingBlockStart--base); | ||
| --#{$form-control}__select--PaddingBlockEnd: var(--#{$form-control}--PaddingBlockEnd--base); | ||
| --#{$form-control}__select--PaddingInlineEnd: var(--#{$form-control}--PaddingInlineEnd--base); |
There was a problem hiding this comment.
This is a pre-existing bug, but I wonder if we should try and fix it in this PR. The select's right padding doesn't take into account the icon width, so long items in the select will display under the icon
Off the top of my head, I might try these changes (all pseudo code, probably doesn't work, full of typos, etc)
// add font-size var for toggle-icon
--#{$form-control}__toggle-icon--FontSize: var(--pf-t--global--icon--size--body--default);
// change this existing declaration to account for toggle icon
--#{$form-control}__select--PaddingInlineEnd: calc(var(--#{$form-control}--PaddingInlineEnd--base) + var(--#{$form-control}__toggle-icon--FontSize));
// add font-size declaration to toggle-icon
.#{$form-control}__toggle-icon {
font-size: var(--#{$form-control}__toggle-icon--FontSize);
}
a8861f0 to
ebc2da5
Compare
|
@mcoker I have reworked the changes and I believe they address all the issues you noted. |
There was a problem hiding this comment.
Looks good with long text, focus ring, rtl 👍🏻
ebc2da5 to
c0767b5
Compare
|
@mcoker tested your latest changes and all looks good! |
|
🎉 This PR is included in version 6.2.0-prerelease.25 🎉 The release is available on: Your semantic-release bot 📦🚀 |
fixes #7222
New available variables
Realated
Textarea text realigned with status icon
Select status icon spacing with down caret corrected
BackstopJS Report.pdf