Update `MaterialBanner` to support Material 3 #105957

TahaTesser · 2022-06-14T13:57:34Z

minimal code sample

import 'package:flutter/material.dart';

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

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

  final bool dark;
  final bool useMaterial3;

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      debugShowCheckedModeBanner: false,
      themeMode: dark ? ThemeMode.dark : ThemeMode.light,
      theme: ThemeData(
        useMaterial3: useMaterial3,
        brightness: Brightness.light,
        colorSchemeSeed: const Color(0xff6750a4),
      ),
      darkTheme: ThemeData(
        useMaterial3: useMaterial3,
        brightness: Brightness.dark,
        colorSchemeSeed: const Color(0xff6750a4),
      ),
      home: const MaterialBannerExample(),
    );
  }
}

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

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(
        title: const Text('MaterialBanner Example'),
      ),
      body: MaterialBanner(
        content: const Text('Hello, I am a Material Banner'),
        leading: const Icon(Icons.agriculture_outlined),
        actions: <Widget>[
          TextButton(
            onPressed: () {},
            child: const Text('OPEN'),
          ),
          TextButton(
            onPressed: () {},
            child: const Text('DISMISS'),
          ),
        ],
      ),
    );
  }
}

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.

TahaTesser · 2022-07-18T10:19:20Z

Just resolved template conflicts.

packages/flutter/lib/src/material/banner.dart

hannah-hyj · 2022-08-05T17:05:58Z

packages/flutter/lib/src/material/banner.dart

I see there are more properties (like actions, containerHeight, divider) in the token file. Should we add them in themeData too?

Apologies for the delay.

actions parameter takes a list of widgets so the styling depends on the widget provided.

containerHeight aka min height isn't customizable currently, there are container height tokens for desktop and mobile. I would prefer to work in a separate PR after Banner is available on https://m3.material.io/.

Added divider color and other existing missing properties tests.

bleroux

LGTM with small nit

packages/flutter/lib/src/material/banner.dart

hannah-hyj

LGTM

guidezpl · 2022-09-05T15:00:03Z

packages/flutter/lib/src/material/banner.dart

+  Color? get backgroundColor => _theme.colorScheme.surface;
+
+  @override
+  Color? get dividerColor => Theme.of(context).colorScheme.surfaceVariant;


This broke golden tests for flutter gallery, is this the correct value?

Yes, based on token

flutter/dev/tools/gen_defaults/data/banner.json

Line 22 in bce9163

"md.comp.banner.divider.color": "surfaceVariant",

Perhaps the golden test is broken this color is added to the M2 template in this PR

Which uses Theme.dividerColor

flutter/packages/flutter/lib/src/material/divider.dart

Line 137 in bce9163

?? (context != null ? (DividerTheme.of(context).color ?? Theme.of(context).dividerColor) : null);

I can revert this PR or remove the divider color from the M2 template.

Okay, thanks. Since this is not breaking per the breaking change policy, and is effectively a fix so let's hold off unless there's a need to revert due to Google-internal usages.

Thanks for the update!

@TahaTesser Actually the "banner.json" token file is only for M3. I think you should remove the divider color from the M2 template.

@TahaTesser Hi Taha, We are reverting this PR in #111288 because it caused some scuba breakages to google3. Please create another PR for this, thanks!

Thank you @Hangyujin! My bad I should have been careful with the M2 template.

…r#105957)

* 5bb612c Roll Flutter Engine from 14eb83a7d0bd to a8e97892ce25 (1 revision) (flutter/flutter#110938) * 798ce22 Roll Flutter Engine from a8e97892ce25 to 0a2f56cd025f (1 revision) (flutter/flutter#110940) * 5de91d9 Roll Flutter Engine from 0a2f56cd025f to b040ed90175b (1 revision) (flutter/flutter#110954) * 202f577 Roll Flutter Engine from b040ed90175b to b94120dfe9c6 (1 revision) (flutter/flutter#110960) * d8fdb83 Update `MaterialBanner` to support Material 3 (flutter/flutter#105957) * fbba194 Migrate `ListTile` disabled icon color to Material 3 (flutter/flutter#102078) * f625359 Roll Flutter Engine from b94120dfe9c6 to 5a2861e499d2 (1 revision) (flutter/flutter#110972) * eb154d5 Roll Flutter Engine from 5a2861e499d2 to 3ff25eb9081c (1 revision) (flutter/flutter#110974) * 0144110 Roll Flutter Engine from 3ff25eb9081c to 47c281f218db (1 revision) (flutter/flutter#110983) * b166390 Roll Flutter Engine from 47c281f218db to 25642a45a55e (1 revision) (flutter/flutter#110991) * bce9163 Roll Flutter Engine from 25642a45a55e to 7b3710878503 (1 revision) (flutter/flutter#110992) * 8cddac7 Roll Flutter Engine from 7b3710878503 to 3c0898c0dfed (1 revision) (flutter/flutter#110993) * 96345a4 Roll Flutter Engine from 3c0898c0dfed to ddc5bb330254 (1 revision) (flutter/flutter#110999) * 723b82e Feat: dSYM debug info for iOS & macOS builds (flutter/flutter#101586)

…r#105957)

hannah-hyj · 2022-09-09T17:37:40Z

packages/flutter/lib/src/material/banner.dart

+  Color? get dividerColor => Theme.of(context).colorScheme.surfaceVariant;
+
+  @override
+  TextStyle? get contentTextStyle => _theme.textTheme.bodyText2;


I think it should also be "theme.textTheme.bodyMedium" in M2

Yep, I was gonna make this since it just points to bodyMedium anyways.

This reverts commit d8fdb83.

* 5bb612c Roll Flutter Engine from 14eb83a7d0bd to a8e97892ce25 (1 revision) (flutter/flutter#110938) * 798ce22 Roll Flutter Engine from a8e97892ce25 to 0a2f56cd025f (1 revision) (flutter/flutter#110940) * 5de91d9 Roll Flutter Engine from 0a2f56cd025f to b040ed90175b (1 revision) (flutter/flutter#110954) * 202f577 Roll Flutter Engine from b040ed90175b to b94120dfe9c6 (1 revision) (flutter/flutter#110960) * d8fdb83 Update `MaterialBanner` to support Material 3 (flutter/flutter#105957) * fbba194 Migrate `ListTile` disabled icon color to Material 3 (flutter/flutter#102078) * f625359 Roll Flutter Engine from b94120dfe9c6 to 5a2861e499d2 (1 revision) (flutter/flutter#110972) * eb154d5 Roll Flutter Engine from 5a2861e499d2 to 3ff25eb9081c (1 revision) (flutter/flutter#110974) * 0144110 Roll Flutter Engine from 3ff25eb9081c to 47c281f218db (1 revision) (flutter/flutter#110983) * b166390 Roll Flutter Engine from 47c281f218db to 25642a45a55e (1 revision) (flutter/flutter#110991) * bce9163 Roll Flutter Engine from 25642a45a55e to 7b3710878503 (1 revision) (flutter/flutter#110992) * 8cddac7 Roll Flutter Engine from 7b3710878503 to 3c0898c0dfed (1 revision) (flutter/flutter#110993) * 96345a4 Roll Flutter Engine from 3c0898c0dfed to ddc5bb330254 (1 revision) (flutter/flutter#110999) * 723b82e Feat: dSYM debug info for iOS & macOS builds (flutter/flutter#101586)

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 Jun 14, 2022

TahaTesser force-pushed the m3_banner branch from a3d73d8 to 820a6b5 Compare June 15, 2022 06:23

This comment was marked as resolved.

Sign in to view

TahaTesser requested a review from darrenaustin June 16, 2022 10:34

TahaTesser force-pushed the m3_banner branch 2 times, most recently from d96ae2a to c3055c7 Compare July 18, 2022 08:28

HansMuller requested a review from hannah-hyj August 1, 2022 17:42

hannah-hyj reviewed Aug 5, 2022

View reviewed changes

TahaTesser force-pushed the m3_banner branch 2 times, most recently from f0c16eb to 529436d Compare August 26, 2022 14:49

bleroux approved these changes Aug 26, 2022

View reviewed changes

packages/flutter/lib/src/material/banner.dart Outdated Show resolved Hide resolved

packages/flutter/lib/src/material/banner.dart Outdated Show resolved Hide resolved

TahaTesser requested review from hannah-hyj and removed request for darrenaustin August 26, 2022 15:38

hannah-hyj approved these changes Aug 29, 2022

View reviewed changes

TahaTesser mentioned this pull request Sep 5, 2022

corrected the chip file names in gen_defaults.dart #110910

Closed

8 tasks

TahaTesser added 3 commits September 5, 2022 10:01

Update MaterialBanner to support Material 3

adc32ba

format fixes

35b0c6a

resolve conflict

26c1561

TahaTesser force-pushed the m3_banner branch from 4d25b43 to 26c1561 Compare September 5, 2022 07:40

fix imports

b63c259

TahaTesser added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 5, 2022

auto-submit bot merged commit d8fdb83 into flutter:master Sep 5, 2022

TahaTesser deleted the m3_banner branch September 5, 2022 09:00

guidezpl reviewed Sep 5, 2022

View reviewed changes

engine-flutter-autoroll mentioned this pull request Sep 5, 2022

Roll Flutter from 759d0e1b20c1 to bce91637c6db (63 revisions) flutter/packages#2565

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Sep 5, 2022

d8fdb83 Update MaterialBanner to support Material 3 (flutter/flutte…

671bba3

…r#105957)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 5, 2022

d8fdb83 Update MaterialBanner to support Material 3 (flutter/flutte…

4035fed

…r#105957)

This was referenced Sep 5, 2022

Roll Flutter from 0e7257e698db to 96345a4bb94c (13 revisions) flutter/plugins#6364

Closed

Roll Flutter from 759d0e1b20c1 to 96345a4bb94c (65 revisions) flutter/packages#2567

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Sep 5, 2022

d8fdb83 Update MaterialBanner to support Material 3 (flutter/flutte…

ee85b92

…r#105957)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 5, 2022

d8fdb83 Update MaterialBanner to support Material 3 (flutter/flutte…

72bfa67

…r#105957)

This was referenced Sep 5, 2022

Roll Flutter from 0e7257e698db to 723b82e4f0f3 (14 revisions) flutter/plugins#6365

Merged

Roll Flutter from 759d0e1b20c1 to 723b82e4f0f3 (66 revisions) flutter/packages#2568

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Sep 5, 2022

d8fdb83 Update MaterialBanner to support Material 3 (flutter/flutte…

4fdef91

…r#105957)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 5, 2022

d8fdb83 Update MaterialBanner to support Material 3 (flutter/flutte…

898b26c

…r#105957)

engine-flutter-autoroll mentioned this pull request Sep 6, 2022

Roll Flutter from 759d0e1b20c1 to 96b72232bd76 (72 revisions) flutter/packages#2570

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 6, 2022

d8fdb83 Update MaterialBanner to support Material 3 (flutter/flutte…

5fd3fdb

…r#105957)

engine-flutter-autoroll mentioned this pull request Sep 6, 2022

Roll Flutter from 759d0e1b20c1 to 9dbc09de1fd7 (74 revisions) flutter/packages#2571

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 6, 2022

d8fdb83 Update MaterialBanner to support Material 3 (flutter/flutte…

ef9e862

…r#105957)

engine-flutter-autoroll mentioned this pull request Sep 6, 2022

Roll Flutter from 759d0e1b20c1 to 27896a7b495a (76 revisions) flutter/packages#2572

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 6, 2022

d8fdb83 Update MaterialBanner to support Material 3 (flutter/flutte…

fc53e80

…r#105957)

engine-flutter-autoroll mentioned this pull request Sep 6, 2022

Roll Flutter from 759d0e1b20c1 to c0354999a73f (77 revisions) flutter/packages#2573

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 6, 2022

d8fdb83 Update MaterialBanner to support Material 3 (flutter/flutte…

3a0adb3

…r#105957)

engine-flutter-autoroll mentioned this pull request Sep 6, 2022

Roll Flutter from 759d0e1b20c1 to 8da0432094c2 (78 revisions) flutter/packages#2574

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 6, 2022

d8fdb83 Update MaterialBanner to support Material 3 (flutter/flutte…

bd0899d

…r#105957)

engine-flutter-autoroll mentioned this pull request Sep 7, 2022

Roll Flutter from 759d0e1b20c1 to 58ec4839159e (93 revisions) flutter/packages#2579

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 7, 2022

d8fdb83 Update MaterialBanner to support Material 3 (flutter/flutte…

8e2c732

…r#105957)

guidezpl mentioned this pull request Sep 9, 2022

Maintenance flutter/gallery#773

Merged

hannah-hyj reviewed Sep 9, 2022

View reviewed changes

CaseyHillers added a commit that referenced this pull request Sep 9, 2022

Revert "Update MaterialBanner to support Material 3 (#105957)"

edbc8b6

This reverts commit d8fdb83.

CaseyHillers mentioned this pull request Sep 9, 2022

Revert "Update MaterialBanner to support Material 3" #111288

Merged

TahaTesser mentioned this pull request Sep 9, 2022

Reland "Update MaterialBanner to support Material 3" #111299

Merged

8 tasks

Update MaterialBanner to support Material 3 #105957

Update MaterialBanner to support Material 3 #105957

Uh oh!

Conversation

TahaTesser commented Jun 14, 2022

Pre-launch Checklist

Uh oh!

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

TahaTesser commented Jul 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TahaTesser Aug 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bleroux left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

hannah-hyj left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Update `MaterialBanner` to support Material 3 #105957

Update `MaterialBanner` to support Material 3 #105957

TahaTesser commented Jul 18, 2022 •

edited

Loading

TahaTesser Aug 26, 2022 •

edited

Loading