Spinner widget #1003
Spinner widget #1003
Conversation
| /// The argument can be either a `Color` or a [`Key<Color>`]. | ||
| /// | ||
| /// [`Key<Color>`]: ../struct.Key.html | ||
| pub fn set_color(&mut self, color: impl Into<KeyOrValue<Color>>) { |
There was a problem hiding this comment.
These two color methods should mention in their docs that the alpha channel is ignored.
There was a problem hiding this comment.
Ah that was an oversight. Fixed it to use the color's alpha.
druid/src/widget/spinner.rs
Outdated
|
|
||
| impl Default for Spinner { | ||
| fn default() -> Self { | ||
| Spinner::new() |
There was a problem hiding this comment.
Can't we use theme::LABEL_COLOR.into(), in the Default implementation?
There was a problem hiding this comment.
Not sure what you're proposing here.
There was a problem hiding this comment.
So the reason you got that clippy warning was because the new function doesn't take any arguments and could be equal to default. So what I'm thinking is that we could do the following:
impl Spinner {
/// Create a spinner widget
pub fn new() -> Spinner {
Spinner::default()
}
}
impl Default for Spinner {
fn default() -> Self {
Spinner {
t: 0.0,
color: theme::LABEL_COLOR.into(),
}
}
}It ends up being the same result for now, but if the new method gets arguments added to it then it'll be an easier change. This doesn't matter too much, but I'm just thinking I might somewhat prefer to have the default function be explicit.
Your call.
There was a problem hiding this comment.
Ah, that makes sense! Thanks for explaining.
xStrom
added
S-waiting-on-author
futurepaul
added
S-needs-review
This is pretty basic except for the spinning math, which I wish I'd commented while I was writing it!
My layout strategy for this is to only match its container size if that container has a constrained height and width. Otherwise it uses the default widget height key to size itself. Does that sound reasonable?