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

Progress Bar progress text implementation in FluentTheme #7288

Merged
merged 7 commits into from
Jan 1, 2022

Conversation

Abdesol
Copy link
Contributor

@Abdesol Abdesol commented Dec 30, 2021

What does the pull request do?

Add a progress text to a progress bar in fluent theme.

What is the current behavior?

ProgressBar ShowProgressText doesn't show the progress text.

What is the updated/expected behavior with this PR?

If the progress par ShowProgressText is set to true, it will show the progress text/status.

Fixed issues

Fixes #7255

@Abdesol
Copy link
Contributor Author

Abdesol commented Dec 30, 2021

This images show the previous behaviour before fix and after the fix.
image

@robloo
Copy link
Contributor

robloo commented Dec 30, 2021

What happens to the percentage text color when progress is >=50%. Seems like it wouldn't be visible.

  1. I think lightweight resource and potentially another brush property are needed to control this.
  2. A new property should be added to disable this without re-templating.

@maxkatz6
Copy link
Member

maxkatz6 commented Dec 30, 2021

A new property should be added to disable this without re-templating.

Why? ShowProgressText is already there and disabled by default.

@Abdesol default progress bar is somewhat 4px height, right? Do you need to adjust height as well if you set ShowProgressText to true or is it automatically done by current layout?

@@ -21,6 +21,7 @@
<Setter Property="Template">
<ControlTemplate>
<Border x:Name="ProgressBarRoot" ClipToBounds="True" Background="{TemplateBinding Background}" BorderBrush="{TemplateBinding BorderBrush}" BorderThickness="{TemplateBinding BorderThickness}" CornerRadius="{TemplateBinding CornerRadius}">
<Grid>
Copy link
Member

Choose a reason for hiding this comment

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

If there is no row or columns, better to go with Panel instead. Less overhead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What happens to the percentage text color when progress is >=50%. Seems like it wouldn't be visible.

  1. I think lightweight resource and potentially another brush property are needed to control this.
  2. A new property should be added to disable this without re-templating.

The Default theme has a similar issue. I will try to make a new pr to fix both of them in the near future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If there is no row or columns, better to go with Panel instead. Less overhead.

Fixed that on the current commit.

@robloo
Copy link
Contributor

robloo commented Dec 30, 2021

Why? ShowProgressText is already there and disabled by default.

Did not know that, sorry

@avaloniaui-team
Copy link
Contributor

You can test this PR using the following package version. 0.10.999-cibuild0017616-beta. (feed url: https://nuget.avaloniaui.net/repository/avalonia-all/index.json)

@avaloniaui-team
Copy link
Contributor

You can test this PR using the following package version. 0.10.999-cibuild0017620-beta. (feed url: https://nuget.avaloniaui.net/repository/avalonia-all/index.json)

@avaloniaui-team
Copy link
Contributor

You can test this PR using the following package version. 0.10.999-cibuild0017621-beta. (feed url: https://nuget.avaloniaui.net/repository/avalonia-all/index.json)

Copy link

@tsukuyomm tsukuyomm left a comment

Choose a reason for hiding this comment

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

looking good

@maxkatz6
Copy link
Member

maxkatz6 commented Jan 1, 2022

@Abdesol in default theme text is written with default text foreground (white/black depending on dark mode). Probably it makes sense to do the same here?
Resource for text color is SystemControlForegroundBaseHighBrush.

@robloo
Copy link
Contributor

robloo commented Jan 1, 2022

@Abdesol in default theme text is written with default text foreground (white/black depending on dark mode). Probably it makes sense to do the same here?
Resource for text color is SystemControlForegroundBaseHighBrush

To fix the issue consistently, text color should be the same as with the ToggleButton which faces the same issue. I would just double check but the SystemControlForegroundBaseHighBrush should be the correct resource.

@Abdesol
Copy link
Contributor Author

Abdesol commented Jan 1, 2022

@Abdesol in default theme text is written with default text foreground (white/black depending on dark mode). Probably it makes sense to do the same here? Resource for text color is SystemControlForegroundBaseHighBrush.

oh yeah. Changed that on the current commit.
Sorry for a lot of commits : (
I am not used to the resources

@avaloniaui-team
Copy link
Contributor

You can test this PR using the following package version. 0.10.999-cibuild0017634-beta. (feed url: https://nuget.avaloniaui.net/repository/avalonia-all/index.json)

@maxkatz6 maxkatz6 merged commit 9f075ae into AvaloniaUI:master Jan 1, 2022
danwalmsley pushed a commit that referenced this pull request Jan 24, 2022
Progress Bar progress text implementation in FluentTheme
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ProgressBar ShowProgressText not implemented in FluentTheme
6 participants