Skip to content

Conversation

@harryterkelsen
Copy link
Contributor

Now that #174588 has landed, the Skwasm renderer should work correctly with the perspective transforms in the datepicker widget.

Fixes #110785

Pre-launch Checklist

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

Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the gemini-code-assist bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.

@harryterkelsen harryterkelsen requested a review from mdebbar August 28, 2025 20:49
@github-actions github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: cupertino flutter/packages/flutter/cupertino repository labels Aug 28, 2025
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request correctly removes the logic that was skipping golden tests for the Cupertino date picker in Skwasm, now that the underlying rendering issue is fixed. The changes are straightforward and achieve the goal of re-enabling these tests. I have one suggestion to refactor a block of repetitive test code, which would improve maintainability.

Comment on lines 1597 to +1619
await tester.pumpWidget(buildApp(CupertinoDatePickerMode.time));
if (!skipPerspectiveTextGoldens) {
await expectLater(
find.byType(CupertinoDatePicker),
matchesGoldenFile('date_picker_test.time.initial.png'),
);
}
await expectLater(
find.byType(CupertinoDatePicker),
matchesGoldenFile('date_picker_test.time.initial.png'),
);

await tester.pumpWidget(buildApp(CupertinoDatePickerMode.date));
if (!skipPerspectiveTextGoldens) {
await expectLater(
find.byType(CupertinoDatePicker),
matchesGoldenFile('date_picker_test.date.initial.png'),
);
}
await expectLater(
find.byType(CupertinoDatePicker),
matchesGoldenFile('date_picker_test.date.initial.png'),
);

await tester.pumpWidget(buildApp(CupertinoDatePickerMode.monthYear));
if (!skipPerspectiveTextGoldens) {
await expectLater(
find.byType(CupertinoDatePicker),
matchesGoldenFile('date_picker_test.monthyear.initial.png'),
);
}
await expectLater(
find.byType(CupertinoDatePicker),
matchesGoldenFile('date_picker_test.monthyear.initial.png'),
);

await tester.pumpWidget(buildApp(CupertinoDatePickerMode.dateAndTime));
if (!skipPerspectiveTextGoldens) {
await expectLater(
find.byType(CupertinoDatePicker),
matchesGoldenFile('date_picker_test.datetime.initial.png'),
);
}
await expectLater(
find.byType(CupertinoDatePicker),
matchesGoldenFile('date_picker_test.datetime.initial.png'),
);
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

This block of code is repetitive. You can refactor it into a loop to improve readability and maintainability.1

      final Map<CupertinoDatePickerMode, String> modes = <CupertinoDatePickerMode, String>{
        CupertinoDatePickerMode.time: 'time',
        CupertinoDatePickerMode.date: 'date',
        CupertinoDatePickerMode.monthYear: 'monthyear',
        CupertinoDatePickerMode.dateAndTime: 'datetime',
      };

      for (final MapEntry<CupertinoDatePickerMode, String> mode in modes.entries) {
        await tester.pumpWidget(buildApp(mode.key));
        await expectLater(
          find.byType(CupertinoDatePicker),
          matchesGoldenFile('date_picker_test.${mode.value}.initial.png'),
        );
      }

Style Guide References

Footnotes

@flutter-dashboard
Copy link

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #174666 at sha 83321fb

@flutter-dashboard flutter-dashboard bot added the will affect goldens Changes to golden files label Aug 28, 2025
@mdebbar
Copy link
Contributor

mdebbar commented Aug 28, 2025

Hmm the generated skwasm goldens are still broken 🤔

@harryterkelsen
Copy link
Contributor Author

Hmm the generated skwasm goldens are still broken 🤔

I may have uploaded this PR before merging in the unification PR from main. I'll try rerunning the tests. Hopefully it should be sorted out now. Otherwise, there is clearly more unification that needs to be done.

@harryterkelsen
Copy link
Contributor Author

harryterkelsen commented Aug 28, 2025

@mdebbar The goldens look better now (modulo some anti-aliasing differences between Skwasm and CanvasKit?)

Copy link
Contributor

@mdebbar mdebbar left a comment

Choose a reason for hiding this comment

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

Looks great! Approving again 🙂

@flutter-dashboard
Copy link

Golden file changes are available for triage from new commit, Click here to view.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #174666 at sha 2bd6668

@mdebbar
Copy link
Contributor

mdebbar commented Sep 16, 2025

From backlog review: @harryterkelsen is this still on your radar?

@harryterkelsen harryterkelsen force-pushed the unskip-datepicker-test-skwsam branch from 2bd6668 to 6aefa6a Compare September 18, 2025 21:45
@harryterkelsen
Copy link
Contributor Author

@mdebbar with the renderer unification relanded, this should also be landable now. I rebased on main so we can see the updated goldens, then I'll land this one as well

@flutter-dashboard
Copy link

Golden file changes are available for triage from new commit, Click here to view.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #174666 at sha 6aefa6a

@harryterkelsen harryterkelsen added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 18, 2025
@auto-submit auto-submit bot added this pull request to the merge queue Sep 18, 2025
Merged via the queue into flutter:master with commit 46dfbf5 Sep 18, 2025
79 checks passed
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Sep 18, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 19, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 19, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 19, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 19, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 19, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 20, 2025
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Sep 20, 2025
Roll Flutter from 8f94cb0 to 9ff2767 (56 revisions)

flutter/flutter@8f94cb0...9ff2767

2025-09-20 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from 0_jKqLGnkILvQ5C8a... to CcCe3HpQtBYhTZscb... (flutter/flutter#175698)
2025-09-20 engine-flutter-autoroll@skia.org Roll Dart SDK from e6e9248aee4f to 9e943fe076c8 (1 revision) (flutter/flutter#175697)
2025-09-20 32538273+ValentinVignal@users.noreply.github.com Add `menuController` to `DropdownMenu` (flutter/flutter#175039)
2025-09-20 engine-flutter-autoroll@skia.org Roll Skia from 1dae085e2f31 to a38a531dec1d (3 revisions) (flutter/flutter#175694)
2025-09-20 bruno.leroux@gmail.com [a11y] TimePicker clock is unnecessarily announced (flutter/flutter#175570)
2025-09-20 engine-flutter-autoroll@skia.org Roll Dart SDK from 78e68d1a7dbf to e6e9248aee4f (4 revisions) (flutter/flutter#175690)
2025-09-19 engine-flutter-autoroll@skia.org Roll Skia from b56003bf2c20 to 1dae085e2f31 (1 revision) (flutter/flutter#175674)
2025-09-19 matanlurey@users.noreply.github.com Update `CODEOWNERS` (for dev-tooling) (flutter/flutter#175201)
2025-09-19 sokolovskyi.konstantin@gmail.com [a11y-app] Add label to TextFormField in AutoCompleteUseCase. (flutter/flutter#175576)
2025-09-19 sokolovskyi.konstantin@gmail.com Fix RadioGroup single selection check. (flutter/flutter#175654)
2025-09-19 engine-flutter-autoroll@skia.org Roll Packages from f2a65fd to 3d5c419 (2 revisions) (flutter/flutter#175668)
2025-09-19 engine-flutter-autoroll@skia.org Roll Skia from c74d2bdbd93c to b56003bf2c20 (2 revisions) (flutter/flutter#175665)
2025-09-19 32538273+ValentinVignal@users.noreply.github.com Add `CupertinoLinearActivityIndicator` (flutter/flutter#170108)
2025-09-19 bkonyi@google.com [ Widget Preview ] Don't update filtered preview set when selecting non-source files (flutter/flutter#175596)
2025-09-19 engine-flutter-autoroll@skia.org Roll Dart SDK from 2c79803c97db to 78e68d1a7dbf (3 revisions) (flutter/flutter#175646)
2025-09-19 mdebbar@google.com Delete unused web_unicode library (flutter/flutter#174896)
2025-09-19 engine-flutter-autoroll@skia.org Roll Skia from 684f3a831216 to c74d2bdbd93c (2 revisions) (flutter/flutter#175644)
2025-09-19 engine-flutter-autoroll@skia.org Roll Skia from 462bdece17bf to 684f3a831216 (3 revisions) (flutter/flutter#175641)
2025-09-19 engine-flutter-autoroll@skia.org Roll Skia from a2c38aa9df80 to 462bdece17bf (11 revisions) (flutter/flutter#175629)
2025-09-18 1961493+harryterkelsen@users.noreply.github.com fix(tool): Use merge-base for content hash in detached HEAD (flutter/flutter#175554)
2025-09-18 1961493+harryterkelsen@users.noreply.github.com [web] Unskip Cupertino datepicker golden tests in Skwasm (flutter/flutter#174666)
2025-09-18 ryjohn@google.com Update rules to include extension rules (flutter/flutter#175618)
2025-09-18 Breakthrough@users.noreply.github.com [engine] Cleanup Fuchsia FDIO library dependencies (flutter/flutter#174847)
2025-09-18 ahmedsameha1@gmail.com Make sure that a CloseButton doesn't crash in 0x0 environment (flutter/flutter#172902)
2025-09-18 bkonyi@google.com [ Tool ] Serve DevTools from DDS, remove ResidentDevToolsHandler (flutter/flutter#174580)
2025-09-18 Breakthrough@users.noreply.github.com [engine][fuchsia] Update to Fuchsia API level 28 and roll latest GN SDK (flutter/flutter#175425)
2025-09-18 jessiewong401@gmail.com Added a 36 device for Firebase Lab Testing (flutter/flutter#175613)
2025-09-18 engine-flutter-autoroll@skia.org Roll Dart SDK from 09a101793af4 to 2c79803c97db (2 revisions) (flutter/flutter#175608)
2025-09-18 louisehsu@google.com Engine Support for Dynamic View Resizing (flutter/flutter#173610)
2025-09-18 engine-flutter-autoroll@skia.org Roll Packages from fdee698 to f2a65fd (3 revisions) (flutter/flutter#175594)
2025-09-18 15619084+vashworth@users.noreply.github.com Connect the FlutterEngine to the FlutterSceneDelegate (flutter/flutter#174910)
2025-09-18 engine-flutter-autoroll@skia.org Roll Dart SDK from de5dd0f1530f to 09a101793af4 (2 revisions) (flutter/flutter#175583)
2025-09-18 engine-flutter-autoroll@skia.org Roll Skia from 7b9fe91446ee to a2c38aa9df80 (1 revision) (flutter/flutter#175579)
2025-09-18 engine-flutter-autoroll@skia.org Roll Skia from ab1b10547461 to 7b9fe91446ee (4 revisions) (flutter/flutter#175569)
2025-09-18 bruno.leroux@gmail.com [a11y-app] Fix form field label and error message (flutter/flutter#174831)
2025-09-18 bruno.leroux@gmail.com Fix InputDecoration does not apply errorStyle to error (flutter/flutter#174787)
2025-09-18 32538273+ValentinVignal@users.noreply.github.com Migrate to `WidgetPropertyResolver` (flutter/flutter#175397)
2025-09-18 32538273+ValentinVignal@users.noreply.github.com Migrate to WidgetState (flutter/flutter#175396)
2025-09-18 engine-flutter-autoroll@skia.org Roll Skia from 79ec8dfcd9d4 to ab1b10547461 (17 revisions) (flutter/flutter#175561)
2025-09-18 30870216+gaaclarke@users.noreply.github.com Removes NOTICES from licenses input (flutter/flutter#174967)
2025-09-18 engine-flutter-autoroll@skia.org Roll Dart SDK from 116f7fe72839 to de5dd0f1530f (2 revisions) (flutter/flutter#175557)
2025-09-17 1961493+harryterkelsen@users.noreply.github.com [reland][web] Refactor renderers to use the same frontend code #174588 (flutter/flutter#175392)
2025-09-17 erickzanardoo@gmail.com feat: Enable WidgetStateColor to be used in ChipThemeData.deleteIconColor (flutter/flutter#171646)
2025-09-17 15619084+vashworth@users.noreply.github.com Filter out unexpected process logs on iOS with better regex matching. (flutter/flutter#175452)
2025-09-17 victorsanniay@gmail.com CupertinoContextMenu child respects available screen width (flutter/flutter#175300)
2025-09-17 38427679+xVemu@users.noreply.github.com Correct documentation in PredictiveBackFullscreenPageTransitionsBuilder (flutter/flutter#174362)
...
Jaineel-Mamtora pushed a commit to Jaineel-Mamtora/flutter_forked that referenced this pull request Sep 24, 2025
)

Now that flutter#174588 has landed, the
Skwasm renderer should work correctly with the perspective transforms in
the datepicker widget.

Fixes flutter#110785

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [x] I signed the [CLA].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [x] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [x] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [x] All existing and new tests are passing.

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

**Note**: The Flutter team is currently trialing the use of [Gemini Code
Assist for
GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code).
Comments from the `gemini-code-assist` bot should not be taken as
authoritative feedback from the Flutter team. If you find its comments
useful you can update your code accordingly, but if you are unsure or
disagree with the feedback, please feel free to wait for a Flutter team
member's review for guidance on which automated comments should be
addressed.

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 12, 2025
lucaantonelli pushed a commit to lucaantonelli/flutter that referenced this pull request Nov 21, 2025
)

Now that flutter#174588 has landed, the
Skwasm renderer should work correctly with the perspective transforms in
the datepicker widget.

Fixes flutter#110785

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [x] I signed the [CLA].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [x] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [x] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [x] All existing and new tests are passing.

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

**Note**: The Flutter team is currently trialing the use of [Gemini Code
Assist for
GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code).
Comments from the `gemini-code-assist` bot should not be taken as
authoritative feedback from the Flutter team. If you find its comments
useful you can update your code accordingly, but if you are unsure or
disagree with the feedback, please feel free to wait for a Flutter team
member's review for guidance on which automated comments should be
addressed.

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
reidbaker pushed a commit to AbdeMohlbi/flutter that referenced this pull request Dec 10, 2025
)

Now that flutter#174588 has landed, the
Skwasm renderer should work correctly with the perspective transforms in
the datepicker widget.

Fixes flutter#110785

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [x] I signed the [CLA].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [x] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [x] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [x] All existing and new tests are passing.

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

**Note**: The Flutter team is currently trialing the use of [Gemini Code
Assist for
GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code).
Comments from the `gemini-code-assist` bot should not be taken as
authoritative feedback from the Flutter team. If you find its comments
useful you can update your code accordingly, but if you are unsure or
disagree with the feedback, please feel free to wait for a Flutter team
member's review for guidance on which automated comments should be
addressed.

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

f: cupertino flutter/packages/flutter/cupertino repository framework flutter/packages/flutter repository. See also f: labels. will affect goldens Changes to golden files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Canvaskit datepicker skiagold tests are flaky

2 participants