Skip to content
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

Add option to customize progress bar rounding #2881

Merged
merged 4 commits into from
Jan 7, 2024

Conversation

YgorSouza
Copy link
Contributor

The minimum filled width is adapted to the radius and the animation is disabled so it renders correctly.

@YgorSouza
Copy link
Contributor Author

To test, add e.g., .rounding(2.0) here:

let progress_bar = egui::ProgressBar::new(progress)
.show_percentage()
.animate(*animate_progress_bar);

Should I add this somewhere in the demo?

@YgorSouza YgorSouza marked this pull request as draft July 23, 2023 15:41
CHANGELOG.md Outdated Show resolved Hide resolved
@YgorSouza
Copy link
Contributor Author

YgorSouza commented Aug 12, 2023

I'm thinking that instead of setting an explicit rounding value, it could instead be a boolean that makes it use the same rounding as the other widgets, like buttons and textboxes, so it is easier to keep the UI consistent. Does that make sense? I also have to fix some visual bugs at low fill percentages before this is ready to merge.

Edit: actually, the way it is currently might be better, as you can just set the rounding to one of the ui settings if you want, e.g., ProgressBar::new(progress).rounding(ui.visuals().noninteractive().rounding)

@YgorSouza YgorSouza marked this pull request as ready for review August 12, 2023 16:26
Copy link
Owner

@emilk emilk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice with a image or gif for this!

@YgorSouza YgorSouza marked this pull request as draft November 10, 2023 22:17
@YgorSouza
Copy link
Contributor Author

It would be nice with a image or gif for this!

Screencast.from.2023-11-10.23-25-49.webm

@YgorSouza YgorSouza marked this pull request as ready for review November 10, 2023 22:37
outer_rect.height(),
),
);
let min_width = 2.0 * rounding.sw.at_least(rounding.nw).at_most(corner_radius);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why only look at sw and nw here? Wouldn't it make sense to use something like rounding.max() instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this min_width is meant to make the rounding look right when the progress bar is empty, I figured only the left side mattered. But maybe it would be better to just use max(). I'll do a quick test and commit it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So there is no method Rounding::max currently. Should I add one?

It doesn't seem to be necessary, though. The problem this min_width avoids is that the filled rect is flattened when it is too small, and then it ends up appearing outside the background rect, like this:

image

Actually, if there was a way to avoid this overflow without setting a minimum width, I think a lot of people would prefer that (see for example #3999). But I don't know if it's possible.

crates/egui/src/widgets/progress_bar.rs Show resolved Hide resolved
crates/egui/src/widgets/progress_bar.rs Show resolved Hide resolved
YgorSouza and others added 4 commits November 16, 2023 22:32
The minimum filled width is adapted to the radius and the animation is
disabled so it renders correctly.
To match what it was before and make the animation look right.
Co-authored-by: Emil Ernerfeldt <[email protected]>
@emilk emilk added the egui label Jan 7, 2024
@emilk emilk merged commit 4a6999b into emilk:master Jan 7, 2024
19 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants