-
-
Notifications
You must be signed in to change notification settings - Fork 82
refactor: Add Stac Theme and ThemeData classes to stac_core #379
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
Conversation
|
Warning Rate limit exceeded@divyanshub024 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 4 minutes and 25 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (17)
WalkthroughA large refactor removes many Freezed-generated STAC theme data classes and generated JSON parts, adds parser extension files that map STAC types to Flutter theme types (parse methods/getters), updates several import paths/exports, and adds a .gitignore entry for Changes
(Note: the theme refactor cohort groups dozens of specific files; each removed Freezed Sequence Diagram(s)sequenceDiagram
autonumber
participant JSON as JSON input
participant STAC as StacModel (decoded)
participant Parser as Parser Extension (new)
participant Flutter as Flutter Theme
Note over JSON,STAC: Previously: JSON -> Freezed.fromJson -> FreezedModel -> parser extension? (removed)
JSON->>STAC: decode to StacModel (was via generated fromJson)
STAC->>Parser: call .parse(context) (new extension)
Parser->>Flutter: construct Flutter ThemeData (AppBar/Card/...)
Note right of Flutter: Theme objects used at runtime
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Attention points:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
packages/stac/lib/src/parsers/theme/stac_date_picker_theme_data_parser.dart (1)
33-34: Guard against null before castingdayShape.
dayShape?.parse(context)evaluates tonullwhendayShapeis absent, but the subsequentas OutlinedBorderforces a non-null cast and will throw at runtime. Wrap this in a null check and only build theWidgetStatePropertyAllwhendayShapeis present.Apply this diff:
- dayShape: - WidgetStatePropertyAll(dayShape?.parse(context) as OutlinedBorder), + dayShape: dayShape == null + ? null + : WidgetStatePropertyAll( + dayShape!.parse(context) as OutlinedBorder, + ),packages/stac/lib/src/parsers/theme/stac_navigation_drawer_theme_data_parser.dart (1)
22-24: UseMaterialStatePropertyAllfor Material-only fields
NavigationDrawerThemeData.labelTextStyleand.iconThemeare typed asMaterialStateProperty, so constructing them withWidgetStatePropertyAllfails to compile. Switch toMaterialStatePropertyAll(or returnnullwhen absent) to satisfy the API contract.- labelTextStyle: WidgetStatePropertyAll(labelTextStyle?.parse(context)), - iconTheme: WidgetStatePropertyAll(iconTheme?.parse(context)), + labelTextStyle: MaterialStatePropertyAll(labelTextStyle?.parse(context)), + iconTheme: MaterialStatePropertyAll(iconTheme?.parse(context)),packages/stac/lib/src/parsers/theme/stac_material_color_parser.dart (2)
10-21: Remove null assertions and add proper null handling for shade color conversions.The
toColor()method returnsColor?(nullable) and can return null when the color string is empty. Using null assertions (!) on all 10 shade conversions without fallback handling will cause runtime crashes if any shade value is empty or misconfigured. Replace with null coalescing (??) to provide fallback colors or restructure to validate shade values before the map is built.
23-27: Replace deprecated.valuewith.toARGB32()on line 25.Line 25 uses the deprecated
Color.valueproperty. Replace it withColor.toARGB32():return MaterialColor( // ignore: deprecated_member_use // Remove this ignore comment (primary.toColor(context))!.toARGB32(), color, );The
ignorecomment can also be removed once the deprecated member is no longer used.
🧹 Nitpick comments (4)
packages/stac/lib/src/parsers/theme/stac_floating_action_button_theme_data_parser.dart (1)
10-10: Consider renaming for consistency.The extension name
StacFloatingActionThemeParseromits "Button" while the extended class isStacFloatingActionButtonThemeData. For better clarity and consistency, consider renaming toStacFloatingActionButtonThemeParser.-extension StacFloatingActionThemeParser on StacFloatingActionButtonThemeData { +extension StacFloatingActionButtonThemeParser on StacFloatingActionButtonThemeData {packages/stac/lib/src/parsers/theme/stac_divider_theme_data_parser.dart (1)
5-18: Clean refactoring approach! Consider adding method documentation.The extension pattern is well-implemented and follows good practices. The null-safe color parsing and direct property mapping are appropriate for this use case.
Consider adding documentation to the
parsemethod for improved maintainability:+ /// Converts this [StacDividerThemeData] to Flutter's [DividerThemeData]. + /// + /// The [context] is used to resolve theme-dependent color values. DividerThemeData parse(BuildContext context) {packages/stac/lib/src/parsers/theme/stac_chip_theme_data_parser.dart (1)
35-35: Parser pattern is inconsistent but likely intentional.The
brightness?.parseuses a getter pattern while other parsers use method calls with context (e.g.,side?.parse(context)). This is acceptable if brightness parsing doesn't require context, but the inconsistency is worth noting for future maintainability.packages/stac/lib/src/parsers/foundation/theme/stac_button_text_theme_parser.dart (1)
8-17: Unusedcontextparameter.The
contextparameter is accepted but never used in this method. This appears intentional for API consistency with other parsers, but consider whether this parser truly needs the context parameter or if a simpler signature would be more appropriate.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (92)
.gitignore(1 hunks)packages/stac/lib/src/framework/stac_app.dart(1 hunks)packages/stac/lib/src/parsers/foundation/foundation.dart(2 hunks)packages/stac/lib/src/parsers/foundation/navigation/stac_navigation_destination_label_behavior_parser.dart(1 hunks)packages/stac/lib/src/parsers/foundation/theme/stac_button_bar_layout_behavior_parser.dart(1 hunks)packages/stac/lib/src/parsers/foundation/theme/stac_button_text_theme_parser.dart(1 hunks)packages/stac/lib/src/parsers/foundation/ui_components/stac_dropdown_menu_entry_parser.dart(1 hunks)packages/stac/lib/src/parsers/theme/stac_app_bar_theme/stac_app_bar_theme.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_app_bar_theme/stac_app_bar_theme.freezed.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_app_bar_theme/stac_app_bar_theme.g.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_app_bar_theme_parser.dart(1 hunks)packages/stac/lib/src/parsers/theme/stac_badge_theme_data/stac_badge_theme_data.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_badge_theme_data/stac_badge_theme_data.freezed.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_badge_theme_data/stac_badge_theme_data.g.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_bottom_app_bar_theme/stac_bottom_app_bar_theme.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_bottom_app_bar_theme/stac_bottom_app_bar_theme.freezed.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_bottom_app_bar_theme/stac_bottom_app_bar_theme.g.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_bottom_app_bar_theme_parser.dart(1 hunks)packages/stac/lib/src/parsers/theme/stac_bottom_nav_bar_theme/stac_bottom_nav_bar_theme.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_bottom_nav_bar_theme/stac_bottom_nav_bar_theme.freezed.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_bottom_nav_bar_theme/stac_bottom_nav_bar_theme.g.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_bottom_nav_bar_theme_parser.dart(1 hunks)packages/stac/lib/src/parsers/theme/stac_bottom_sheet_theme/stac_bottom_sheet_theme.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_bottom_sheet_theme/stac_bottom_sheet_theme.freezed.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_bottom_sheet_theme/stac_bottom_sheet_theme.g.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_bottom_sheet_theme_parser.dart(1 hunks)packages/stac/lib/src/parsers/theme/stac_button_theme_data/stac_button_theme_data.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_button_theme_data/stac_button_theme_data.freezed.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_button_theme_data/stac_button_theme_data.g.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_button_theme_data_parser.dart(1 hunks)packages/stac/lib/src/parsers/theme/stac_card_theme_data/stac_card_theme_data.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_card_theme_data/stac_card_theme_data.freezed.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_card_theme_data/stac_card_theme_data.g.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_card_theme_data_parser.dart(1 hunks)packages/stac/lib/src/parsers/theme/stac_checkbox_theme_data/stac_checkbox_theme_data.freezed.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_checkbox_theme_data_parser.dart(2 hunks)packages/stac/lib/src/parsers/theme/stac_chip_theme_data/stac_chip_theme_data.freezed.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_chip_theme_data/stac_chip_theme_data.g.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_chip_theme_data_parser.dart(2 hunks)packages/stac/lib/src/parsers/theme/stac_color_scheme/stac_color_scheme.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_color_scheme/stac_color_scheme.freezed.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_color_scheme_parser.dart(1 hunks)packages/stac/lib/src/parsers/theme/stac_date_picker_theme_data/stac_date_picker_theme_data.freezed.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_date_picker_theme_data/stac_date_picker_theme_data.g.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_date_picker_theme_data_parser.dart(2 hunks)packages/stac/lib/src/parsers/theme/stac_dialog_theme/stac_dialog_theme.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_dialog_theme/stac_dialog_theme.freezed.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_dialog_theme/stac_dialog_theme.g.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_dialog_theme_data_parser.dart(1 hunks)packages/stac/lib/src/parsers/theme/stac_dialog_theme_parser.dart(1 hunks)packages/stac/lib/src/parsers/theme/stac_divider_theme_data/stac_divider_theme_data.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_divider_theme_data/stac_divider_theme_data.freezed.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_divider_theme_data/stac_divider_theme_data.g.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_divider_theme_data_parser.dart(1 hunks)packages/stac/lib/src/parsers/theme/stac_drawer_theme_data/stac_drawer_theme_data.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_drawer_theme_data/stac_drawer_theme_data.freezed.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_drawer_theme_data/stac_drawer_theme_data.g.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_drawer_theme_data_parser.dart(1 hunks)packages/stac/lib/src/parsers/theme/stac_floating_action_button_theme_data/stac_floating_action_button_theme_data.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_floating_action_button_theme_data/stac_floating_action_button_theme_data.freezed.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_floating_action_button_theme_data/stac_floating_action_button_theme_data.g.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_floating_action_button_theme_data_parser.dart(1 hunks)packages/stac/lib/src/parsers/theme/stac_icon_theme_data/stac_icon_theme_data.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_icon_theme_data/stac_icon_theme_data.freezed.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_icon_theme_data_parser.dart(1 hunks)packages/stac/lib/src/parsers/theme/stac_list_tile_theme_data/stac_list_tile_theme_data.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_list_tile_theme_data/stac_list_tile_theme_data.freezed.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_list_tile_theme_data/stac_list_tile_theme_data.g.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_list_tile_theme_data_parser.dart(1 hunks)packages/stac/lib/src/parsers/theme/stac_material_banner_theme_data/stac_material_banner_theme_data.freezed.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_material_banner_theme_data/stac_material_banner_theme_data.g.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_material_banner_theme_data_parser.dart(1 hunks)packages/stac/lib/src/parsers/theme/stac_material_color/stac_material_color.freezed.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_material_color_parser.dart(2 hunks)packages/stac/lib/src/parsers/theme/stac_navigation_bar_theme_data/stac_navigation_bar_theme_data.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_navigation_bar_theme_data/stac_navigation_bar_theme_data.freezed.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_navigation_bar_theme_data/stac_navigation_bar_theme_data.g.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_navigation_bar_theme_data_parser.dart(1 hunks)packages/stac/lib/src/parsers/theme/stac_navigation_drawer_theme_data/stac_navigation_drawer_theme_data.freezed.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_navigation_drawer_theme_data/stac_navigation_drawer_theme_data.g.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_navigation_drawer_theme_data_parser.dart(1 hunks)packages/stac/lib/src/parsers/theme/stac_scrollbar_theme_data/stac_scrollbar_theme_data.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_scrollbar_theme_data/stac_scrollbar_theme_data.freezed.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_scrollbar_theme_data/stac_scrollbar_theme_data.g.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_scrollbar_theme_data_parser.dart(1 hunks)packages/stac/lib/src/parsers/theme/stac_snack_bar_theme_data/stac_snack_bar_theme_data.freezed.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_snack_bar_theme_data/stac_snack_bar_theme_data.g.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_snack_bar_theme_data_parser.dart(2 hunks)packages/stac/lib/src/parsers/theme/stac_tab_bar_theme_data/stac_tab_bar_theme_data.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_tab_bar_theme_data/stac_tab_bar_theme_data.freezed.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_tab_bar_theme_data/stac_tab_bar_theme_data.g.dart(0 hunks)packages/stac/lib/src/parsers/theme/stac_tab_bar_theme_data_parser.dart(1 hunks)
💤 Files with no reviewable changes (61)
- packages/stac/lib/src/parsers/theme/stac_button_theme_data/stac_button_theme_data.dart
- packages/stac/lib/src/parsers/theme/stac_app_bar_theme/stac_app_bar_theme.dart
- packages/stac/lib/src/parsers/theme/stac_divider_theme_data/stac_divider_theme_data.g.dart
- packages/stac/lib/src/parsers/theme/stac_dialog_theme/stac_dialog_theme.g.dart
- packages/stac/lib/src/parsers/theme/stac_navigation_bar_theme_data/stac_navigation_bar_theme_data.dart
- packages/stac/lib/src/parsers/theme/stac_icon_theme_data/stac_icon_theme_data.dart
- packages/stac/lib/src/parsers/theme/stac_drawer_theme_data/stac_drawer_theme_data.g.dart
- packages/stac/lib/src/parsers/theme/stac_navigation_bar_theme_data/stac_navigation_bar_theme_data.g.dart
- packages/stac/lib/src/parsers/theme/stac_bottom_nav_bar_theme/stac_bottom_nav_bar_theme.g.dart
- packages/stac/lib/src/parsers/theme/stac_bottom_app_bar_theme/stac_bottom_app_bar_theme.g.dart
- packages/stac/lib/src/parsers/theme/stac_badge_theme_data/stac_badge_theme_data.g.dart
- packages/stac/lib/src/parsers/theme/stac_scrollbar_theme_data/stac_scrollbar_theme_data.g.dart
- packages/stac/lib/src/parsers/theme/stac_snack_bar_theme_data/stac_snack_bar_theme_data.freezed.dart
- packages/stac/lib/src/parsers/theme/stac_list_tile_theme_data/stac_list_tile_theme_data.g.dart
- packages/stac/lib/src/parsers/theme/stac_card_theme_data/stac_card_theme_data.dart
- packages/stac/lib/src/parsers/theme/stac_card_theme_data/stac_card_theme_data.freezed.dart
- packages/stac/lib/src/parsers/theme/stac_material_banner_theme_data/stac_material_banner_theme_data.g.dart
- packages/stac/lib/src/parsers/theme/stac_app_bar_theme/stac_app_bar_theme.g.dart
- packages/stac/lib/src/parsers/theme/stac_bottom_sheet_theme/stac_bottom_sheet_theme.freezed.dart
- packages/stac/lib/src/parsers/theme/stac_chip_theme_data/stac_chip_theme_data.g.dart
- packages/stac/lib/src/parsers/theme/stac_bottom_nav_bar_theme/stac_bottom_nav_bar_theme.freezed.dart
- packages/stac/lib/src/parsers/theme/stac_card_theme_data/stac_card_theme_data.g.dart
- packages/stac/lib/src/parsers/theme/stac_badge_theme_data/stac_badge_theme_data.dart
- packages/stac/lib/src/parsers/theme/stac_date_picker_theme_data/stac_date_picker_theme_data.g.dart
- packages/stac/lib/src/parsers/theme/stac_bottom_sheet_theme/stac_bottom_sheet_theme.g.dart
- packages/stac/lib/src/parsers/theme/stac_button_theme_data/stac_button_theme_data.g.dart
- packages/stac/lib/src/parsers/theme/stac_scrollbar_theme_data/stac_scrollbar_theme_data.dart
- packages/stac/lib/src/parsers/theme/stac_dialog_theme/stac_dialog_theme.dart
- packages/stac/lib/src/parsers/theme/stac_bottom_app_bar_theme/stac_bottom_app_bar_theme.freezed.dart
- packages/stac/lib/src/parsers/theme/stac_floating_action_button_theme_data/stac_floating_action_button_theme_data.freezed.dart
- packages/stac/lib/src/parsers/theme/stac_divider_theme_data/stac_divider_theme_data.dart
- packages/stac/lib/src/parsers/theme/stac_app_bar_theme/stac_app_bar_theme.freezed.dart
- packages/stac/lib/src/parsers/theme/stac_bottom_app_bar_theme/stac_bottom_app_bar_theme.dart
- packages/stac/lib/src/parsers/theme/stac_color_scheme/stac_color_scheme.dart
- packages/stac/lib/src/parsers/theme/stac_tab_bar_theme_data/stac_tab_bar_theme_data.dart
- packages/stac/lib/src/parsers/theme/stac_tab_bar_theme_data/stac_tab_bar_theme_data.g.dart
- packages/stac/lib/src/parsers/theme/stac_date_picker_theme_data/stac_date_picker_theme_data.freezed.dart
- packages/stac/lib/src/parsers/theme/stac_divider_theme_data/stac_divider_theme_data.freezed.dart
- packages/stac/lib/src/parsers/theme/stac_color_scheme/stac_color_scheme.freezed.dart
- packages/stac/lib/src/parsers/theme/stac_badge_theme_data/stac_badge_theme_data.freezed.dart
- packages/stac/lib/src/parsers/theme/stac_navigation_drawer_theme_data/stac_navigation_drawer_theme_data.g.dart
- packages/stac/lib/src/parsers/theme/stac_checkbox_theme_data/stac_checkbox_theme_data.freezed.dart
- packages/stac/lib/src/parsers/theme/stac_list_tile_theme_data/stac_list_tile_theme_data.freezed.dart
- packages/stac/lib/src/parsers/theme/stac_floating_action_button_theme_data/stac_floating_action_button_theme_data.dart
- packages/stac/lib/src/parsers/theme/stac_dialog_theme/stac_dialog_theme.freezed.dart
- packages/stac/lib/src/parsers/theme/stac_bottom_nav_bar_theme/stac_bottom_nav_bar_theme.dart
- packages/stac/lib/src/parsers/theme/stac_icon_theme_data/stac_icon_theme_data.freezed.dart
- packages/stac/lib/src/parsers/theme/stac_navigation_bar_theme_data/stac_navigation_bar_theme_data.freezed.dart
- packages/stac/lib/src/parsers/theme/stac_floating_action_button_theme_data/stac_floating_action_button_theme_data.g.dart
- packages/stac/lib/src/parsers/theme/stac_bottom_sheet_theme/stac_bottom_sheet_theme.dart
- packages/stac/lib/src/parsers/theme/stac_drawer_theme_data/stac_drawer_theme_data.dart
- packages/stac/lib/src/parsers/theme/stac_material_color/stac_material_color.freezed.dart
- packages/stac/lib/src/parsers/theme/stac_button_theme_data/stac_button_theme_data.freezed.dart
- packages/stac/lib/src/parsers/theme/stac_tab_bar_theme_data/stac_tab_bar_theme_data.freezed.dart
- packages/stac/lib/src/parsers/theme/stac_list_tile_theme_data/stac_list_tile_theme_data.dart
- packages/stac/lib/src/parsers/theme/stac_drawer_theme_data/stac_drawer_theme_data.freezed.dart
- packages/stac/lib/src/parsers/theme/stac_navigation_drawer_theme_data/stac_navigation_drawer_theme_data.freezed.dart
- packages/stac/lib/src/parsers/theme/stac_material_banner_theme_data/stac_material_banner_theme_data.freezed.dart
- packages/stac/lib/src/parsers/theme/stac_snack_bar_theme_data/stac_snack_bar_theme_data.g.dart
- packages/stac/lib/src/parsers/theme/stac_chip_theme_data/stac_chip_theme_data.freezed.dart
- packages/stac/lib/src/parsers/theme/stac_scrollbar_theme_data/stac_scrollbar_theme_data.freezed.dart
🔇 Additional comments (37)
.gitignore (1)
45-46: Good practice: FVM cache directory ignored.Adding
.fvm/to gitignore is appropriate for Flutter projects using FVM (Flutter Version Manager). This prevents the version cache from being accidentally committed to version control.packages/stac/lib/src/parsers/foundation/ui_components/stac_dropdown_menu_entry_parser.dart (1)
3-3: Clean import consolidation.The refactoring to consolidate theme-related parsers under
themes.dartimproves code organization and aligns well with the PR's objective to centralize theme functionality.packages/stac/lib/src/parsers/theme/stac_floating_action_button_theme_data_parser.dart (2)
1-5: Imports look good.All necessary dependencies are included for the parser implementation.
11-29: Implementation looks solid.The parse method correctly maps all properties with appropriate null handling and context-aware parsing for colors and text styles. The pattern is consistent with other theme parsers in the codebase.
packages/stac/lib/src/parsers/theme/stac_icon_theme_data_parser.dart (3)
1-4: LGTM: Clean imports.All imports are necessary and properly used in the implementation.
6-9: LGTM: Clear documentation and extension declaration.The documentation accurately describes the extension's purpose, and the naming follows a clear, consistent pattern.
10-22: LGTM: Clean and straightforward implementation.The parse method correctly maps all IconThemeData fields with appropriate null-safe handling. The implementation follows the extension-based parsing pattern described in the PR objectives.
packages/stac/lib/src/parsers/theme/stac_divider_theme_data_parser.dart (1)
1-3: LGTM! Clean import structure.The imports are well-organized and all necessary for the parser implementation.
packages/stac/lib/src/framework/stac_app.dart (1)
2-2: Import consolidation verified as correct.The new import path properly exports
StacThemefromstac_corevia the centralizedthemes.dartexport file, and all parser extensions are also available. No remaining references to the old import path exist elsewhere in the codebase. The change is complete and aligns with the refactoring objectives.packages/stac/lib/src/parsers/theme/stac_snack_bar_theme_data_parser.dart (1)
1-35: LGTM! Clean refactoring from Freezed to extension-based parser.The implementation is well-structured and consistent:
- New imports for behavior and dismissDirection parsers are correctly integrated
- Documentation clearly explains the extension's purpose
- Parser API design is logical (context-free
.parsefor enums/simple types,.parse(context)for theme-aware types)- Null safety is properly handled throughout
- Formatting improvement on lines 30-32 enhances readability
packages/stac/lib/src/parsers/theme/stac_drawer_theme_data_parser.dart (3)
1-5: LGTM!The imports are clean, necessary, and properly organized for the parser extension implementation.
7-9: Clear documentation.The doc comment effectively explains the purpose and behavior of the parser extension.
10-24: Code implementation is correct and approved.The extension properly converts
StacDrawerThemeDatato Flutter'sDrawerThemeDatawith appropriate null-safe handling. The parsing API difference on line 21 is intentional:clipBehavior?.parsecorrectly accesses the getter fromStacClipParser, whileshape?.parse(context)andendShape?.parse(context)call methods that require context. Each parser appropriately adapts to its implementation needs.packages/stac/lib/src/parsers/theme/stac_scrollbar_theme_data_parser.dart (1)
5-24: LGTM! Clean parser implementation.The extension follows a consistent pattern for converting STAC theme data to Flutter's ScrollbarThemeData. The use of
WidgetStatePropertyAllfor state-independent properties and context-aware color resolution is appropriate.packages/stac/lib/src/parsers/theme/stac_checkbox_theme_data_parser.dart (1)
6-6: LGTM! Clean refactoring from Freezed to extension-based parser.The changes correctly implement the extension-based parser pattern:
- The import for
stac_material_tap_target_size_parseris necessary for the.parsemethod on line 27- Documentation clearly describes the extension's purpose
- The
.parsemethod usage is consistent with other parsers in this file (e.g.,visualDensity?.parseon line 28)Also applies to: 10-12, 27-27
packages/stac/lib/src/parsers/theme/stac_navigation_bar_theme_data_parser.dart (3)
1-10: LGTM! Clear structure and documentation.The imports are well-organized, and the documentation clearly explains the extension's purpose. The separation of the parser extension from the data class (now in stac_core) aligns well with the PR's refactoring goals.
20-21: Verify the null-handling semantics with WidgetStateProperty.all().When
labelTextStyleoriconThemeis null, the current code createsWidgetStateProperty.all(null), which explicitly sets the property to null for all states. This differs from passingnulldirectly to the constructor, which would mean "no override, use default."Consider whether the intended behavior is:
- Current: Always override with the parsed value (or null)
- Alternative: Only override when a value is provided
If null should mean "no override," consider this pattern:
labelTextStyle: labelTextStyle != null ? WidgetStateProperty.all(labelTextStyle.parse(context)) : null, iconTheme: iconTheme != null ? WidgetStateProperty.all(iconTheme.parse(context)) : null,
22-22: Verify the parse pattern for labelBehavior.The
labelBehavior?.parsepattern (without context parameter) differs from other fields that useparse(context). This suggestsparseis a property or getter rather than a method, which may be correct iflabelBehavioris an enum or doesn't require context.Please verify this API is consistent with the
labelBehaviortype definition in stac_core.packages/stac/lib/src/parsers/theme/stac_tab_bar_theme_data_parser.dart (2)
1-11: LGTM! Clean imports and documentation.The imports are well-organized and the documentation clearly explains the extension's purpose. The separation of concerns is good—data classes in
stac_core, parsers in thestacpackage.
12-23: LGTM! Parsing logic is correct and consistent.The field mappings are appropriate, and the parsing approach is consistent:
- Context-dependent parsers (decoration, text styles) correctly pass
context- Simple conversions (size, insets) use the
.parsegetter- Colors consistently use the
.toColor(context)utilitypackages/stac/lib/src/parsers/theme/stac_chip_theme_data_parser.dart (3)
1-10: LGTM! Imports are clean and necessary.All imports are properly utilized throughout the parser extension.
12-14: LGTM! Clear documentation.The documentation concisely explains the extension's purpose and transformation.
18-18: Review null handling forWidgetStatePropertyAll.When
coloris null, this createsWidgetStatePropertyAll(null)which explicitly sets the color to null rather than leaving it undefined. This may prevent Flutter from using its default chip color from the theme.Consider using a conditional:
color: color != null ? WidgetStatePropertyAll(color.toColor(context)) : null,This ensures that when the STAC theme doesn't specify a color, Flutter's default theming behavior is preserved.
packages/stac/lib/src/parsers/theme/stac_material_banner_theme_data_parser.dart (2)
7-10: LGTM! Clear documentation and proper extension declaration.The documentation clearly explains the extension's purpose, and the extension declaration follows proper Dart conventions.
11-22: <!-- [scratchpad_start]
[task verification_3/3]
Finalize findings and determine outcome.[observations]
- StacEdgeInsets.parse IS a GETTER (not a method) returning EdgeInsets
- String?.toColor(context) extension exists and takes context (for color parsing)
- StacTextStyle.parse(context) takes context (for theme-aware text style)
- Verified StacSnackBarThemeDataParser uses identical mixed pattern:
.parse(getter) for EdgeInsets.parse(context)(method) for TextStyle and borders.toColor(context)for colors- All patterns are consistent, intentional, and correct
[analysis]
CRITICAL: Review comment is based on FALSE PREMISE
- Claim: "breaking refactor removing the Freezed-generated StacMaterialBannerThemeData class"
- Reality: Old class STILL EXISTS; this adds extension, not replacing it
- NOT a breaking change—purely additive
- Implementation is CORRECT and follows codebase conventions
[pending]
None - verification complete.[done]
- Old Freezed class still exists (not removed/replaced)
- Parser adds extension method to existing class
- Parse patterns are correct and consistent with other parsers
- No migration issues exist
- Review comment premise is incorrect
[conclusion]
Use tag - review comment fundamentally mischaracterizes the change.
</scratchpad_end] -->Review comment premise is incorrect—this is not a breaking refactor.
The old
StacMaterialBannerThemeDataFreezed class inpackages/stac_core/lib/foundation/theme/stac_material_banner_theme_data/still exists with its@JsonSerializable()andfromJsonfactory intact. This PR adds an extension method, not a replacement or removal. This is an additive change, not a breaking refactor.The parse method implementation is correct and consistent with other theme parsers in the codebase. The mixed patterns are intentional:
.parse(getter) forStacEdgeInsetshandles numeric conversion only,.parse(context)forStacTextStyleprovides theme context for text styling, and.toColor(context)for colors handles hex parsing and theme-aware color resolution.Likely an incorrect or invalid review comment.
packages/stac/lib/src/parsers/theme/stac_bottom_sheet_theme_parser.dart (3)
1-11: LGTM! Clean imports and documentation.The imports are well-organized and the documentation clearly describes the extension's purpose.
13-29: Field mapping is complete and correct.Flutter's BottomSheetThemeData accepts 13 parameters: backgroundColor, surfaceTintColor, elevation, modalBackgroundColor, modalBarrierColor, shadowColor, modalElevation, shape, showDragHandle, dragHandleColor, dragHandleSize, clipBehavior, and constraints. The parser correctly maps all 13 fields from StacBottomSheetThemeData with appropriate type conversions.
22-27: The review comment is incorrect. The parsing patterns are intentional and correct.
shape?.parse(context): StacBorder uses StacShapeBorderParser, which is a method requiring BuildContextdragHandleSize?.parse,clipBehavior?.parse,constraints?.parse: StacSize, StacClip, and StacBoxConstraints use getter-based parsers that don't require contextEach field type correctly matches its corresponding parser implementation. The inconsistency reflects intentional design differences in how different parsers are implemented.
Likely an incorrect or invalid review comment.
packages/stac/lib/src/parsers/theme/stac_material_color_parser.dart (2)
1-3: LGTM! Import structure aligns with refactoring.The addition of
stac_coreimport properly supports the migration to extension-based parsing.
5-8: LGTM! Clean extension pattern with clear documentation.The extension-based API is idiomatic and well-documented.
packages/stac/lib/src/parsers/theme/stac_color_scheme_parser.dart (1)
10-60: LGTM! Comprehensive ColorScheme mapping.The parser correctly handles all ColorScheme fields with appropriate null handling. The non-null assertions on required fields (primary, onPrimary, secondary, onSecondary, error, onError, surface, onSurface) align with Flutter's ColorScheme requirements.
packages/stac/lib/src/parsers/theme/stac_list_tile_theme_data_parser.dart (1)
10-31: LGTM! Clean ListTileThemeData mapping.The parser correctly handles all ListTileThemeData fields with appropriate nullable handling and delegates to the correct nested parsers.
packages/stac/lib/src/parsers/foundation/navigation/stac_navigation_destination_label_behavior_parser.dart (1)
9-18: LGTM! Clean enum mapping.The exhaustive switch correctly maps all enum values between the STAC and Flutter types.
packages/stac/lib/src/parsers/foundation/theme/stac_button_bar_layout_behavior_parser.dart (1)
8-15: LGTM! Clean enum mapping.The exhaustive switch correctly maps both enum values between the STAC and Flutter types.
packages/stac/lib/src/parsers/foundation/foundation.dart (1)
75-75: LGTM!The new parser exports are properly organized and consistent with the existing export structure.
Also applies to: 95-96
packages/stac/lib/src/parsers/theme/stac_dialog_theme_parser.dart (1)
13-26: LGTM!Clean and straightforward implementation with proper null-safety and appropriate use of parsing utilities. The field mappings are correctly applied.
packages/stac/lib/src/parsers/theme/stac_button_theme_data_parser.dart (1)
11-17: No changes needed—design pattern is intentional and appropriate.The hardcoded defaults are not unnecessary duplication. The values match Flutter's ButtonThemeData defaults, but explicitly providing them here is deliberate: it makes defaults visible in the code, ensures stable behavior even if Flutter's defaults change, and clearly separates user-specified values from defaults. This is a sound defensive programming practice.
Likely an incorrect or invalid review comment.
packages/stac/lib/src/parsers/theme/stac_app_bar_theme_parser.dart
Outdated
Show resolved
Hide resolved
packages/stac/lib/src/parsers/theme/stac_bottom_app_bar_theme_parser.dart
Outdated
Show resolved
Hide resolved
packages/stac/lib/src/parsers/theme/stac_bottom_app_bar_theme_parser.dart
Outdated
Show resolved
Hide resolved
packages/stac/lib/src/parsers/theme/stac_bottom_nav_bar_theme_parser.dart
Outdated
Show resolved
Hide resolved
packages/stac/lib/src/parsers/theme/stac_card_theme_data_parser.dart
Outdated
Show resolved
Hide resolved
packages/stac/lib/src/parsers/theme/stac_chip_theme_data_parser.dart
Outdated
Show resolved
Hide resolved
packages/stac/lib/src/parsers/theme/stac_dialog_theme_data_parser.dart
Outdated
Show resolved
Hide resolved
packages/stac/lib/src/parsers/theme/stac_scrollbar_theme_data_parser.dart
Outdated
Show resolved
Hide resolved
… to non-nullable AppBarTheme
- Updated various widget classes to ensure consistent formatting in JSON serialization. - Simplified null checks and improved readability across multiple files, including StacButton, StacCard, StacColumn, and others. - Enhanced maintainability by standardizing the structure of child widget handling and property parsing.
Description
We have added Stac Theme to stac_core to provide the DSL support for the Stac theme. This will allow us to write the theme in Dart instead of json.
Type of Change
Summary by CodeRabbit
Refactor
New Features
Chores