-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WIP] UnitRangeControl: Add combined unit and range control component #40462
Conversation
Size Change: +341 B (0%) Total Size: 1.23 MB
ℹ️ View Unchanged
|
I haven't looked at the code in details yet, but an option that came to mind is to edit the existing 1. Allow for the slider and the input to invert places Currently, the slider always sits on the left side, and the input on the right side: We could look into adding a new prop that toggles between these 2 variants (exact APIs TBD) 2. Allow for We could add a new prop, something like What do folks think? |
The thing is, if we were to build a composite "range slider + number input" component today from scratch, I think the API would look like the one @aaronrobertshaw is proposing here. I like how the props for each of the subcomponents are clearly encapsulated and passed through. The way the composite component takes on the minimal amount of "meta" responsiblity is very clean. I think this makes it easy to grok and maintain. If anything, I'd even prefer |
After a quick chat with @mirka , inspired by her previous comment, we discussed a potential approach to introducing this new component
Thoughts? We also agreed that, instead of offering the possibility to swap the position of slider and input (left and right), we should probably agree on a "standard design" for the component and use it across the editor. |
Thanks @ciampo and @mirka for the early feedback on this, it's appreciated 🙇
👍 The plan sounds good to me, I'll work through the steps outlined above. I've already made a start on converting the |
Thank you @aaronrobertshaw ! I opened #40507 to track this properly and to add a bit more detail to the plan outlined above. I guess this PR can be closed for now, and serve as an inspiration for later? |
Thanks for creating the tracking issue. I'll close this now. |
What?
Adds a new component combining and linking both a
UnitControl
andRangeControl
.Why?
We have a number of controls in the editor sidebar that should also have a slider beside them. A standardized component combining these out of the box will help improve consistency and make updating sidebar controls easier.
How?
Creates a new component that:
UnitControl
andRangeControl
within aHStack
${ value }${ unit }
toonChange
when slider changesTodo
UnitControl
orRangeControl
README.md
Testing Instructions
npm run storybook:dev
UnitControl
orRangeControl
props via the story's controlsScreenshots or screencast
Screen.Recording.2022-04-20.at.5.57.01.pm.mp4