max-width attribute and auto-margins on network images for Flutter 3.10 #1359
Conversation
|
|
@TheCarpetMerchant Do you intend to finish it or would you prefer someone to take over? |
|
@eEQK Would prefer someone at least look it over. I didn't even run tests because my local setup kept erroring out ; it just works for my use case as is. If what I did is enough then great, but it would still be better someone take it over, especially to separate the 2 commits which are really two different things. |
lib/src/css_box_widget.dart
Outdated
| Width? maxWidthCalculated; | ||
| if (style.maxWidth != null && style.width != null) { | ||
| if (style.maxWidth!.unit == Unit.percent && | ||
| style.width!.unit == Unit.px) { | ||
| // If our max is a percentage, we want to look at the size available and not be bigger than that. | ||
| try { | ||
| double width = | ||
| MediaQuery.of(context).size.width * (style.maxWidth!.value / 100); | ||
| maxWidthCalculated = Width(width, style.width!.unit); | ||
| } catch (_) {} | ||
| } else if (style.width!.unit == style.maxWidth!.unit && | ||
| style.width!.value > style.maxWidth!.value) { | ||
| maxWidthCalculated = Width(style.maxWidth!.value, style.maxWidth!.unit); | ||
| } | ||
| } |
There was a problem hiding this comment.
Could be extracted to a method called calculateMaxWidth and then you'd have final maxWidthCalculated = _calculateMaxWidth(style);
this way you don't have to use a var (finals should be used whenever possible)
lib/src/css_box_widget.dart
Outdated
| double width = | ||
| MediaQuery.of(context).size.width * (style.maxWidth!.value / 100); | ||
| maxWidthCalculated = Width(width, style.width!.unit); | ||
| } catch (_) {} |
There was a problem hiding this comment.
silent errors are generally not a good idea - you should print the exception with a stacktrace at the very least (if this code happens to fail, you won't know unless you debug carefully the whole flow)
lib/src/css_box_widget.dart
Outdated
| @@ -59,8 +59,24 @@ class CssBoxWidget extends StatelessWidget { | |||
| final direction = _checkTextDirection(context, textDirection); | |||
| final padding = style.padding?.resolve(direction); | |||
|
|
|||
| Width? maxWidthCalculated; | |||
| if (style.maxWidth != null && style.width != null) { | |||
There was a problem hiding this comment.
| if (style.maxWidth != null && style.width != null) { | |
| final maxWidth = style.maxWidth; | |
| final width = style.width; | |
| if (maxWidth != null && width != null) { |
this way you don't have to use the bang operator below (style.width!), just use width
There was a problem hiding this comment.
also, if it's not entirely clear what an if checks for, I really like to assign names to stuff I put inside if, e.g. instead of
if (maxWidth != null && width != null) {I would do
final hasWidthConstraint = maxWidth != null && width != null;
if(hasWidthConstraint)but I'd say it's fine to leave the one below as it is since it's pretty self-explanatory what you're trying to do there
if (maxWidth.unit == Unit.percent && width.unit == Unit.px)
lib/src/css_box_widget.dart
Outdated
| maxWidthCalculated = Width(width, style.width!.unit); | ||
| } catch (_) {} | ||
| } else if (style.width!.unit == style.maxWidth!.unit && | ||
| style.width!.value > style.maxWidth!.value) { |
There was a problem hiding this comment.
I think here would also be a good idea to name what the condition checks for
|
@eEQK Finally back to it. What's missing for now it the ability to compare 2 Widths that aren't in the same unit, if you know how to do that on a sufficient "best effort" basis at least that'd be great. |
This PR is not ready to be merged. I made it as best as I could while trying to learn the inner workings of this beautiful package :)
CssBoxWidget; I have no idea if this is complete or even correct.shrinkWrapto network images so they could work with Flutter 3.10, which apparently is the cause of images going out of screen ([BUG] Images extend beyond layout boundaries in flutter_html: ^3.0.0-beta.2 #1357)Hopefully, it's not too shabby. The use of shrinkWrap should probably be removed in favor of understanding what's going wrong in the
_calculateUsedMargins, but I haven't even tried given shrinkWrap fixes it (by bypassing the issue), at least AFAIK.