Add CupertinoLinearActivityIndicator
#170108
Conversation
github-actions
bot
added
framework
ValentinVignal
force-pushed
the
flutter/add-cupertino-linear-activity-indicator
branch
from
a8baca1 to
15ee6a9
Compare
| @override | ||
| void paint(Canvas canvas, Size size) { | ||
| // Draw the background of the progress bar. | ||
| canvas.drawRRect( |
There was a problem hiding this comment.
I'm wondering if the native version is a rounded rect or a rounded superellipse (squircle). Can you attach images to the PR description comparing the native version?
There was a problem hiding this comment.
I've tried to look at https://developer.apple.com/design/human-interface-guidelines/progress-indicators
and from the photo
it looks like rounded rect? But again, I half guessing here
There was a problem hiding this comment.
The native photo attached has less-rounded corners on the edges than the flutter photo attached. But the actual blue line has the same rounding (I think) in both.
There was a problem hiding this comment.
It's true on the picture it looks like the container has less rounded corners.
However, if I look at https://developer.apple.com/design/human-interface-guidelines/progress-indicators
it looks like the background AND the blue line are having a stadium shape, which is like the current implementation
Having said that, what would you prefer me to do?
I can replace the current native image attached to the PR description with the ones from https://developer.apple.com/design/human-interface-guidelines/progress-indicators
There was a problem hiding this comment.
Can you attach images from a running native iOS app? The goal is to match live widgets, not HIG website photos.
There was a problem hiding this comment.
Fair enough, I create a straightforward macOS app:
It looks like a stadium shape to me. What do you think?
|
@victorsanni I have attached the picture from the issue in the description. I might need a bit of assistance to provide what you need from me, as I'm not very familiar with Cupertino and Apple specs/environment. For context, I saw the issue #156167 and noticed that the PR to fix it #156287 was closed due to a lack of tests. So, I've added those tests. I'm not saying I'm unwilling to apply your suggestion; I just need you to be extra explicit because I might not fully understand everything. |
ValentinVignal
force-pushed
the
flutter/add-cupertino-linear-activity-indicator
branch
from
15ee6a9 to
022699e
Compare
| /// {@end-tool} | ||
| /// | ||
| /// See also: | ||
| /// |
There was a problem hiding this comment.
Add the circular progress indicator to the 'See also" section, just before the HIG link.
There was a problem hiding this comment.
And also add this to the 'See also' section of the circular progress indicator.
| required this.progress, | ||
| this.height = 4.5, | ||
| this.color, | ||
| super.key, |
There was a problem hiding this comment.
Should be the first argument in the constructor.
| /// The color of the progress bar. | ||
| /// | ||
| /// Defaults to [CupertinoColors.activeBlue] if no color is specified. This | ||
| /// color represents the portion of the bar that indicates progress. |
There was a problem hiding this comment.
This sentence should be in a paragraph before the default line.
| /// The current progress of the linear activity indicator. | ||
| /// | ||
| /// This value must be between zero and one. A value of 0.0 means no progress | ||
| /// and 1.0 means that progress is complete. The value will be clamped to be |
There was a problem hiding this comment.
Is it clamped if it asserts? Don't think this line is necessary.
|
|
||
| /// The current progress of the linear activity indicator. | ||
| /// | ||
| /// This value must be between zero and one. A value of 0.0 means no progress |
There was a problem hiding this comment.
| /// This value must be between zero and one. A value of 0.0 means no progress | |
| /// This value must be between 0.0 and 1.0. A value of 0.0 means no progress |
Be explicit.
|
|
||
| /// The height of the line used to draw the linear activity indicator. | ||
| /// | ||
| /// The default height is 4.5 units. Must be positive. |
There was a problem hiding this comment.
| /// The default height is 4.5 units. Must be positive. | |
| /// Defaults to 4.5 units. Must be positive. |
Removes redundancy.
|
Thank you for the feedbacks. I improved the documentation in Improve documentation |
Fixes #156167
Continuation of #156287
Native version:
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.