ColorPicker: Cleanly implement large size #42002
Conversation
ciampo
added
[Type] Enhancement
ciampo
force-pushed
the
fix/color-picker-large
branch
from
43c6a1e to
98284d2
Compare
There was a problem hiding this comment.
It's great to see the results of previous work, code definitely looks cleaner!
The only side-effect of this change is the horizontal padding in the input fields — previously it was 8px, while with the changes from this PR, it becomes 16px (see source code)
| trunk | this PR |
|---|---|
 |
 |
I'm going to approve this PR, as I think still think this is fine for the sake of applying a more uniform design across our components (cc @jameskoster in case you have any feedback).
|
I say this with no understanding of the impact on the underlying component(s), or how best to proceed, but that spacing does feel a bit large to me :) An 8px gap between prefix and input value seems more natural. The 16px gap on the right is good. It would be nice to match the prefix spacing with that, currently it is 14px for some reason 🤔 |
|
Great catch @ciampo, and thanks for the check @jameskoster. I proposed #42166 for better default padding in the prefix/suffix case. If that looks good, let's wait for that to land before merging this PR.
Will do 👍 |
mirka
force-pushed
the
fix/color-picker-large
branch
from
98284d2 to
4160194
Compare
|
Spacings looking good after #42166 ✨
|
What?
Replaces the hacky implementation of the 40px size.
Why?
We now have large size variants on the underlying components that can be used cleanly via props.
Testing Instructions
npm run storybook:dev