[Framework/Tool] Decouple preview theme imports#188176
Conversation
Decouple `PreviewThemeData` from the `material` and `cupertino` design systems in `package:flutter/widget_previews.dart` to prepare for their extraction into dedicated packages. Previously, `PreviewThemeData` directly referenced `ThemeData` and `CupertinoThemeData`, creating a circular dependency if those packages were decoupled from `package:flutter`. Changes: - Refactored `PreviewThemeData` into an `abstract base class` with an `apply` method in `packages/flutter/lib/src/widget_previews/widget_previews.dart`. - Added `MultiPreviewThemeData` to support applying multiple themes sequentially. - Created `MaterialPreviewThemeData` and `CupertinoPreviewThemeData` in their respective packages to implement the theme application logic using `Theme` and `CupertinoTheme` widgets. - Updated the widget preview rendering scaffold template and integration tests to use the new `apply` API.
|
I've marked this PR as not ready to port to flutter/packages yet. The changes not in the material or cupertino libraries will remain here after. |
|
Advice for PR splitting: Since this touches material/cupertino along with other parts of flutter/flutter, it will need to be split up. If the material/cupertino changes are dependent on the other non-material/cupertino changes in this PR, then the material/cupertino changes will need to wait for them to roll to stable before they can be ported over to material_ui/cupertino_ui in flutter/packages (which uses the stable branch).
|
Remove `MaterialPreviewThemeData` and `CupertinoPreviewThemeData` from the framework in `package:flutter`. To address review comments and allow splitting the PR (since these classes will live in the decoupled `material_ui` and `cupertino_ui` packages in `flutter/packages` once ready): - Deleted `packages/flutter/lib/src/material/preview_theme.dart` and `packages/flutter/lib/src/cupertino/preview_theme.dart`. - Removed their exports from `packages/flutter/lib/material.dart` and `packages/flutter/lib/cupertino.dart`. - Defined local test-specific dummy implementations of `PreviewThemeData` in `theming_test.dart` and the mock test project in `basic_project_exhaustive_previews.dart`.
|
I've updated the PR to address the feedback. Specifically:
All tests (integration and hermetic) and static analysis pass successfully. |
There was a problem hiding this comment.
Code Review
This pull request refactors PreviewThemeData into an abstract base class with an apply method, replacing the previous hardcoded Material and Cupertino theme properties, and introduces MultiPreviewThemeData to chain multiple themes. Corresponding updates are made to the rendering scaffold and tests. Feedback suggests reversing the theme application order in MultiPreviewThemeData so that the first theme in the list is applied as the outermost wrapper, aligning with intuitive nesting expectations.
| /// A [PreviewThemeData] that applies multiple themes in sequence. | ||
| final class MultiPreviewThemeData extends PreviewThemeData { | ||
| /// Creates a [MultiPreviewThemeData] that applies [themes] in sequence. | ||
| const MultiPreviewThemeData(this.themes); | ||
|
|
||
| /// The Cupertino [CupertinoThemeData] to apply when dark mode is enabled. | ||
| final CupertinoThemeData? cupertinoDark; | ||
| /// The list of themes to apply. | ||
| final List<PreviewThemeData> themes; | ||
|
|
||
| /// Returns the pair of [ThemeData] and [CupertinoThemeData] corresponding to | ||
| /// the value of [brightness]. | ||
| (ThemeData?, CupertinoThemeData?) themeForBrightness(Brightness brightness) { | ||
| if (brightness == Brightness.light) { | ||
| return (materialLight, cupertinoLight); | ||
| @override | ||
| Widget apply(BuildContext context, Widget child) { | ||
| var result = child; | ||
| for (final PreviewThemeData theme in themes) { | ||
| result = theme.apply(context, result); | ||
| } | ||
| return (materialDark, cupertinoDark); | ||
| return result; | ||
| } |
There was a problem hiding this comment.
The current implementation of MultiPreviewThemeData.apply applies themes in the order they are defined, which means the first theme in the list becomes the innermost wrapper (closest to the child) and the last theme becomes the outermost wrapper. This is counter-intuitive, as users typically expect the first element in a list of wrappers to be the outermost parent.
We should reverse the iteration order using themes.reversed so that the first theme in the list is the outermost wrapper, and explicitly document this behavior in the class and constructor documentation.
| /// A [PreviewThemeData] that applies multiple themes in sequence. | |
| final class MultiPreviewThemeData extends PreviewThemeData { | |
| /// Creates a [MultiPreviewThemeData] that applies [themes] in sequence. | |
| const MultiPreviewThemeData(this.themes); | |
| /// The Cupertino [CupertinoThemeData] to apply when dark mode is enabled. | |
| final CupertinoThemeData? cupertinoDark; | |
| /// The list of themes to apply. | |
| final List<PreviewThemeData> themes; | |
| /// Returns the pair of [ThemeData] and [CupertinoThemeData] corresponding to | |
| /// the value of [brightness]. | |
| (ThemeData?, CupertinoThemeData?) themeForBrightness(Brightness brightness) { | |
| if (brightness == Brightness.light) { | |
| return (materialLight, cupertinoLight); | |
| @override | |
| Widget apply(BuildContext context, Widget child) { | |
| var result = child; | |
| for (final PreviewThemeData theme in themes) { | |
| result = theme.apply(context, result); | |
| } | |
| return (materialDark, cupertinoDark); | |
| return result; | |
| } | |
| /// A [PreviewThemeData] that applies multiple themes in sequence, where the | |
| /// first theme in the list is the outermost wrapper. | |
| final class MultiPreviewThemeData extends PreviewThemeData { | |
| /// Creates a [MultiPreviewThemeData] that applies [themes] in sequence, | |
| /// nesting them from first (outermost) to last (innermost). | |
| const MultiPreviewThemeData(this.themes); | |
| /// The list of themes to apply. | |
| final List<PreviewThemeData> themes; | |
| @override | |
| Widget apply(BuildContext context, Widget child) { | |
| var result = child; | |
| for (final PreviewThemeData theme in themes.reversed) { | |
| result = theme.apply(context, result); | |
| } | |
| return result; | |
| } | |
| } |
References
- Documentation should be useful and explain the why and the how, and code should be optimized for readability and intuitive behavior. (link)
Ensure the first theme in the list becomes the outermost wrapper (closest to the parent) and the last theme becomes the innermost wrapper (closest to the child). Update documentation comments to clarify this behavior.
…#12081) Manual roll Flutter from 0c80830e465b to ca9f874f5284 (119 revisions) Manual roll requested by [email protected] flutter/flutter@0c80830...ca9f874 2026-06-30 [email protected] [tool] Don't require a Flutter compile task when staging jniLibs (flutter/flutter#188805) 2026-06-30 [email protected] [Impeller] Fix potential overflow when allocating buffers at the next power of two size (flutter/flutter#188742) 2026-06-30 [email protected] [Framework/Tool] Decouple preview theme imports (flutter/flutter#188176) 2026-06-30 [email protected] Add import of `dart:_js_interop_wasm` in sdk rewriter tool (flutter/flutter#188620) 2026-06-30 [email protected] Detach LLDB and print stack trace on process stop (flutter/flutter#188576) 2026-06-30 [email protected] [Linux] Fix FlCompositorOpenGL.pixels comment (flutter/flutter#188754) 2026-06-30 [email protected] [flutter_tools] Fix crash in flutter create when pubspec.yaml is empty (flutter/flutter#188385) 2026-06-30 [email protected] Roll Packages from 656ccaa to 274ed3e (23 revisions) (flutter/flutter#188792) 2026-06-30 [email protected] In AndroidImageGenerator, check that the destination pixel buffer has sufficient capacity for the decoded data (flutter/flutter#188752) 2026-06-30 [email protected] Roll pub packages (flutter/flutter#188773) 2026-06-30 [email protected] [Impeller] Add a flat VertexAttributeFormat for vertex inputs (flutter/flutter#188684) 2026-06-30 [email protected] Roll pub packages (flutter/flutter#188764) 2026-06-30 [email protected] Roll Dart SDK from 0cb483880b6b to e1bdb9ce3327 (2 revisions) (flutter/flutter#188763) 2026-06-29 [email protected] Handle 'no permissions' adb device state (flutter/flutter#187248) 2026-06-29 [email protected] [windows]: adjusts uniform buffers to hit hlsl optimization (flutter/flutter#188538) 2026-06-29 [email protected] Roll Skia from bfb7860cb9c7 to 71947c4110b0 (9 revisions) (flutter/flutter#188747) 2026-06-29 [email protected] ci: extract wait-for-engine-build logic into a reusable composite action (flutter/flutter#188748) 2026-06-29 [email protected] Provide guided migration logs when iOS app crashes on simulator (flutter/flutter#188736) 2026-06-29 [email protected] Revert "[flutter_tools] Track asset transformer dependencies for hot reload" (flutter/flutter#188751) 2026-06-29 [email protected] Migrate ABI splits to new AGP dsl (flutter/flutter#188369) 2026-06-29 [email protected] Add Impeller+OpenGLES startup benchmark for mokey (flutter/flutter#188495) 2026-06-29 [email protected] Increase macOS minimum supported version from 10.15 to 12 to support Xcode 27 (flutter/flutter#188520) 2026-06-29 [email protected] Moves test ownership validation to flutter/flutter (flutter/flutter#188655) 2026-06-29 [email protected] Add --flavor support for Windows desktop builds (flutter/flutter#187034) 2026-06-29 [email protected] Android_hardware_smoke_test: Enable pixel exact local file comparator to read goldens from flutter asset URI (flutter/flutter#188587) 2026-06-29 [email protected] [flutter_tools] Use DeviceHub.app for iOS simulator path on Xcode 27+ (flutter/flutter#187910) 2026-06-29 [email protected] Roll Skia from 111e7582d081 to bfb7860cb9c7 (2 revisions) (flutter/flutter#188731) 2026-06-29 [email protected] Use `revert` label instead of `revert_wf` (flutter/flutter#188639) 2026-06-29 [email protected] Properly await Dart Development Service shutdown with timeout (flutter/flutter#188387) 2026-06-29 [email protected] [flutter_tools] Track asset transformer dependencies for hot reload (flutter/flutter#187947) 2026-06-29 [email protected] [Tool] Tolerate malformed UTF-8 in process streaming decoders (flutter/flutter#188453) 2026-06-29 [email protected] [flutter_tools] Use new ddc modules in test (flutter/flutter#188240) 2026-06-29 [email protected] Roll Packages from c1f7d92 to 656ccaa (12 revisions) (flutter/flutter#188728) 2026-06-29 [email protected] Remove unused fields (flutter/flutter#188705) 2026-06-29 [email protected] Clear text input handler widget on view dispose (flutter/flutter#188701) 2026-06-29 [email protected] Free compositor in view renderer finalize to avoid use-after-free (flutter/flutter#188702) 2026-06-29 [email protected] [linux] Use GWeakRef in mock signal handler test helper (flutter/flutter#188700) 2026-06-29 [email protected] Fixing few related editing issues with LTR/RTL text (flutter/flutter#188503) 2026-06-29 [email protected] [Flutter GPU] Add Texture.fromImage to wrap a ui.Image texture (flutter/flutter#188605) 2026-06-29 [email protected] [Flutter GPU] Honor the enable argument in RenderPass.setDepthWriteEnable (flutter/flutter#188715) 2026-06-29 [email protected] Roll Skia from ba1942d8c3e1 to 111e7582d081 (1 revision) (flutter/flutter#188721) 2026-06-29 [email protected] Roll Skia from 587d8befe1ee to ba1942d8c3e1 (6 revisions) (flutter/flutter#188717) 2026-06-29 [email protected] Remove some refs to package:intl (flutter/flutter#188504) 2026-06-29 [email protected] [VPAT] Update a11y assessment app FAB example to announce value change when it's updated. (flutter/flutter#188466) ...
…flutter#12081) Manual roll Flutter from 0c80830e465b to ca9f874f5284 (119 revisions) Manual roll requested by [email protected] flutter/flutter@0c80830...ca9f874 2026-06-30 [email protected] [tool] Don't require a Flutter compile task when staging jniLibs (flutter/flutter#188805) 2026-06-30 [email protected] [Impeller] Fix potential overflow when allocating buffers at the next power of two size (flutter/flutter#188742) 2026-06-30 [email protected] [Framework/Tool] Decouple preview theme imports (flutter/flutter#188176) 2026-06-30 [email protected] Add import of `dart:_js_interop_wasm` in sdk rewriter tool (flutter/flutter#188620) 2026-06-30 [email protected] Detach LLDB and print stack trace on process stop (flutter/flutter#188576) 2026-06-30 [email protected] [Linux] Fix FlCompositorOpenGL.pixels comment (flutter/flutter#188754) 2026-06-30 [email protected] [flutter_tools] Fix crash in flutter create when pubspec.yaml is empty (flutter/flutter#188385) 2026-06-30 [email protected] Roll Packages from 656ccaa to 274ed3e (23 revisions) (flutter/flutter#188792) 2026-06-30 [email protected] In AndroidImageGenerator, check that the destination pixel buffer has sufficient capacity for the decoded data (flutter/flutter#188752) 2026-06-30 [email protected] Roll pub packages (flutter/flutter#188773) 2026-06-30 [email protected] [Impeller] Add a flat VertexAttributeFormat for vertex inputs (flutter/flutter#188684) 2026-06-30 [email protected] Roll pub packages (flutter/flutter#188764) 2026-06-30 [email protected] Roll Dart SDK from 0cb483880b6b to e1bdb9ce3327 (2 revisions) (flutter/flutter#188763) 2026-06-29 [email protected] Handle 'no permissions' adb device state (flutter/flutter#187248) 2026-06-29 [email protected] [windows]: adjusts uniform buffers to hit hlsl optimization (flutter/flutter#188538) 2026-06-29 [email protected] Roll Skia from bfb7860cb9c7 to 71947c4110b0 (9 revisions) (flutter/flutter#188747) 2026-06-29 [email protected] ci: extract wait-for-engine-build logic into a reusable composite action (flutter/flutter#188748) 2026-06-29 [email protected] Provide guided migration logs when iOS app crashes on simulator (flutter/flutter#188736) 2026-06-29 [email protected] Revert "[flutter_tools] Track asset transformer dependencies for hot reload" (flutter/flutter#188751) 2026-06-29 [email protected] Migrate ABI splits to new AGP dsl (flutter/flutter#188369) 2026-06-29 [email protected] Add Impeller+OpenGLES startup benchmark for mokey (flutter/flutter#188495) 2026-06-29 [email protected] Increase macOS minimum supported version from 10.15 to 12 to support Xcode 27 (flutter/flutter#188520) 2026-06-29 [email protected] Moves test ownership validation to flutter/flutter (flutter/flutter#188655) 2026-06-29 [email protected] Add --flavor support for Windows desktop builds (flutter/flutter#187034) 2026-06-29 [email protected] Android_hardware_smoke_test: Enable pixel exact local file comparator to read goldens from flutter asset URI (flutter/flutter#188587) 2026-06-29 [email protected] [flutter_tools] Use DeviceHub.app for iOS simulator path on Xcode 27+ (flutter/flutter#187910) 2026-06-29 [email protected] Roll Skia from 111e7582d081 to bfb7860cb9c7 (2 revisions) (flutter/flutter#188731) 2026-06-29 [email protected] Use `revert` label instead of `revert_wf` (flutter/flutter#188639) 2026-06-29 [email protected] Properly await Dart Development Service shutdown with timeout (flutter/flutter#188387) 2026-06-29 [email protected] [flutter_tools] Track asset transformer dependencies for hot reload (flutter/flutter#187947) 2026-06-29 [email protected] [Tool] Tolerate malformed UTF-8 in process streaming decoders (flutter/flutter#188453) 2026-06-29 [email protected] [flutter_tools] Use new ddc modules in test (flutter/flutter#188240) 2026-06-29 [email protected] Roll Packages from c1f7d92 to 656ccaa (12 revisions) (flutter/flutter#188728) 2026-06-29 [email protected] Remove unused fields (flutter/flutter#188705) 2026-06-29 [email protected] Clear text input handler widget on view dispose (flutter/flutter#188701) 2026-06-29 [email protected] Free compositor in view renderer finalize to avoid use-after-free (flutter/flutter#188702) 2026-06-29 [email protected] [linux] Use GWeakRef in mock signal handler test helper (flutter/flutter#188700) 2026-06-29 [email protected] Fixing few related editing issues with LTR/RTL text (flutter/flutter#188503) 2026-06-29 [email protected] [Flutter GPU] Add Texture.fromImage to wrap a ui.Image texture (flutter/flutter#188605) 2026-06-29 [email protected] [Flutter GPU] Honor the enable argument in RenderPass.setDepthWriteEnable (flutter/flutter#188715) 2026-06-29 [email protected] Roll Skia from ba1942d8c3e1 to 111e7582d081 (1 revision) (flutter/flutter#188721) 2026-06-29 [email protected] Roll Skia from 587d8befe1ee to ba1942d8c3e1 (6 revisions) (flutter/flutter#188717) 2026-06-29 [email protected] Remove some refs to package:intl (flutter/flutter#188504) 2026-06-29 [email protected] [VPAT] Update a11y assessment app FAB example to announce value change when it's updated. (flutter/flutter#188466) ...
Decouple
PreviewThemeDatafrom thematerialandcupertinodesign systems inpackage:flutter/widget_previews.dartto prepare for their extraction into dedicated packages.Previously,
PreviewThemeDatadirectly referencedThemeDataandCupertinoThemeData, creating a circular dependency if those packages were decoupled frompackage:flutter.Changes:
PreviewThemeDatainto anabstract base classwith anapplymethod inpackages/flutter/lib/src/widget_previews/widget_previews.dart.MultiPreviewThemeDatato support applying multiple themes sequentially.MaterialPreviewThemeDataandCupertinoPreviewThemeDatain their respective packages to implement the theme application logic usingThemeandCupertinoThemewidgets.applyAPI.