-
Notifications
You must be signed in to change notification settings - Fork 152
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
Added borderColor #58
base: main
Are you sure you want to change the base?
Conversation
When borderColor is set, a border with the given color is added to the image exterior, making it better to see the image ending when the baseColor and the image background are similar
Very good!! |
what a good guy. @vinicios-cervantes |
Hello @chooyan-eng, please review this PR |
@luciano-cervantes As I have few time to maintain this package, I've now started to check the PR. As long as reading your PR description and capture, it's nice idea! Wait for my checking your code. |
Still waiting... |
Complicated... |
Very complicated @luciano-cervantes |
It has already completed a birthday @vinicios-cervantes . 🎉🎉 |
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.
@vinicios-cervantes
I really apologize for leaving this PR for more than a year!
As I checked and commented some points, check and fix (or leave your comments to) them. Thanks.
this.fixArea = false, | ||
this.progressIndicator = const SizedBox.shrink(), | ||
this.interactive = false, | ||
}) : assert((initialSize ?? 1.0) <= 1.0, | ||
'initialSize must be less than 1.0, or null meaning not specified.'), | ||
}) : assert((initialSize ?? 1.0) <= 1.0, 'initialSize must be less than 1.0, or null meaning not specified.'), |
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.
Can you format the code so that unrelated changes are not included in this PR?
? MediaQuery.of(context).size.height * _scale | ||
: null, | ||
fit: BoxFit.contain, | ||
child: Flex( |
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.
Do we need Flex
? It seems children
always has only one child and Flex
is not necessary.
Positioned( | ||
left: _imageRect.left, | ||
top: _imageRect.top, | ||
child: Image.memory( |
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.
/// When [borderColor] is set, a border with the given color is added to the | ||
/// image exterior, making it better to see the image ending when | ||
/// the [baseColor] and the image background are similar | ||
final Color? borderColor; |
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.
Why don't we accept Border
, not only Color
?
It enables users to decide other configurations, such as width.
When borderColor is set, a border with the given color is added to the image exterior, making it better to see the image ending when the baseColor and the image background are similar
demo: