Skip to content

Conversation

@dev-lup
Copy link
Contributor

@dev-lup dev-lup commented Jan 12, 2025

Fix: Ensure CupertinoAlertDialog divider spans full width and resolves divider color (#158522)

The _Divider in CupertinoAlertDialog did not span the full width and the dividerColor was not resolved correctly. This fix uses a SizedBox with double.infinity as the width for the divider and ensures that the dividerColor is properly applied.

Fixes #158522

Replace this paragraph with a description of what this PR is changing or adding, and why. Consider including before/after screenshots.

List which issues are fixed by this PR. You must list at least one issue. An issue is not required if the PR fixes something trivial like a typo.

If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.

Pre-launch Checklist

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

…s divider color (flutter#158522)

The `_Divider` in CupertinoAlertDialog did not span the full width and the `dividerColor` was not resolved correctly. This fix uses a `SizedBox` with `double.infinity` as the width for the divider and ensures that the `dividerColor` is properly applied.

Fixes flutter#158522
@flutter-dashboard
Copy link

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.

@google-cla
Copy link

google-cla bot commented Jan 12, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@github-actions github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: cupertino flutter/packages/flutter/cupertino repository labels Jan 12, 2025
@Piinks
Copy link
Contributor

Piinks commented Jan 15, 2025

Hey @dev-lup, thanks for the contribution! There are some failing checks here, can you take a look?

@Piinks
Copy link
Contributor

Piinks commented Jan 15, 2025

This will also need a test, and a signed CLA, please see the bot comments above.

@dev-lup
Copy link
Contributor Author

dev-lup commented Jan 16, 2025

hi @Piinks , I signed the CLA, will write the test and let you know, thanks for reply

@dev-lup
Copy link
Contributor Author

dev-lup commented Jan 17, 2025

hi @Piinks, I pushed test as well, I checked the failing tests but could not able to understand them, please check if you can guide me through.

@dkwingsmt dkwingsmt self-requested a review January 22, 2025 19:24
@dkwingsmt
Copy link
Contributor

The customer test failure should have nothing to do with your PR and updating to the latest main should likely solve it.

There are some format issues. Can you run the autoformatter? https://docs.flutter.dev/tools/formatting

Copy link
Contributor

@dkwingsmt dkwingsmt left a comment

Choose a reason for hiding this comment

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

LGTM, thank you for investigating!

@chunhtai chunhtai self-requested a review February 3, 2025 23:55
Copy link
Contributor

@chunhtai chunhtai left a comment

Choose a reason for hiding this comment

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

LGTM

@chunhtai
Copy link
Contributor

chunhtai commented Feb 4, 2025

looks like there are some format error, can you run formatter on the code? or just turn on auto format

@dev-lup
Copy link
Contributor Author

dev-lup commented Feb 4, 2025

on running auto format it changes more then my code.

@dkwingsmt
Copy link
Contributor

Can you check out the "Linux analyse" details and check out the errors?

@dev-lup
Copy link
Contributor Author

dev-lup commented Feb 4, 2025

something like this is failing, can you tell me what may went wrong.
Command: dart --enable-asserts /b/s/w/ir/x/w/flutter/dev/bots/analyze.dart
Command exited with exit code 1 but expected zero exit code.

@dkwingsmt
Copy link
Contributor

If you click into the details you should see the following: (I've copied it out for you)


                                                                                                    
Found 2 Dart files which were formatted incorrectly.
To fix, run `dart format packages/flutter/test/cupertino/dialog_test.dart packages/flutter/lib/src/cupertino/dialog.dart` or:

git apply <<DONE
diff --git a/packages/flutter/test/cupertino/dialog_test.dart b/packages/flutter/test/cupertino/dialog_test.dart
index 4013d0d126..0000000000 100644
--- a/packages/flutter/test/cupertino/dialog_test.dart
+++ b/packages/flutter/test/cupertino/dialog_test.dart
@@ -1989,12 +1989,14 @@ void main() {
     );
   });
 
-  testWidgets('CupertinoAlertDialog divider spans full width and applies color', (WidgetTester tester) async {
-      const double kCupertinoDialogWidth = 270.0;
-      const double kDividerThickness = 0.3;
-      const Size expectedSize = Size(kCupertinoDialogWidth, kDividerThickness);
+  testWidgets('CupertinoAlertDialog divider spans full width and applies color', (
+    WidgetTester tester,
+  ) async {
+    const double kCupertinoDialogWidth = 270.0;
+    const double kDividerThickness = 0.3;
+    const Size expectedSize = Size(kCupertinoDialogWidth, kDividerThickness);
 
-      await tester.pumpWidget(
+    await tester.pumpWidget(
       CupertinoApp(
         home: MediaQuery(
           data: const MediaQueryData(platformBrightness: Brightness.dark),
@@ -2016,16 +2018,17 @@ void main() {
 
     final Finder decoratedBoxFinder = find.byType(DecoratedBox);
 
-    expect(decoratedBoxFinder, findsAny, reason:  'There should exist at least one DecoratedBox');
+    expect(decoratedBoxFinder, findsAny, reason: 'There should exist at least one DecoratedBox');
 
-    final Iterable<Element> elements = decoratedBoxFinder.evaluate().where(
-      (Element decoratedBoxElement) {
-        final DecoratedBox decoratedBox = decoratedBoxElement.widget as DecoratedBox;
-        return (decoratedBox.decoration is BoxDecoration?) && (decoratedBox.decoration as BoxDecoration?)?.color ==
-                CupertinoDynamicColor.resolve(
-                    CupertinoColors.separator, decoratedBoxElement) &&  tester.getSize(find.byWidget(decoratedBox)) == expectedSize;
-      },
-    );
+    final Iterable<Element> elements = decoratedBoxFinder.evaluate().where((
+      Element decoratedBoxElement,
+    ) {
+      final DecoratedBox decoratedBox = decoratedBoxElement.widget as DecoratedBox;
+      return (decoratedBox.decoration is BoxDecoration?) &&
+          (decoratedBox.decoration as BoxDecoration?)?.color ==
+              CupertinoDynamicColor.resolve(CupertinoColors.separator, decoratedBoxElement) &&
+          tester.getSize(find.byWidget(decoratedBox)) == expectedSize;
+    });
 
     expect(elements.length, 1, reason: 'No DecoratedBox matches the specified criteria.');
   });
@@ -2188,3 +2191,4 @@ class LegacyAction extends StatelessWidget {
     );
   }
 }
diff --git a/packages/flutter/lib/src/cupertino/dialog.dart b/packages/flutter/lib/src/cupertino/dialog.dart
index 94b2d305b5..0000000000 100644
--- a/packages/flutter/lib/src/cupertino/dialog.dart
+++ b/packages/flutter/lib/src/cupertino/dialog.dart
@@ -408,8 +408,13 @@ class _CupertinoAlertDialogState extends State<CupertinoAlertDialog> {
             bottom: Column(
               children: <Widget>[
                 SizedBox(
-                width: double.infinity,
-                child: _Divider(dividerColor: dividerColor, hiddenColor: backgroundColor, hidden: false)),
+                  width: double.infinity,
+                  child: _Divider(
+                    dividerColor: dividerColor,
+                    hiddenColor: backgroundColor,
+                    hidden: false,
+                  ),
+                ),
                 Flexible(child: scrolledActionsSection),
               ],
             ),
@@ -2590,3 +2595,4 @@ class _RenderPriorityColumn extends RenderFlex {
     return (topChildHeight: 0, bottomChildHeight: maxHeight);
   }
 }
DONE

╔═╡ERROR #1╞════════════════════════════════════════════════════════════════════
║ Command: bin/cache/dart-sdk/bin/dart --enable-asserts /b/s/w/ir/x/w/flutter/dev/tools/bin/format.dart
║ Command exited with exit code 1 but expected zero exit code.
║ Working directory: /b/s/w/ir/x/w/flutter
╚═══════════════════════════════════════════════════════════════════════════════
▌20:12:28▐ Analyzing code in the framework with the following rules:
 * No "double.clamp"
 * No "Stopwatch"
 * RenderBox subclass intrinsic calculation best practices
 * Add "@protected" to public State subtypes
▌20:14:10▐ Analyzing code in the test folder with the following rules:
 * No "Stopwatch"
▌20:15:37▐ Analyzing code in the tool with the following rules:
 * Avoid "Future.catchError" and "Future.onError"
▌20:16:37▐ Executable allowlist...
▌20:16:37▐ Dart analysis (with --watch)...
RUNNING: cd .; bin/flutter analyze --flutter-repo --watch --benchmark
workingDirectory: /b/s/w/ir/x/w/flutter, executable: /b/s/w/ir/x/w/flutter/bin/flutter, arguments: [analyze, --flutter-repo, --watch, --benchmark]
Analyzing Flutter repository...                                    21.3s

@dev-lup
Copy link
Contributor Author

dev-lup commented Feb 4, 2025

thanks for helping me, but on running

dart format packages/flutter/test/cupertino/dialog_test.dart packages/flutter/lib/src/cupertino/dialog.dart

its changing more code then I am expecting.

I am attaching git diff for you to check.
log.txt

@dkwingsmt
Copy link
Contributor

Sorry, I'm not sure either. Haven't used the autoformatter much since the autoformating change. Can you manually edit the files or pick the files from the git changes?

@dev-lup
Copy link
Contributor Author

dev-lup commented Feb 4, 2025

I can try formatting the code according to the formatter.

Thanks

@dev-lup dev-lup force-pushed the fix/cupertino-d-h-divider branch from 1c9044d to 3a8a97f Compare February 4, 2025 07:04
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 7, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 8, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 8, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 9, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 9, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 10, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 10, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 10, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 10, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 10, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 11, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 11, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 11, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 11, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 11, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 20, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 20, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 21, 2025
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.

Cupertino dialog horizontal divider is not showing correctly in dark mode

5 participants