InputControl: Decrease large default padding if has prefix/suffix #42166
Conversation
| prefix: <span style={ { marginInlineStart: 8 } }>@</span>, | ||
| }; | ||
|
|
||
| export const WithSuffix = Template.bind( {} ); | ||
| WithSuffix.args = { | ||
| ...Default.args, | ||
| suffix: <button style={ { marginRight: 4 } }>Send</button>, | ||
| suffix: <button style={ { marginInlineEnd: 4 } }>Send</button>, |
There was a problem hiding this comment.
Tweaked examples for better RTL.
| @@ -18,22 +18,13 @@ type SelectProps = { | |||
|
|
|||
| type InputProps = { | |||
| disableUnits?: boolean; | |||
| size: SelectSize; | |||
There was a problem hiding this comment.
Unrelated, but minor cleanup of an unused type.
| const paddingStyles = ( { disableUnits }: InputProps ) => { | ||
| if ( disableUnits ) return ''; | ||
|
|
||
| return css` | ||
| ${ rtl( { paddingRight: 8 } )() }; | ||
| `; | ||
| }; | ||
|
|
There was a problem hiding this comment.
CSS override removed 🎉
Btw @aaronrobertshaw: When building BorderControl, do you remember if there were any blockers to rendering the BorderControlDropdown as a prefix on the UnitControl? I'm wondering if we can remove some CSS overrides in there too if this PR lands.
There was a problem hiding this comment.
My memory is failing me on this one, I don't recall any blockers. It might have been as simple as I didn't see the prefix prop in the UnitControl readme and didn't follow the props through to the underlying InputControl.
It also looks like we might need to tweak the UnitControl types, as attempting to pass the border control dropdown via a prefix prop on the UnitControl results in a type error that it expects prefix to be string | undefined.
I'd be happy to work on rendering the border control's dropdown as a prefix in a follow up to this PR.
There was a problem hiding this comment.
That sounds great, thanks! It's nice to see things getting less hacky as we converge on common patterns 🙂
There was a problem hiding this comment.
I've put together a PR (#42212) that now renders the dropdown via the UnitControl prefix and attempts to clean up the styles. There could still be more to polish on that control when it comes time to add a large 40px size variant.
packages/components/src/input-control/styles/input-control-styles.tsx
Outdated
Show resolved
Hide resolved
|
Seems to be working as expected, though ideally we should increase the margin/padding on the prefix/suffix as well, so that it matches the input:
|
Currently But I agree that it's kind of necessary that the prefix/suffix elements know what the standard padding is for a given size variant. As a follow-up task, I'll see if I can propose something where |
Solution proposed in #42378 👍 This PR can proceed independently of that PR though. |
ciampo
added
[Type] Enhancement
What?
Ensures that the padding between the
prefix/suffixand the text input doesn't get too big in the large size variant of the component.Why?
The large size variant of the component increases the side paddings of the text input to 16px (previously 8px). In most cases, this is too big when using with a
prefixorsuffixelement. Some actual examples of this can be seen in the large variants ofUnitControlandColorPicker:How?
Added an internal API to specify custom side padding on the
InputFieldsubcomponent. This API is internal-only for now, and is meant to minimally satisfy the internal customization needs in the components library. Though, I tried to make it generic enough so it would be easy to expose the API externally onInputControlitself, if we want to do so in the future.Testing Instructions
npm run storybook:devsizevariant. This should also work in RTL mode.Screenshots or screencast