Toggle Switch Widget #231
Toggle Switch Widget #231
Conversation
There was a problem hiding this comment.
Thank you for this! It looks good to me overall, just a few minor comments inline. I do think we could debate whether the label should be inside / outside / none at all (conveniently, switches seem to be totally different on every OS), but I think that debate can come later when we have a full palette of widgets to critique.
src/widget/switch.rs
Outdated
| ); | ||
|
|
||
| // paint different background for on and off state | ||
| let background_gradient = if *data { |
There was a problem hiding this comment.
I have an orange "caution" color that none of the widgets have used yet. Could potentially use it here for the off state, but this also looks fine as-is. Orange might be too intense for a typical switch use case? Here's the figma if you're interested.
There was a problem hiding this comment.
I think that as future work color should be configurable.
examples/switch.rs
Outdated
| value: bool, | ||
| } | ||
|
|
||
| fn build_widget() -> impl Widget<DemoState> { |
There was a problem hiding this comment.
this might be a bit too minimal of an example. potentially we could put switch inside another example, or expand this example to show off some other complementary aspects of druid. but I defer to @cmyr who has helped keep a handle on this directory.
There was a problem hiding this comment.
I find that when I'm using sample code, I benefit when examples are more complex, use API in interesting ways, or otherwise show me things I might be less likely to figure out on my own.
So this is okay, but it would also be totally okay to add this to an existing example, to change behaviour in some way.
There was a problem hiding this comment.
I couldn't think of how existing examples could be extended in a useful way right now. But maybe there will be some better examples in the future that could replace this one.
There was a problem hiding this comment.
Awesome @scholtzan this is graet.
a bunch of little nits and observations.
The one thing this is missing (to me) is an animation during the transition. But that can be future work.
examples/switch.rs
Outdated
| value: bool, | ||
| } | ||
|
|
||
| fn build_widget() -> impl Widget<DemoState> { |
There was a problem hiding this comment.
I find that when I'm using sample code, I benefit when examples are more complex, use API in interesting ways, or otherwise show me things I might be less likely to figure out on my own.
So this is okay, but it would also be totally okay to add this to an existing example, to change behaviour in some way.
src/widget/switch.rs
Outdated
| impl SwitchRaw { | ||
| fn knob_hit_test(&self, knob_width: f64, mouse_pos: Point) -> bool { | ||
| let knob_circle = Circle::new(self.knob_pos, knob_width / 2.); | ||
| if mouse_pos.distance(knob_circle.center) < knob_circle.radius { |
There was a problem hiding this comment.
the less obvious but maybe canonical way to do this might be if knob_circle.winding(mouse_pos) > 0 { .. but personally I think that the Shape trait should just have a contains(pt: Point) type method that calls winding under the hood; I think it would be a much simpler API to discover and to use, in many cases.
src/widget/switch.rs
Outdated
| ); | ||
|
|
||
| // paint different background for on and off state | ||
| let background_gradient = if *data { |
There was a problem hiding this comment.
I think that as future work color should be configurable.
scholtzan
force-pushed
the
switch
branch
2 times, most recently
from
0e2545f to
2a36f8b
Compare
|
Feel free to merge this @scholtzan! |
|
I don't have write access to the repo |
|
oops, you do now! |
I was playing around with widgets a bit after reading Building a widget for Druid and implemented a toggle switch:
Any comments are appreciated.