Skip to content
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

Normalize Card theme #151914

Merged
merged 2 commits into from
Jul 29, 2024
Merged

Conversation

QuncCccccc
Copy link
Contributor

@QuncCccccc QuncCccccc commented Jul 17, 2024

This PR is to make preparations to make CardTheme conform to Flutter's conventions for component themes:

  • Added a CardThemeData class which defines overrides for the defaults for Card properties.
  • Added 2 CardTheme constructor parameters: CardThemeData? data and Widget? child. This is now the preferred way to configure a CardTheme:
CardTheme(
  data: CardThemeData(color: xxx, elevation: xxx, ...),
  child: Card(...)
)

These two properties are made nullable to not break existing apps which has customized ThemeData.cardTheme.

  • Changed the type of theme defaults from CardTheme to CardThemeData.

TODO:

  • Fix internal failures that may have breakages.
  • Change the type of ThemeData.cardTheme from CardTheme to CardThemeData. This may cause breaking changes, a migration guide will be created.

Addresses the "theme normalization" sub project within #91772

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].
  • I followed the [breaking change policy] and added [Data Driven Fixes] where supported.
  • All existing and new tests are passing.

@github-actions github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. c: tech-debt Technical debt, code quality, testing, etc. a: text input Entering text in a text field or keyboard related problems f: scrolling Viewports, list views, slivers, etc. and removed a: text input Entering text in a text field or keyboard related problems f: scrolling Viewports, list views, slivers, etc. c: tech-debt Technical debt, code quality, testing, etc. labels Jul 17, 2024
@QuncCccccc QuncCccccc force-pushed the normalize_card_theme branch from 1fabdb3 to dd7b041 Compare July 22, 2024 17:19
@QuncCccccc QuncCccccc force-pushed the normalize_card_theme branch 2 times, most recently from 9605643 to ef8f64b Compare July 25, 2024 19:07
@QuncCccccc QuncCccccc marked this pull request as ready for review July 25, 2024 20:01
Copy link
Member

@TahaTesser TahaTesser left a comment

Choose a reason for hiding this comment

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

Great update for consistent API. LGTM!

Copy link
Member

@guidezpl guidezpl left a comment

Choose a reason for hiding this comment

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

Nice. I wonder if you can also introduce a fix now, for replacing individual properties

CardTheme(clipBehavior: ...) ⇒ CardTheme(data: CardThemeData(clipBehavior: ...))


/// Overrides the default value for [Card.clipBehavior].
///
/// This property is obsolete: please use the [CardThemeData.clipBehavior]
Copy link
Member

Choose a reason for hiding this comment

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

here and below

Suggested change
/// This property is obsolete: please use the [CardThemeData.clipBehavior]
/// This property is obsolete and will be deprecated in a future release: please use the [CardThemeData.clipBehavior]

Copy link
Contributor

Choose a reason for hiding this comment

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

a fix

Dart fix can't help with this one unfortunately. We looked at it a bunch of ways, but since the class CardTheme itself will still exist undeprecated, we can't auto migrate it to CardThemeData.


/// Creates a copy of this object with the given fields replaced with the
/// new values.
CardTheme copyWith({
Copy link
Member

Choose a reason for hiding this comment

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

nit: add a mention that this will be deprecated

/// Linearly interpolate between two Card themes.
///
/// {@macro dart.ui.shadow.lerp}
static CardTheme lerp(CardTheme? a, CardTheme? b, double t) {
Copy link
Member

Choose a reason for hiding this comment

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

nit: add a note that this will be deprecated

@QuncCccccc QuncCccccc force-pushed the normalize_card_theme branch from 3986622 to f16e813 Compare July 29, 2024 00:33
@QuncCccccc QuncCccccc requested a review from guidezpl July 29, 2024 06:18
@QuncCccccc QuncCccccc added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 29, 2024
@auto-submit auto-submit bot merged commit 2e7fa83 into flutter:master Jul 29, 2024
132 checks passed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 29, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 29, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 31, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 31, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 31, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 31, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 31, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 31, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 1, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 1, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 1, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 1, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 1, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 2, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 2, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 2, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 2, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 2, 2024
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Aug 3, 2024
…7295)

Manual roll requested by stuartmorgan@google.com

flutter/flutter@9d5ede0...85960d2

2024-07-30 leroux_bruno@yahoo.fr Fix Shortcut label for CharacterActivator does not include modifiers (flutter/flutter#152233)
2024-07-30 rexios@rexios.dev [wiki] Remove text saying wiki edits do not get code reviewed (flutter/flutter#152530)
2024-07-30 engine-flutter-autoroll@skia.org Roll Flutter Engine from cb96743cc955 to c58d87d62c20 (1 revision) (flutter/flutter#152527)
2024-07-30 gspencergoog@users.noreply.github.com Add `find.backButton` finder and `StandardComponentType` enum to find components in tests. (flutter/flutter#149349)
2024-07-30 engine-flutter-autoroll@skia.org Roll Flutter Engine from 49324cd0b678 to cb96743cc955 (4 revisions) (flutter/flutter#152524)
2024-07-30 jonahwilliams@google.com [devicelab] remove Skia specific and unused devicelab metrics. (flutter/flutter#152523)
2024-07-30 kalathiyadimil@gmail.com Add Dimil Kalathiya to authors (flutter/flutter#152491)
2024-07-29 engine-flutter-autoroll@skia.org Roll Flutter Engine from 240fb460675d to 49324cd0b678 (4 revisions) (flutter/flutter#152521)
2024-07-29 victorsanniay@gmail.com Stop CupertinoScrollbar's track from paging the scroll view on tap (flutter/flutter#152197)
2024-07-29 engine-flutter-autoroll@skia.org Roll Flutter Engine from 4ef0f0d988ec to 240fb460675d (1 revision) (flutter/flutter#152507)
2024-07-29 engine-flutter-autoroll@skia.org Roll Flutter Engine from a0219f7a429c to 4ef0f0d988ec (1 revision) (flutter/flutter#152505)
2024-07-29 jonahwilliams@google.com [flutter_tools] remove raster stats CLI option. (flutter/flutter#152501)
2024-07-29 zanderso@users.noreply.github.com Shift some mac Android tests to Mokey devices in staging (flutter/flutter#152499)
2024-07-29 jonahwilliams@google.com [devicelab] enable impeller in external texture test. (flutter/flutter#152502)
2024-07-29 engine-flutter-autoroll@skia.org Roll Flutter Engine from d1995f302c4d to a0219f7a429c (2 revisions) (flutter/flutter#152500)
2024-07-29 engine-flutter-autoroll@skia.org Roll Flutter Engine from d803bb16c8b0 to d1995f302c4d (4 revisions) (flutter/flutter#152496)
2024-07-29 36861262+QuncCccccc@users.noreply.github.com Normalize Card theme (flutter/flutter#151914)
2024-07-29 engine-flutter-autoroll@skia.org Roll Flutter Engine from 2db47a1f3d0d to d803bb16c8b0 (1 revision) (flutter/flutter#152489)
2024-07-29 engine-flutter-autoroll@skia.org Roll Packages from 3d358d9 to 247fb5f (8 revisions) (flutter/flutter#152488)
2024-07-29 reidbaker@google.com Update New-Android-version.md (flutter/flutter#152395)
2024-07-29 engine-flutter-autoroll@skia.org Roll Flutter Engine from 510f19a7e629 to 2db47a1f3d0d (1 revision) (flutter/flutter#152485)
2024-07-29 engine-flutter-autoroll@skia.org Roll Flutter Engine from 87cbf33b9028 to 510f19a7e629 (1 revision) (flutter/flutter#152473)
2024-07-29 engine-flutter-autoroll@skia.org Roll Flutter Engine from 2cf3986cae52 to 87cbf33b9028 (2 revisions) (flutter/flutter#152469)
2024-07-29 32538273+ValentinVignal@users.noreply.github.com Add tests for deletable_chip_attributes.on_deleted.0.dart (flutter/flutter#152361)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC rmistry@google.com,stuartmorgan@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
TytaniumDev pushed a commit to TytaniumDev/flutter that referenced this pull request Aug 7, 2024
This PR is to make preparations to make `CardTheme` conform to Flutter's conventions for component themes:
* Added a `CardThemeData` class which defines overrides for the defaults for `Card` properties.
* Added 2 `CardTheme` constructor parameters: `CardThemeData? data` and `Widget? child`. This is now the preferred way to configure a `CardTheme`:
```dart
CardTheme(
  data: CardThemeData(color: xxx, elevation: xxx, ...),
  child: Card(...)
)
```

These two properties are made nullable to not break existing apps which has customized `ThemeData.cardTheme`.
* Changed the type of theme defaults from `CardTheme` to `CardThemeData`.

TODO: 
* Fix internal failures that may have breakages.
* Change the type of `ThemeData.cardTheme` from `CardTheme` to `CardThemeData`. This may cause breaking changes, a migration guide will be created.

Addresses the "theme normalization" sub project within flutter#91772
@QuncCccccc QuncCccccc mentioned this pull request Aug 12, 2024
9 tasks
Buchimi pushed a commit to Buchimi/flutter that referenced this pull request Sep 2, 2024
This PR is to make preparations to make `CardTheme` conform to Flutter's conventions for component themes:
* Added a `CardThemeData` class which defines overrides for the defaults for `Card` properties.
* Added 2 `CardTheme` constructor parameters: `CardThemeData? data` and `Widget? child`. This is now the preferred way to configure a `CardTheme`:
```dart
CardTheme(
  data: CardThemeData(color: xxx, elevation: xxx, ...),
  child: Card(...)
)
```

These two properties are made nullable to not break existing apps which has customized `ThemeData.cardTheme`.
* Changed the type of theme defaults from `CardTheme` to `CardThemeData`.

TODO: 
* Fix internal failures that may have breakages.
* Change the type of `ThemeData.cardTheme` from `CardTheme` to `CardThemeData`. This may cause breaking changes, a migration guide will be created.

Addresses the "theme normalization" sub project within flutter#91772
auto-submit bot pushed a commit that referenced this pull request Oct 4, 2024
Following #151914, this PR is to normalize `ThemeData.cardTheme`; change the `CardTheme cardTheme` property to `CardThemeData cardTheme` in `ThemeData`. In `ThemeData()` and `ThemeData.copyWith()`, the `cardTheme` parameter type is changed to `Object?` to accept both `CardTheme` and `CardThemeData` so that we won't cause immediate breaking change and make sure rolling is smooth. Once all component themes are normalized, these `Object?` types should be changed to `xxxThemeData`.

There's no way to create a dart fix because we can't add a "@deprecated" label for `CardTheme` because `CardTheme` is a new InheritedWidget subclass now.

Addresses the "theme normalization" sub project within #91772
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 11, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit Merge PR when tree becomes green via auto submit App f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants