Mamatt/progress ring determinate #93
Conversation
|
I think I resolved all of the comments. Is this ready to publish to master? |
|
|
||
| Progress _Bar_ has both a determinate and indeterminate mode. | ||
| In determine mode the rectangle goes from empty to full, in indeterminate mode it shows an animation. | ||
| Progress _Ring_ currently only has an indeterminate mode, but this spec is adding an indeterminate mode. |
There was a problem hiding this comment.
| Progress _Ring_ currently only has an indeterminate mode, but this spec is adding an indeterminate mode. | |
| Progress _Ring_ currently only has an indeterminate mode, but this spec is adding a determinate mode. |
| In determine mode the rectangle goes from empty to full, in indeterminate mode it shows an animation. | ||
| Progress _Ring_ currently only has an indeterminate mode, but this spec is adding an indeterminate mode. | ||
|
|
||
| ProgressBar is also getting a feature to replaces its built-in visualization (a spinning ring) |
There was a problem hiding this comment.
| ProgressBar is also getting a feature to replaces its built-in visualization (a spinning ring) | |
| ProgressRing is also getting a feature to replaces its built-in visualization (a spinning ring) |
| The typical visual appearance is a ring-shaped "spinner" that animates a filled area as progress continues. | ||
|
|
||
| By default, ProgressRing will display an indeterminate spinner that continuously | ||
| animates until the control is no longer visible ?? or |
There was a problem hiding this comment.
Unfinished wordsmithing. Suggest
... that continuously animates as long as the IsActive property is
true.
No need to say that it animates only when visible. Collapsed items cannot animate.
There was a problem hiding this comment.
Recommend: it will animate until it's IsActive is false or Visibility is collapsed.
| animates until the control is no longer visible ?? or | ||
| [ProgressRing.IsActive](https://docs.microsoft.com/uwp/api/Microsoft.UI.Xaml.Controls.ProgressRing.IsActive) | ||
| is set to false. You can change the behavior to display | ||
| a determinate ring that fills in based on the value you provide, using the `IsIndeterminate` property. |
There was a problem hiding this comment.
| a determinate ring that fills in based on the value you provide, using the `IsIndeterminate` property. | |
| a determinate ring that fills in based on the value you provide, by setting the `IsIndeterminate` property to `false` (default `true`). |
|
|
||
| By default ProgressRing is indeterminate (a spinning circle). | ||
|
|
||
| > Issue: not the same without an AniGif |
There was a problem hiding this comment.
Internal editing comment needs to be addressed.
There was a problem hiding this comment.
Recommend: add a picture
| Boolean IsIndeterminate{ get; set; }; | ||
|
|
||
| IAnimatedVisualSource DeterminateSource{ get; set; }; | ||
| IAnimatedVisualSource IndeterminateSource{ get; set; }; |
There was a problem hiding this comment.
Style: Sometimes read/write properties are expressed as Blah { get; set;} and sometimes as just Blah. Please be consistent with the rest of WinUI on this stylistic choice.
|
|
||
| ## Design Behavior Details | ||
| The determinate progress ring will be an empty ring when the progress is set to a value of 0. | ||
| When value is set to 1, a blue dot will appear and then continue to fill as the value increases. |
There was a problem hiding this comment.
The value is a Double, so is this really saying "any nonzero value", like ½?
There was a problem hiding this comment.
Shouldn't this be when the Value is set to Minimum?
There was a problem hiding this comment.
Discussion: Point is that should only be empty at 0 and full at 100.
Recommend: update
|
|
||
| ```xaml | ||
| [webhosthidden] | ||
| unsealed runtimeclass ProgressRing : Windows.UI.Xaml.Controls.Control |
There was a problem hiding this comment.
How horrible would it be if we just made ProgressRing derive from RangeBase?
There was a problem hiding this comment.
It would bring in LargeChange and SmallChange APIs, which make no sense on ProgressRing. And there's no polymorphism scenario to motivate a common case class.
| See [AnimatedVisualPlayer](https://docs.microsoft.com/uwp/api/Microsoft.UI.Xaml.Controls.AnimatedVisualPlayer) | ||
| for more information on animated visuals. | ||
|
|
||
| > Picture would be nice |
There was a problem hiding this comment.
Please clean up internal comments.
There was a problem hiding this comment.
Recommend: add a picture
|
|
||
| Show progress ring that visually looks like a thermometer boiling over. | ||
| See [AnimatedVisualPlayer](https://docs.microsoft.com/uwp/api/Microsoft.UI.Xaml.Controls.AnimatedVisualPlayer) | ||
| for more information on animated visuals. |
There was a problem hiding this comment.
Does this mean that everybody who uses a ProgressRing, even if they just use the standard animation, still has to pull in all the code for a Lottie player?
There was a problem hiding this comment.
Discussion: Yes, the internal implementation of the ring uses a Lottie animation.
There was a problem hiding this comment.
Discussion: Yes, the internal implementation of the ring uses a Lottie animation.
Wasn't there a way to use standard Xaml animations via re-templating?
Hello! I updated this spec so we can add the determinate state to progress ring.