It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group.

This PR will need images with side-by-side comparisons with native, new tests, and fixes to existing tests.

This PR will need images with side-by-side comparisons with native, new tests, and fixes to existing tests.

Sure @victorsanni! I will add images with a side-by-side comparison with native, and also fix the existing tests. However, I'm having a bit of trouble understanding the golden test documentation. Could you please help me with the following:

Do we need to create or update the golden test case? If yes, what are the steps involved in doing so?

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 #157218 at sha a1da43a

Sure @victorsanni! I will add images with a side-by-side comparison with native, and also fix the existing tests. However, I'm having a bit of trouble understanding the golden test documentation. Could you please help me with the following:

Do we need to create or update the golden test case? If yes, what are the steps involved in doing so?

Considering its a color change, the goldens are expected to change.

Do we need to create or update the golden test case? If yes, what are the steps involved in doing so?

Nothing you need to do. One of us with access to the golden test tool will triage it and approve the changes from this PR when it's ready to merge. As such, screenshots will be helpful.

Can you explain how you got those numbers? And can you illustrate (probably with a side-by-side comparison, as mentioned by comments above) how you verify that those numbers produce native-like effects?

Thank you for you contribution!

Hi @victorsanni, @MitchellGoodwin, @dkwingsmt, here's the side-by-side comparison with native.

Native	Flutter

Also, @dkwingsmt, I derived those numbers from GitHub Issue #80921, where they were initially suggested for achieving native-like effects.

To verify the results, I created an alert dialog box in SwiftUI and manually compared the colors to ensure accuracy.

Hi @victorsanni, @MitchellGoodwin, @dkwingsmt, here's the side-by-side comparison with native.

Are the Flutter images before or after this PR? It would be easy to see the changes if the images show native, flutter before, and flutter after.

It looks like the separator between the actions and the main content needs to be lightened as well.

And the middle divider is also a little too light. Just from visually looking at it I wonder if the dividers are translucent. @thejitenpatel when trying to fix this, it might be helpful to draw the native dialog in different color backgrounds.

Hi @victorsanni, @MitchellGoodwin as requested, here's the detailed comparison of before and after PR changes with native.

Before PR

Native	Flutter (master)

After PR

Native	Flutter (PR #157218)

@thejitenpatel to add to @dkwingsmt's comment, there's no pressure to add the separator fix. But it would be nice to have that here as well, in order to close the linked issue entirely :)

Thank you Victor for the addition, although I must note that the separator issue is actually really complicated, and I would recommend doing so in a different PR.

(I don't know why the current screenshots show almost no horizontal separators, maybe it's a bug)

I can't find the issues on this problem for now, but there has been back and forth on how the thickness of the separator is defined, and my latest discovery is that iOS devices draw them with 1 physical pixel thick. Yes, this means that the separator on a low-res iPhone is thicker than a high-res one. But how should Flutter handle it? There used to be complaint on the separators looking differently on Android.

In short, fixing the separator might require further investigation and comparison. (Feel free to research on it though!)

I saw there was another open issue for the separators: #61164

Maybe this issue can be closed with this PR and target the separator fix to that issue instead?

I saw there was another open issue for the separators: #61164

Maybe this issue can be closed with this PR and target the separator fix to that issue instead?

Sounds good!

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 #157218 at sha a942ab0

Hi @MitchellGoodwin,

Thanks for the suggestion! I tried using CupertinoColors.separator.resolveFrom(context) & final Color dividerColor = CupertinoDynamicColor.resolve(CupertinoColors.separator, context); as recommended, but it doesn't seem to be working as expected. The divider color still isn’t adapting to the theme correctly. Do you have any additional suggestions or insights on what might be going wrong? Any help would be appreciated!

Have you checked whether the value of dividerColor has changed as intended?

The divider color still isn’t adapting to the theme correctly.

Is it showing as the same color as before? And is it not picking up the brightness, the elevation, or both?

CupertinoDynamicColor.resolve may need to be called further down. Possibly in the _Divider class itself.

I haven't tested to be sure, but I believe that this PR didn't introduce that behavior, so it's not required to fix it in this PR. But my instinct is that the divider color will be an easy fix somewhere, it just may need some trial and error to find where it is. Give it a try and see what happens. If it doesn't work after a bit of trying, I'll approve the PR as-is and make a separate issue for that divider.

Hi @MitchellGoodwin,
Thanks for the suggestion! I tried using CupertinoColors.separator.resolveFrom(context) & final Color dividerColor = CupertinoDynamicColor.resolve(CupertinoColors.separator, context); as recommended, but it doesn't seem to be working as expected. The divider color still isn’t adapting to the theme correctly. Do you have any additional suggestions or insights on what might be going wrong? Any help would be appreciated!

Have you checked whether the value of dividerColor has changed as intended?

Hi @dkwingsmt,

I've checked that the dividerColor is changing as expected, but it seems the actual Divider itself isn't reflecting this change.

Is it showing as the same color as before? And is it not picking up the brightness, the elevation, or both?

CupertinoDynamicColor.resolve may need to be called further down. Possibly in the _Divider class itself.

@MitchellGoodwin, It’s showing as the same color as before—no noticeable change. It doesn’t seem to be picking up the brightness correctly, and the elevation appears to be slightly lower than expected.

Attaching screenshot of this PR for your reference:

Hi @MitchellGoodwin,

You’re correct that this PR didn’t introduce the behavior. The issue occurs specifically when a horizontal divider is drawn—it doesn’t resolve the color as expected (screenshot 1.0). It is taking elevatedColor property if I'm not wrong, whereas the vertical divider resolves it correctly via the _CupertinoAlertActionSection class (screenshot 1.1).

To address this, I added CupertinoDynamicColor.resolve in the _CupertinoAlertDialogState class, which helps resolve the color issue for the horizontal divider. However, it still doesn’t fully address the brightness, and the elevation appears slightly less than expected.

To further address the color issue, as suggested, I also applied CupertinoDynamicColor.resolve directly in the Divider class (screenshot 1.2). The Color got resolved but it's not taking brightness if I am not wrong and elevation seems little less as expected (screenshot 2.0).

Interestingly, if there are more than two buttons, all horizontal dividers render with the correct color, except for the first one (screenshot 3.0)

Screenshot 1.0:

Screentshot 1.1:

Screenshot 1.2:

Screenshot 2.0: Dialog with single button in this PR

Screenshot 3.0: Current behavior after adding more than two buttons in this PR

Oh! You're right. I think I made a mistake at
https://github.com/flutter/flutter/pull/150410/files#diff-4ea2907696de4fecf42f2ca56278a7b1d90c28921b1259d18ae28b1a2553d8e8R378

The dividerColor is never resolved here before being passed to _Divider, which doesn't resolve either. Thank you for your investigation!