-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Progress bar #519
Progress bar #519
Conversation
The animation looks awesome! I think I like it a little better with the selection background and text colors. What do you think? The diff is: diff --git a/egui_demo_lib/src/apps/demo/progress_bar.rs b/egui_demo_lib/src/apps/demo/progress_bar.rs
index f0cb976..f37120f 100644
--- a/egui_demo_lib/src/apps/demo/progress_bar.rs
+++ b/egui_demo_lib/src/apps/demo/progress_bar.rs
@@ -72,11 +72,7 @@ impl Widget for ProgressBar {
),
);
- let (dark, bright) = if visuals.dark_mode {
- (0.2, 0.3)
- } else {
- (0.8, 0.9)
- };
+ let (dark, bright) = (0.8, 1.0);
let color_factor = if animate {
ui.ctx().request_repaint();
lerp(dark..=bright, ui.input().time.cos().abs())
@@ -87,7 +83,7 @@ impl Widget for ProgressBar {
ui.painter().rect(
inner_rect,
corner_radius,
- Color32::from_gray((color_factor * 255.0) as u8),
+ Color32::from(Rgba::from(visuals.selection.bg_fill) * color_factor as f32),
Stroke::none(),
);
@@ -119,7 +115,9 @@ impl Widget for ProgressBar {
Align2::LEFT_CENTER,
text,
TextStyle::Button,
- visuals.text_color(),
+ visuals
+ .override_text_color
+ .unwrap_or(visuals.selection.stroke.color),
);
}
Also, shouldn't the widget be included in |
@parasyte Much better! I applied the suggestions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks really good!
} | ||
|
||
/// Whether to display a loading animation. Note that this require the UI to be redrawn. | ||
/// Defaults to `false`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is an interesting question what the default should be here.
It wastes some CPU in reactive mode using eframe
, but basically anywhere else it is nice with an animation.
In the future we may want to have a global egui option for "do you want animations or not?", but for now... either false
or true
work for me to be honest.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The animation now plays on hover, I'll leave it up to you to decide whether it should be animated by default.
visuals | ||
.override_text_color | ||
.unwrap_or(visuals.selection.stroke.color), | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can easily overflow the size of the progress bar, which would look pretty ugly.
I think we either should either:
- measure the text (https://docs.rs/egui/0.13.0/egui/struct.Painter.html#method.layout_no_wrap) and use it as a minimum width of the widget
- clip the text (https://docs.rs/egui/0.13.0/egui/struct.Painter.html#method.sub_region)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, the text now gets clipped in 1d0d86f.
ui.add(doc_link_label("ProgressBar", "ProgressBar")); | ||
let progress = *scalar / 360.0; | ||
let progress_bar = egui::ProgressBar::new(progress) | ||
.text(format!("{}%", (progress * 100.0) as usize)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like something so common that we should have a helper for it, e.g. .show_percentage()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added in 1d0d86f
Co-authored-by: Emil Ernerfeldt <[email protected]>
Co-authored-by: Emil Ernerfeldt <[email protected]>
Co-authored-by: Emil Ernerfeldt <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wonderful - thank you!
Closes #496.
simplescreenrecorder-2021-06-26_22.56.43.mp4