Skip to content

165977 Add focus support for CupertinoAlertDialogAction - #179224

Closed
O-Hannonen wants to merge 15 commits into
flutter:masterfrom
O-Hannonen:165977-add-keyboard-focus-support-for-cupertino-alert-dialog-action
Closed

O-Hannonen wants to merge 15 commits into
flutter:masterfrom
O-Hannonen:165977-add-keyboard-focus-support-for-cupertino-alert-dialog-action

Conversation

@O-Hannonen

Copy link
Copy Markdown
Contributor

This PR

How the new implementation looks and behaves

Simulator.Screen.Recording.-.iPhone.16.Plus.-.2025-11-28.at.19.49.11.mov

How the native counterpart looks and behaves

Simulator.Screen.Recording.-.iPhone.16e.-.2025-11-28.at.19.53.35.mov

Sample code

Sample code
import 'package:flutter/cupertino.dart';
import 'package:flutter/material.dart';

void main() => runApp(const AlertDialogApp());

class AlertDialogApp extends StatelessWidget {
  const AlertDialogApp({super.key});

  @override
  Widget build(BuildContext context) {
    return CupertinoApp(
      theme: CupertinoThemeData(brightness: Brightness.light),
      home: AlertDialogExample(),
    );
  }
}

class AlertDialogExample extends StatelessWidget {
  AlertDialogExample({super.key});

  FocusNode twoFocusNode = FocusNode();

  Future<void> _showModal(BuildContext context) {
    return showCupertinoModalPopup(
      context: context,
      builder: (BuildContext context) => CupertinoActionSheet(
        title: Text('title'),
        actions: <Widget>[
          CupertinoActionSheetAction(
            child: Text('text'),
            onPressed: () {
              Navigator.pop(context);
            },
          ),
          CupertinoActionSheetAction(
            child: Text('image'),
            onPressed: () {
              Navigator.pop(context);
            },
          ),
        ],
        cancelButton: CupertinoActionSheetAction(
          child: Text('cancel'),
          isDefaultAction: true,
          onPressed: () {
            Navigator.pop(context);
          },
        ),
      ),
    );
  }

  Future<void> _showAlertDialog(BuildContext context) {
    return showCupertinoDialog<void>(
      context: context,
      builder: (BuildContext context) => CupertinoAlertDialog(
        title: GestureDetector(
          child: const Text('Alert'),
          onTap: () {
            twoFocusNode.requestFocus();
            print(twoFocusNode.hasPrimaryFocus);
          },
        ),
        content: const Text('Proceed with destructive action?'),
        actions: <CupertinoDialogAction>[
          CupertinoDialogAction(
            isDefaultAction: true,
            onPressed: () {
              print('No');
              Navigator.pop(context);
            },
            child: const Text('No'),
          ),

          CupertinoDialogAction(
            isDestructiveAction: true,
            onPressed: () {
              print('Yes');
              Navigator.pop(context);
            },
            child: const Text('Yes'),
          ),
        ],
      ),
    );
  }

  Future<void> _showLongAlertDialog(BuildContext context) {
    return showCupertinoDialog<void>(
      context: context,
      builder: (BuildContext context) => CupertinoAlertDialog(
        title: GestureDetector(
          child: const Text('Alert'),
          onTap: () {
            twoFocusNode.requestFocus();
            print(twoFocusNode.hasPrimaryFocus);
          },
        ),
        content: const Text('Proceed with destructive action?'),
        actions: <CupertinoDialogAction>[
          CupertinoDialogAction(
            isDefaultAction: true,
            onPressed: () {
              print('No');
              Navigator.pop(context);
            },
            child: const Text('No'),
          ),
          CupertinoDialogAction(
            isDefaultAction: true,
            onPressed: () {
              print('Maybe');
              Navigator.pop(context);
            },
            child: const Text('Maybe'),
          ),

          CupertinoDialogAction(
            isDestructiveAction: true,
            onPressed: () {
              print('Yes');
              Navigator.pop(context);
            },
            child: const Text('Yes'),
          ),
        ],
      ),
    );
  }

  @override
  Widget build(BuildContext context) {
    return CupertinoPageScaffold(
      navigationBar: const CupertinoNavigationBar(
        middle: Text('CupertinoAlertDialog Sample'),
      ),
      child: Center(
        child: Column(
          mainAxisAlignment: MainAxisAlignment.center,
          children: [
            CupertinoButton(
              onPressed: () => _showAlertDialog(context),
              child: const Text('CupertinoAlertDialog'),
            ),
            CupertinoButton(
              onPressed: () => _showLongAlertDialog(context),
              child: const Text('long CupertinoAlertDialog'),
            ),
          ],
        ),
      ),
    );
  }
}

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.

@github-actions github-actions Bot added framework flutter/packages/flutter repository. See also f: labels. p: cupertino_ui cupertino_ui package in flutter/packages f: focus Focus traversal, gaining or losing focus labels Nov 28, 2025
@O-Hannonen O-Hannonen changed the title 165977 add keyboard focus support for cupertino alert dialog action 165977 Add focus support for CupertinoAlertDialogAction Nov 28, 2025

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request adds keyboard focus support to CupertinoDialogAction and updates CupertinoFocusHalo to use a superellipse shape, which is a great enhancement for accessibility and visual consistency. The implementation is solid and well-tested. I've left a couple of minor suggestions to simplify color calculations by removing redundant HSLColor conversions, which will improve code clarity and maintainability.

@victorsanni victorsanni 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.

Hi can you fix the merge conflict?

? Border.fromBorderSide(BorderSide(color: _effectiveFocusOutlineColor, width: 3.5))
: null,
decoration: ShapeDecoration(
shape: RoundedSuperellipseBorder(

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.

I'm not sure we can guarantee this will look natural with all current widgets.

Can we do something like CupertinoFocusHalo.roundedSupereillipse to conditionally use RoundedSuperellipseBorder?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The only other internal usage of CupertinoFocusHalo so far is CupertinoActionSheet, and there it was an oversight on my behalf to not use superellipse from the very beginning, as the component otherwise uses it.

But yeah you're absolutely right that this is also exported in cupertino.dart so someone could have already implemented something else on top of this that does not use superellipse. So good point, I'll implement the suggestion!

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.

Thanks for making that update. I think we might want to consider putting this new constructor into it's own separate PR. It has it's own tests specifically for it, so it would be easier to review if split out, and the other PR is dependent on it. If something happens that makes us have to revert this PR because of the CupertinoDialogAction behavior, then we would also have to revert that other PR if it is merged because it needs the shaped halo.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Makes sense, I'll open another PR for that!

);

testWidgets(
'CupertinoDialogActions in CupertinoDialog cannot be traversed with keyboard when onPressed is null',

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.

Looks like this test is failing on some web checks.

@github-actions github-actions Bot removed the f: focus Focus traversal, gaining or losing focus label Jan 9, 2026

@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.

Generally looking good. There are some comments shared by #179225. I suggest we finish #179225 before finalizing the review on this one.

Comment on lines +2260 to +2261
/// A transparency of [kCupertinoButtonTintedOpacityLight] (light mode) or
/// [kCupertinoButtonTintedOpacityDark] (dark mode) is automatically applied to this color.

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.

(Same comment from the other PR) Will it be better to embed the transparency into the default color, so that the app can have full control over the transparency?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good point, let's finish this discussion in #179225 first

github-merge-queue Bot pushed a commit that referenced this pull request Jan 15, 2026
This PR closes #180719

Related to
[this](#179224 (comment))
and
[this](#179225 (comment)).

## 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.

<!-- 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
ikramhasan pushed a commit to ikramhasan/flutter that referenced this pull request Jan 15, 2026
This PR closes flutter#180719

Related to
[this](flutter#179224 (comment))
and
[this](flutter#179225 (comment)).

## 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.

<!-- 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
@Piinks

Piinks commented Feb 3, 2026

Copy link
Copy Markdown
Contributor

Flyby comment from triage - the analyzer is failing here.

@dkwingsmt

Copy link
Copy Markdown
Contributor

I'm converting this PR to a draft since it's blocked by another.

@dkwingsmt
dkwingsmt marked this pull request as draft February 3, 2026 23:54
@flutter-dashboard

Copy link
Copy Markdown

This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again.

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.

@Piinks

Piinks commented Apr 6, 2026

Copy link
Copy Markdown
Contributor

After touching base with @dkwingsmt, we are going to close this PR for now until we can address the related issues. Thank you @O-Hannonen!

@Piinks Piinks closed this Apr 6, 2026
chunhtai pushed a commit to chunhtai/packages that referenced this pull request Jun 12, 2026
This PR closes #180719

Related to
[this](flutter/flutter#179224 (comment))
and
[this](flutter/flutter#179225 (comment)).

## 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.

<!-- 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
Nakiha pushed a commit to Nakiha/VoidPlayer-Flutter that referenced this pull request Sep 28, 2026
This PR closes #180719

Related to
[this](flutter/flutter#179224 (comment))
and
[this](flutter/flutter#179225 (comment)).

## 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.

<!-- 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

framework flutter/packages/flutter repository. See also f: labels. p: cupertino_ui cupertino_ui package in flutter/packages

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Proposal] Add keyboard support for CupertinoAlertDialog

5 participants