Closed
Conversation
mirka
commented
Jun 17, 2022
Comment on lines
-45
to
-52
| // All inputs should be the same height so this should be changed at the component level. | ||
| // That involves changing heights of multiple input types probably buttons too etc. | ||
| // So until that is done we are already using the new height on the color picker so it matches the mockups. | ||
| const inputHeightStyle = ` | ||
| &&& ${ Input } { | ||
| height: 40px; | ||
| }`; | ||
|
|
Member
Author
There was a problem hiding this comment.
I'll go ahead and remove these 40px styles without moving them to a large size variant, since it's just a hack anyway. We can first add a large size variant to InputControl once we're ready to introduce large variants.
15 tasks
Contributor
Sounds good — just ping once we have a new approach and this PR is ready for review again :) |
Member
Author
|
New plan: Instead of removing the 40px hack, I will be switching it to a clean implementation via size props on the underlying input controls. On hold until I add a large size variant on |
Member
Author
|
Closing in favor of #42002 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🚧 Currently on hold to reassess approach 🚧
Part of #39397
What?
Downsize the 40px inputs in
ColorPickerto the new default 36px.We can land these size changes immediately without coordination via
__nextprop, since they are already in an inconsistent state.Why?
As part of the effort to move toward consistent component sizes.
Testing Instructions
npm run storybook:devColorPicker. The RGB/HSL/HEX inputs should all be at 36px height.