Skip to content

dialog: Styling for action-button text when the action is destructive #1032

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
chrisbobbe opened this issue Oct 29, 2024 · 3 comments
Open
Labels
a-design Visual and UX design
Milestone

Comments

@chrisbobbe
Copy link
Collaborator

We have a function for a confirmation / suggested-action dialog:

void showSuggestedActionDialog({
  required BuildContext context,
  required String title,
  required String message,
  required String? actionButtonText,
  required VoidCallback onActionButtonPress,
}) {
  final zulipLocalizations = ZulipLocalizations.of(context);
  showDialog<void>(
    context: context,
    builder: (BuildContext context) => AlertDialog(
      title: Text(title),
      content: SingleChildScrollView(child: Text(message)),
      actions: [
        TextButton(
          onPressed: () => Navigator.pop(context),
          child: _dialogActionText(zulipLocalizations.dialogCancel)),
        TextButton(
          onPressed: onActionButtonPress,
          child: _dialogActionText(actionButtonText ?? zulipLocalizations.dialogContinue)),
      ]));
}

It should grow a param bool destructive = false, and, when that's true, show the action button in a style that's appropriate for a destructive action.

On iOS (post-#996) this should be straightforward: just pass isDestructiveAction: true to CupertinoDialogAction.

In Material 3 style for Android, it looks like there isn't actually a special "destructive"/"danger" style for destructive actions. See the "Dialogs" M3 doc, which even includes a confirmation dialog for a "Delete" action, and the "Delete" button is styled just the same as the "Cancel" button. So, until/unless we get a design from Vlad that says otherwise, we'll just keep the styling we get by default, and give showSuggestedActionDialog a dartdoc that says the new param only has an effect on iOS.

@Gaurav-Kushwaha-1225
Copy link
Contributor

Hello @chrisbobbe,

Could you please assign this issue to me? I'm a first-time contributor and have already completed the project setup.
I've also resolved this issue.

Looking forward to your response!

@Gaurav-Kushwaha-1225
Copy link
Contributor

Hello @chrisbobbe,

Could you please assign this issue to me? I'm a first-time contributor and have already completed the project setup. I've also resolved this issue.

Looking forward to your response!

Hii @chrisbobbe @gnprice,

Just following up! Here are the changes I’ve made so far:

I’ve completed the project setup and resolved the issue. Could you please assign this issue to me?
Looking forward to contributing!

  • Before

void showSuggestedActionDialog({
  required BuildContext context,
  required String title,
  required String message,
  required String? actionButtonText,
  required VoidCallback onActionButtonPress,
}) {
  final zulipLocalizations = ZulipLocalizations.of(context);
  showDialog<void>(
    context: context,
    builder: (BuildContext context) => AlertDialog(
      title: Text(title),
      content: SingleChildScrollView(child: Text(message)),
      actions: [
        TextButton(
          onPressed: () => Navigator.pop(context),
          child: _dialogActionText(zulipLocalizations.dialogCancel)),
        TextButton(
          onPressed: () {
            onActionButtonPress();
            Navigator.pop(context);
          },
          child: _dialogActionText(actionButtonText ?? zulipLocalizations.dialogContinue)),
      ]));
  • After:

void showSuggestedActionDialog({
  required BuildContext context,
  required String title,
  required String message,
  required String? actionButtonText,
  required VoidCallback onActionButtonPress,
  bool destructive = false,
}) {
  final zulipLocalizations = ZulipLocalizations.of(context);
  showDialog<void>(
    context: context,
    builder: (BuildContext context) {
      return Theme.of(context).platform == TargetPlatform.android
          ? AlertDialog(
            title: Text(title),
            content: SingleChildScrollView(child: Text(message)),
            actions: [
              TextButton(
                onPressed: () => Navigator.pop(context),
                child: _dialogActionText(zulipLocalizations.dialogCancel),
              ),
              TextButton(
                onPressed: () {
                  onActionButtonPress();
                  Navigator.pop(context);
                },
                child: _dialogActionText(
                  actionButtonText ?? zulipLocalizations.dialogContinue,
                ),
              ),
            ],
          )
          : CupertinoAlertDialog(
            title: Text(title),
            content: SingleChildScrollView(child: Text(message)),
            actions: [
              CupertinoDialogAction(
                onPressed: () => Navigator.pop(context),
                child: _dialogActionText(zulipLocalizations.dialogCancel),
              ),
              CupertinoDialogAction(
                onPressed: () {
                  onActionButtonPress();
                  Navigator.pop(context);
                },
                isDestructiveAction: destructive,
                child: _dialogActionText(
                  actionButtonText ?? zulipLocalizations.dialogContinue,
                ),
              ),
            ],
          );
    },
  );
}

@gnprice
Copy link
Member

gnprice commented Nov 20, 2024

It looks like the main change there is what's called for in #996. Let's wait for that issue to be completed; there's an open PR.

I recommend also taking a look at that PR, #1017. It demonstrates some differences in approach that are important for keeping the codebase easy to read and maintain.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-design Visual and UX design
Projects
Status: No status
Development

No branches or pull requests

3 participants