Skip to content

Fix: Ensure CupertinoAlertDialog divider spans full width and resolve - #161490

Merged
auto-submit[bot] merged 10 commits into
flutter:masterfrom
dev-lup:fix/cupertino-d-h-divider
Feb 6, 2025
Merged

auto-submit[bot] merged 10 commits into
flutter:masterfrom
dev-lup:fix/cupertino-d-h-divider

Conversation

@dev-lup

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

Copy link
Copy Markdown
Contributor

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
Copy Markdown

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

google-cla Bot commented Jan 12, 2025

Copy link
Copy Markdown

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. p: cupertino_ui cupertino_ui package in flutter/packages labels Jan 12, 2025
@Piinks

Piinks commented Jan 15, 2025

Copy link
Copy Markdown
Contributor

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

@Piinks

Piinks commented Jan 15, 2025

Copy link
Copy Markdown
Contributor

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

@dev-lup

dev-lup commented Jan 16, 2025

Copy link
Copy Markdown
Contributor Author

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

@dev-lup

dev-lup commented Jan 17, 2025

Copy link
Copy Markdown
Contributor Author

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
Copy Markdown
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

@dkwingsmt dkwingsmt left a comment

Copy link
Copy Markdown
Contributor

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
Comment thread packages/flutter/lib/src/cupertino/dialog.dart Outdated

@chunhtai chunhtai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@chunhtai

chunhtai commented Feb 4, 2025

Copy link
Copy Markdown
Contributor

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

@dev-lup

dev-lup commented Feb 4, 2025 •

Copy link
Copy Markdown
Contributor Author

on running auto format it changes more then my code.

@dkwingsmt

Copy link
Copy Markdown
Contributor

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

@dev-lup

dev-lup commented Feb 4, 2025

Copy link
Copy Markdown
Contributor Author

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
Copy Markdown
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

dev-lup commented Feb 4, 2025

Copy link
Copy Markdown
Contributor Author

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
Copy Markdown
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

dev-lup commented Feb 4, 2025

Copy link
Copy Markdown
Contributor Author

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

framework flutter/packages/flutter repository. See also f: labels. p: cupertino_ui cupertino_ui package in flutter/packages 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