Skip to content

Conversation

@TahaTesser
Copy link
Member

@TahaTesser TahaTesser commented Feb 22, 2022

Part of: #91605

Updated the Dialog/AlertDialog widget with support for Material Design 3.

Fixes: #99025

minimal code sample
import 'package:flutter/material.dart';

const seed = Color(0xFF6750A4);

const lightColorScheme = ColorScheme(
	brightness: Brightness.light,
	primary : Color(0xFF6750A4),
	onPrimary : Color(0xFFFFFFFF),
	primaryContainer : Color(0xFFEADDFF),
	onPrimaryContainer : Color(0xFF21005D),
	secondary : Color(0xFF625B71),
	onSecondary : Color(0xFFFFFFFF),
	secondaryContainer : Color(0xFFE8DEF8),
	onSecondaryContainer : Color(0xFF1D192B),
	tertiary : Color(0xFF7D5260),
	onTertiary : Color(0xFFFFFFFF),
	tertiaryContainer : Color(0xFFFFD8E4),
	onTertiaryContainer : Color(0xFF31111D),
	error : Color(0xFFB3261E),
	errorContainer : Color(0xFFF9DEDC),
	onError : Color(0xFFFFFFFF),
	onErrorContainer : Color(0xFF410E0B),
	background : Color(0xFFFFFBFE),
	onBackground : Color(0xFF1C1B1F),
	surface : Color(0xFFFFFBFE),
	onSurface : Color(0xFF1C1B1F),
	surfaceVariant : Color(0xFFE7E0EC),
	onSurfaceVariant : Color(0xFF49454F),
	outline : Color(0xFF79747E),
	onInverseSurface : Color(0xFFF4EFF4),
	inverseSurface : Color(0xFF313033),
	inversePrimary : Color(0xFFD0BCFF),
	shadow : Color(0xFF000000),
);

const darkColorScheme = ColorScheme(
	brightness: Brightness.dark,
	primary : Color(0xFFD0BCFF),
	onPrimary : Color(0xFF381E72),
	primaryContainer : Color(0xFF4F378B),
	onPrimaryContainer : Color(0xFFEADDFF),
	secondary : Color(0xFFCCC2DC),
	onSecondary : Color(0xFF332D41),
	secondaryContainer : Color(0xFF4A4458),
	onSecondaryContainer : Color(0xFFE8DEF8),
	tertiary : Color(0xFFEFB8C8),
	onTertiary : Color(0xFF492532),
	tertiaryContainer : Color(0xFF633B48),
	onTertiaryContainer : Color(0xFFFFD8E4),
	error : Color(0xFFF2B8B5),
	errorContainer : Color(0xFF8C1D18),
	onError : Color(0xFF601410),
	onErrorContainer : Color(0xFFF9DEDC),
	background : Color(0xFF1C1B1F),
	onBackground : Color(0xFFE6E1E5),
	surface : Color(0xFF1C1B1F),
	onSurface : Color(0xFFE6E1E5),
	surfaceVariant : Color(0xFF49454F),
	onSurfaceVariant : Color(0xFFCAC4D0),
	outline : Color(0xFF938F99),
	onInverseSurface : Color(0xFF1C1B1F),
	inverseSurface : Color(0xFFE6E1E5),
	inversePrimary : Color(0xFF6750A4),
	shadow : Color(0xFF000000),
);

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

class MyApp extends StatelessWidget {
  const MyApp({Key? key, this.dark = false, this.useMaterial3 = true})
      : super(key: key);

  final bool dark;
  final bool useMaterial3;

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      debugShowCheckedModeBanner: false,
      title: 'Material App',
      themeMode: dark ? ThemeMode.dark : ThemeMode.light,
      theme: ThemeData.light().copyWith(
        useMaterial3: useMaterial3,
        colorScheme:
            useMaterial3 ? lightColorScheme : const ColorScheme.light(),
      ),
      darkTheme: ThemeData.dark().copyWith(
        useMaterial3: useMaterial3,
        colorScheme: useMaterial3 ? darkColorScheme : const ColorScheme.dark(),
      ),
      home: const Sample(),
    );
  }
}

class Sample extends StatelessWidget {
  const Sample({Key? key}) : super(key: key);

  Future<void> _showAlertDialog(BuildContext context) async {
    return showDialog<void>(
      context: context,
      barrierDismissible: false, // user must tap button!
      builder: (BuildContext context) {
        return AlertDialog(
          title: const Text('Basic dialog title'),
          content: const Text(
              'A dialog is a type of modal window that\nappears in front of app content to\nprovide critical information, or prompt\nfor a decision to be made.'),
          actions: <Widget>[
            TextButton(
              style: TextButton.styleFrom(
                textStyle: Theme.of(context).textTheme.labelLarge,
              ),
              child: const Text('Enabled'),
              onPressed: () {
                Navigator.of(context).pop();
              },
            ),
            TextButton(
              style: TextButton.styleFrom(
                textStyle: Theme.of(context).textTheme.labelLarge,
              ),
              child: const Text('Enabled'),
              onPressed: () {
                Navigator.of(context).pop();
              },
            ),
          ],
        );
      },
    );
  }

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(
        title: const Text(' Sample'),
      ),
      body: Center(
        child: FloatingActionButton.extended(
          onPressed: () async => _showAlertDialog(context),
          label: const Text('M3 Alert Dialog'),
        ),
      ),
    );
  }
}

In order to use the Dialog/AlertDialog with the new Material 3 defaults, turn on the useMaterial3 flag in the ThemeData:

  return MaterialApp(
    theme: ThemeData.light().copyWith(useMaterial3: true),
    // ...
  );

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

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

@flutter-dashboard flutter-dashboard bot added f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. c: contributor-productivity Team-specific productivity, code health, technical debt. labels Feb 22, 2022
@bernaferrari
Copy link
Contributor

This is fairly small, but..
image

@LasseRosenow
Copy link
Contributor

LasseRosenow commented Feb 27, 2022

@bernaferrari

I think this is because the font is somewhat different. The fact that the text box seem to line up with the text of the button is just coincidence.

See:
image

@bernaferrari
Copy link
Contributor

Is that from material 3? Yeah, fair enough. I think they used the buttons without inner padding in the image.

@LasseRosenow
Copy link
Contributor

Is that from material 3? Yeah, fair enough. I think they used the buttons without inner padding in the image.

Yes its from here: https://m3.material.io/components/dialogs/specs

@LasseRosenow
Copy link
Contributor

Btw. this happens when you put the spec version and the flutter version on top of each other:

image

So there are some things off:

  1. The font does not match
  2. The flutter version is a bit higher for some reason.

@bernaferrari
Copy link
Contributor

Thanks. There was a different issue (which I believe they have fixed since then 🎉), but I'm really happy with the current outcome.

@TahaTesser
Copy link
Member Author

TahaTesser commented Feb 28, 2022

@bernaferrari @LasseRosenow
Thanks for the comments, I should've mentioned in the description dialog currently doesn't match pixel by pixel.
I wanted to get an initial review before modifying a lot of existing code to match M3.

I've created a dialog overlay example https://github.com/TahaTesser/M3_Dialog_Overlay example (based on https://github.com/nt4f04uNd/tabs_overlay)

(top dialog is native)

Preview Preview

I will look into changes needed to match the M3 design.

cc: @darrenaustin
Instead of modifying dialog size, padding, margin, etc in the existing dialog, I am considering creating separate the would-be presented based on the m3 theme condition.
What are your thoughts?

@bernaferrari
Copy link
Contributor

[I know this is radical, but..] can't you make the Dialog work like buttons? Something like
Dialog(style: DialogThemeDeprecated()) and then we could have style: DialogThemeM3() which eventually would become the default.

I don't know what are the plans for Dialog theme, if they refactored already or not. But maybe, MAYBE, that could also be done.

Copy link
Contributor

@darrenaustin darrenaustin left a comment

Choose a reason for hiding this comment

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

@TahaTesser thanks for taking the initiative and tackling this migration.

Looks good, with a few suggestions and comments below.

You will probably also want to merge with the latest from master as I updated the token JSON files in #99292 earlier today.

Comment on lines 1155 to 1156
Copy link
Contributor

Choose a reason for hiding this comment

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

For properties that don't need a computation (like looking up a color from the ColorScheme), you can just include them as a parameter to the super class in the constructor. The main reason we want to override some of the properties with getter methods is so we aren't computing something until it actually needed. In the case of constants we don't need to do this.

Comment on lines 1161 to 1165
Copy link
Contributor

Choose a reason for hiding this comment

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

Same with these constant properties, just make them parameters to the super class in the constructor.

@darrenaustin
Copy link
Contributor

cc: @darrenaustin Instead of modifying dialog size, padding, margin, etc in the existing dialog, I am considering creating separate the would-be presented based on the m3 theme condition. What are your thoughts?

Not sure I understand what you are asking here. The current plan for migration is to use the component theme data classes to encapsulate as much of the token information as we can. This way if the code just depends on a FooThemeData for defaults it is easy to switch out a M2 theme data for a M3 theme data (as you have done with the DialogTheme).

Copy link
Contributor

@darrenaustin darrenaustin left a comment

Choose a reason for hiding this comment

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

Looks good. Just have a comment below that should be updated with a TODO.

Also, can you add Dialog and AlertDialog to the list of components affected by the useMaterial3 flag in doc comment for ThemeData.useMaterial3?

Otherwise, I think we are good to go. Thx!

@TahaTesser TahaTesser requested a review from darrenaustin March 2, 2022 22:13
Copy link
Contributor

@darrenaustin darrenaustin left a comment

Choose a reason for hiding this comment

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

flutter-lgtm

Thx!

@fluttergithubbot
Copy link
Contributor

This pull request is not suitable for automatic merging in its current state.

  • The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

Update template

Kick tests

refactor defaults
Kick tests

Kick tests
@TahaTesser
Copy link
Member Author

TahaTesser commented Mar 4, 2022

Rebasing one more time to update any infra changes in the updated master branch so tests can pass

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 11, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 11, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 11, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 11, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 11, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 11, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 11, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 11, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 12, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 14, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 14, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 14, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 14, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 14, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 14, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 14, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 14, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 14, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 14, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 14, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 14, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 14, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 14, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 14, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 15, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 15, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 15, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 15, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 15, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c: contributor-productivity Team-specific productivity, code health, technical debt. f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.

Projects

Status: Done (PR merged)

Development

Successfully merging this pull request may close these issues.

Update dialogs to support Material 3

5 participants