-
Notifications
You must be signed in to change notification settings - Fork 3.6k
[two_dimensional_scrollables] Fix paint bug when rows/columns are pinned and axes are reversed #5038
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
Conversation
goderbauer
left a comment
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.
LGTM
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.
Just wondering why this one and the next one is included in this PR as "changed". Aren't they 100% identical?
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.
Yeah I was curious about this too. Probably really minor pixel diffs. Running flutter test --update-goldens output new images per git when I ran it for the new golden test here, so I figured I would let them be updated.
| foregroundDecoration: const TableSpanDecoration( | ||
| border: TableSpanBorder( | ||
| trailing: BorderSide( | ||
| color: Colors.orange, | ||
| width: 3, | ||
| ))), |
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.
formatting nit (probably just missing some trailing commas?):
| foregroundDecoration: const TableSpanDecoration( | |
| border: TableSpanBorder( | |
| trailing: BorderSide( | |
| color: Colors.orange, | |
| width: 3, | |
| ))), | |
| foregroundDecoration: const TableSpanDecoration( | |
| border: TableSpanBorder( | |
| trailing: BorderSide( | |
| color: Colors.orange, | |
| width: 3, | |
| ), | |
| ), | |
| ), |
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.
Good catch, I found a couple more cases of this, fixed them all. :)
| leading: BorderSide( | ||
| color: Colors.green, | ||
| width: 3, | ||
| ))), |
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.
same nit here
… are pinned and axes are reversed (flutter/packages#5038)
… are pinned and axes are reversed (flutter/packages#5038)
… are pinned and axes are reversed (flutter/packages#5038)
flutter/packages@4b483f2...93c3f69 2023-10-12 [email protected] [shared_preferences] update file version constraints (flutter/packages#5121) 2023-10-12 [email protected] [cross_file] Improved documentation about ignored parameters in IO module. (flutter/packages#4416) 2023-10-11 [email protected] [in_app_purchase] [#135759] Fix. doc reference finishPurchase to completePurchase. (flutter/packages#5081) 2023-10-10 [email protected] Update repo README issues link (flutter/packages#5106) 2023-10-10 [email protected] [flutter_markdown] Fix typo in readme (flutter/packages#5096) 2023-10-10 [email protected] [camerax] Implements torch mode (flutter/packages#4903) 2023-10-10 [email protected] [shared_preferences] fix documentation wording (flutter/packages#4986) 2023-10-10 [email protected] [two_dimensional_scrollables] Fix paint bug when rows/columns are pinned and axes are reversed (flutter/packages#5038) 2023-10-10 [email protected] Roll Flutter from f52fe4f to 83134ac (25 revisions) (flutter/packages#5104) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
… are reversed (#5916) Fixes flutter/flutter#141704 This fixes the rect computation for TableSpanDecorations when one or both axes are reversed. We previously fixed similar issues when the clipRects applied to areas of the table did not account for reversed. This is another similar case. - #5187 - #5038 
Fixes flutter/flutter#135386
This adds one more golden to verify the painting, but once public mock_canvas rolls to stable in flutter_test, we can remove all of the goldens and just verify using the
paintsmethod.Pre-launch Checklist
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style.///).If you need help, consider asking for advice on the #hackers-new channel on Discord.