Skip to content
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

[two_dimensional_scrollables] Fix paint bug when rows/columns are pinned and axes are reversed #5038

Merged
merged 6 commits into from
Oct 10, 2023

Conversation

Piinks
Copy link
Contributor

@Piinks Piinks commented Sep 29, 2023

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 paints method.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use dart format.)
  • I signed the CLA.
  • The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
  • I listed at least one issue that this PR fixes in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.
  • I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Copy link
Member

@goderbauer goderbauer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

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?

Copy link
Contributor Author

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.

Comment on lines 1057 to 1062
foregroundDecoration: const TableSpanDecoration(
border: TableSpanBorder(
trailing: BorderSide(
color: Colors.orange,
width: 3,
))),
Copy link
Member

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?):

Suggested change
foregroundDecoration: const TableSpanDecoration(
border: TableSpanBorder(
trailing: BorderSide(
color: Colors.orange,
width: 3,
))),
foregroundDecoration: const TableSpanDecoration(
border: TableSpanBorder(
trailing: BorderSide(
color: Colors.orange,
width: 3,
),
),
),

Copy link
Contributor Author

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,
))),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same nit here

@Piinks Piinks added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 10, 2023
@auto-submit auto-submit bot merged commit bec2c95 into flutter:main Oct 10, 2023
70 checks passed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 11, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 11, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 12, 2023
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Oct 12, 2023
flutter/packages@4b483f2...93c3f69

2023-10-12 goderbauer@google.com [shared_preferences] update file version constraints (flutter/packages#5121)
2023-10-12 supercionci@hotmail.it [cross_file] Improved documentation about ignored parameters in IO module. (flutter/packages#4416)
2023-10-11 41873024+droidbg@users.noreply.github.com [in_app_purchase] [#135759] Fix. doc reference finishPurchase to completePurchase. (flutter/packages#5081)
2023-10-10 zeucxb@gmail.com Update repo README issues link (flutter/packages#5106)
2023-10-10 43643339+nohli@users.noreply.github.com [flutter_markdown] Fix typo in readme (flutter/packages#5096)
2023-10-10 43054281+camsim99@users.noreply.github.com [camerax] Implements torch mode (flutter/packages#4903)
2023-10-10 25266387+Leptopoda@users.noreply.github.com [shared_preferences] fix documentation wording (flutter/packages#4986)
2023-10-10 katelovett@google.com [two_dimensional_scrollables] Fix paint bug when rows/columns are pinned and axes are reversed (flutter/packages#5038)
2023-10-10 engine-flutter-autoroll@skia.org 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 flutter-ecosystem@google.com,rmistry@google.com 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
HugoOlthof pushed a commit to moneybird/packages that referenced this pull request Dec 13, 2023
…ned and axes are reversed (flutter#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 `paints` method.
auto-submit bot pushed a commit that referenced this pull request Jan 22, 2024
… 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

![image](https://github.com/flutter/packages/assets/16964204/ae7c6872-c740-4617-95a4-8302a3085d59)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit Merge PR when tree becomes green via auto submit App p: two_dimensional_scrollables Issues pertaining to the two_dimensional_scrollables package
Projects
None yet
2 participants