feat(repo): Add theme export from gallery - #147
Open
renefloor wants to merge 3 commits into
Open
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (15)
🚧 Files skipped from review as they are similar to previous changes (8)
📝 WalkthroughWalkthroughChangesTheme Studio and Export
macOS build integration
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Toolbar
participant ThemeExportPage
participant ThemeExportConfiguration
participant ThemeExportCodePane
Toolbar->>ThemeExportPage: Open Export Theme
ThemeExportPage->>ThemeExportConfiguration: Seed light and dark configurations
ThemeExportPage->>ThemeExportConfiguration: Apply linked or independent edits
ThemeExportConfiguration->>ThemeExportCodePane: Provide refreshed generated Dart code
ThemeExportPage->>ThemeExportCodePane: Render selectable code
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #147 +/- ##
==========================================
+ Coverage 58.17% 62.31% +4.13%
==========================================
Files 187 202 +15
Lines 7615 8650 +1035
==========================================
+ Hits 4430 5390 +960
- Misses 3185 3260 +75 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
renefloor
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (5)
apps/design_system_gallery/test/core/theme_code_generator_test.dart (1)
211-294: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a case where one component property differs between light and dark.
The component tests cover equal-on-both-sides and light-only. They do not cover the split case, which produces two suffixed consts for the same component property. That path is the default in
ThemeExportConfiguration, because component property colors start unlinked.test('a component property with different colors per side becomes two suffixed consts', () { final code = generateThemeCode( light: const ThemeExportSide( componentOverrides: {'Online Indicator': {'backgroundOnline': Color(0xFF111111)}}, ), dark: const ThemeExportSide( componentOverrides: {'Online Indicator': {'backgroundOnline': Color(0xFF222222)}}, ), ); expect(code, contains('const onlineIndicatorBackgroundOnlineLight = ')); expect(code, contains('const onlineIndicatorBackgroundOnlineDark = ')); expect(code, contains('backgroundOnline: onlineIndicatorBackgroundOnlineLight')); expect(code, contains('backgroundOnline: onlineIndicatorBackgroundOnlineDark')); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/design_system_gallery/test/core/theme_code_generator_test.dart` around lines 211 - 294, Add a test alongside the existing component theme tests covering different light and dark values for the same Online Indicator backgroundOnline property. In the new test, use distinct colors on each ThemeExportSide and assert that generateThemeCode emits Light and Dark suffixed constants and passes each corresponding constant to the light and dark StreamOnlineIndicatorThemeData constructors.apps/design_system_gallery/lib/app/theme_export_page.dart (1)
210-227: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value
firstWherethrows when an active component name has no descriptor.Line 211 uses
firstWherewith noorElse. It throwsStateErrorduring build ifactiveComponentThemesever contains a name that is not incomponentThemeDescriptors. Today both come from the same descriptor list, so the invariant holds. It breaks silently if a descriptor is renamed or removed while state persists.A lookup map also removes the O(components × descriptors) scan from every rebuild:
♻️ Proposed lookup
+ final descriptorsByName = {for (final d in componentThemeDescriptors) d.name: d}; for (final component in export.activeComponentThemes) { - final descriptor = componentThemeDescriptors.firstWhere((d) => d.name == component); + final descriptor = descriptorsByName[component]; + if (descriptor == null) continue;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/design_system_gallery/lib/app/theme_export_page.dart` around lines 210 - 227, Update the component-theme rendering loop around activeComponentThemes to use a name-keyed descriptor lookup instead of firstWhere, and safely skip or otherwise handle active components with no matching descriptor without throwing during build. Preserve rendering for components with valid descriptors and avoid rescanning componentThemeDescriptors on each rebuild.apps/design_system_gallery/lib/config/theme_configuration.dart (1)
244-249: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winComponent themes are keyed by a free-form
Stringwith no total name-to-descriptor lookup.addComponentThemeandsetComponentColoraccept anyString, but both consumers resolve that name withcomponentThemeDescriptors.firstWhere(...)and noorElse. An unknown name therefore throwsStateError— once inside_rebuildTheme, and once insidebuild. Add one shared resolver (for example aMap<String, ComponentThemeDescriptor>built fromcomponentThemeDescriptors, or adescriptorFor(String name)helper returning a nullable descriptor) and use it at both sites.
apps/design_system_gallery/lib/config/theme_configuration.dart#L244-L249: replace thefirstWherein_buildComponentThemewith the shared lookup and returnnull(with anassert) for an unknown name.apps/design_system_gallery/lib/widgets/theme_studio/theme_customization_panel.dart#L102-L108: use the same lookup and skip rendering a section whose name has no descriptor, so an unknown name never breaks the panel'sbuild.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/design_system_gallery/lib/config/theme_configuration.dart` around lines 244 - 249, The component theme name lookup can throw for unknown free-form names. In apps/design_system_gallery/lib/config/theme_configuration.dart#L244-L249, add a shared nullable resolver for componentThemeDescriptors and update _buildComponentTheme to assert and return null when no descriptor exists; in apps/design_system_gallery/lib/widgets/theme_studio/theme_customization_panel.dart#L102-L108, reuse that resolver and skip sections without descriptors so build remains safe.apps/design_system_gallery/lib/widgets/theme_export/export_column_row.dart (1)
51-51: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace the hardcoded seam width with a spacing token.
Line 51 hardcodes a 40 px layout value. Read this value from
context.streamSpacingso the export layout follows the configured spacing scale.As per coding guidelines, use
StreamThemetokens for styling instead of hardcoded values.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/design_system_gallery/lib/widgets/theme_export/export_column_row.dart` at line 51, Update the SizedBox in the export column row to use context.streamSpacing instead of the hardcoded width of 40, preserving the existing middle widget fallback and aligning the layout with StreamTheme spacing tokens.Source: Coding guidelines
apps/design_system_gallery/lib/widgets/theme_export/theme_export_code_pane.dart (1)
71-75: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPaint the border with
foregroundDecoration.Line 74 paints the border in
decoration. Move the border toContainer.foregroundDecorationand keep the background and radius indecoration.As per coding guidelines, use
foregroundDecorationfor borders to prevent clipping.Proposed fix
decoration: BoxDecoration( color: colorScheme.backgroundElevation0, borderRadius: BorderRadius.all(context.streamRadius.sm), - border: Border.all(color: colorScheme.borderSubtle), ), + foregroundDecoration: BoxDecoration( + borderRadius: BorderRadius.all(context.streamRadius.sm), + border: Border.all(color: colorScheme.borderSubtle), + ),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/design_system_gallery/lib/widgets/theme_export/theme_export_code_pane.dart` around lines 71 - 75, Update the Container decoration in the theme export code pane so decoration retains only color and borderRadius, while the border is supplied through Container.foregroundDecoration. Preserve the existing border color and radius values.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/design_system_gallery/AGENTS.md`:
- Around line 59-73: Update the config structure list to include
component_theme_descriptors.dart and theme_export_configuration.dart with
accurate descriptions. Revise the test coverage sentence to clearly state that
tests cover StreamColorScheme in theme_color_slot.dart, component themes, export
configuration, the color picker tile, and the export page.
In `@apps/design_system_gallery/lib/app/theme_export_page.dart`:
- Around line 278-304: Update the Row containing the title and subtitle so the
title Text uses Expanded and receives the remaining width, while the subtitle
chip’s Flexible uses flex: 0 to size to its content. Preserve the existing gap,
styling, and ellipsis truncation, then verify spacing and truncation against the
Figma section-header specification.
In `@apps/design_system_gallery/lib/core/theme_code_generator.dart`:
- Around line 83-86: In
apps/design_system_gallery/lib/core/theme_code_generator.dart lines 83-86,
rewrite the comment to state that ThemeExportConfiguration edits component
property colors independently per brightness and leaves them initially unlinked.
In apps/design_system_gallery/test/core/theme_code_generator_test.dart lines
211-294, add coverage for a component property with different light and dark
colors, asserting both brightness-suffixed constants and their corresponding
references.
In
`@apps/design_system_gallery/lib/widgets/theme_studio/add_component_theme_button.dart`:
- Around line 33-38: Update the button Container’s border styling to use
foregroundDecoration instead of decoration, matching ToolbarButton’s existing
approach and the project guideline; preserve the current rounded border
appearance and padding.
In `@apps/design_system_gallery/lib/widgets/theme_studio/color_picker_tile.dart`:
- Around line 178-187: Update the placeholder Text caption in the color picker
tile so it remains laid out for height alignment but is excluded from the
semantics tree, preventing customized tiles from announcing “default.”
In
`@apps/design_system_gallery/lib/widgets/theme_studio/theme_customization_panel.dart`:
- Around line 102-108: Add a shared name-based component theme resolution helper
in ThemeConfiguration that safely returns no descriptor when a name is absent,
then use it in both the panel’s activeComponentThemes rendering and the lookup
near setComponentColor/addComponentTheme instead of firstWhere. Skip or
otherwise preserve valid behavior for unresolved names so build never throws
StateError.
In `@apps/design_system_gallery/test/app/theme_export_page_test.dart`:
- Around line 129-131: Update the Avatar exclusion assertion in the theme export
page test to search for text containing “Avatar” within a SimpleDialog
descendant, rather than matching the exact unscoped text. Keep the existing
Online Indicator assertion unchanged.
---
Nitpick comments:
In `@apps/design_system_gallery/lib/app/theme_export_page.dart`:
- Around line 210-227: Update the component-theme rendering loop around
activeComponentThemes to use a name-keyed descriptor lookup instead of
firstWhere, and safely skip or otherwise handle active components with no
matching descriptor without throwing during build. Preserve rendering for
components with valid descriptors and avoid rescanning componentThemeDescriptors
on each rebuild.
In `@apps/design_system_gallery/lib/config/theme_configuration.dart`:
- Around line 244-249: The component theme name lookup can throw for unknown
free-form names. In
apps/design_system_gallery/lib/config/theme_configuration.dart#L244-L249, add a
shared nullable resolver for componentThemeDescriptors and update
_buildComponentTheme to assert and return null when no descriptor exists; in
apps/design_system_gallery/lib/widgets/theme_studio/theme_customization_panel.dart#L102-L108,
reuse that resolver and skip sections without descriptors so build remains safe.
In `@apps/design_system_gallery/lib/widgets/theme_export/export_column_row.dart`:
- Line 51: Update the SizedBox in the export column row to use
context.streamSpacing instead of the hardcoded width of 40, preserving the
existing middle widget fallback and aligning the layout with StreamTheme spacing
tokens.
In
`@apps/design_system_gallery/lib/widgets/theme_export/theme_export_code_pane.dart`:
- Around line 71-75: Update the Container decoration in the theme export code
pane so decoration retains only color and borderRadius, while the border is
supplied through Container.foregroundDecoration. Preserve the existing border
color and radius values.
In `@apps/design_system_gallery/test/core/theme_code_generator_test.dart`:
- Around line 211-294: Add a test alongside the existing component theme tests
covering different light and dark values for the same Online Indicator
backgroundOnline property. In the new test, use distinct colors on each
ThemeExportSide and assert that generateThemeCode emits Light and Dark suffixed
constants and passes each corresponding constant to the light and dark
StreamOnlineIndicatorThemeData constructors.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d0098e22-ae9a-4b59-9c22-241b39487486
📒 Files selected for processing (29)
apps/design_system_gallery/AGENTS.mdapps/design_system_gallery/lib/app/theme_export_page.dartapps/design_system_gallery/lib/config/component_theme_descriptors.dartapps/design_system_gallery/lib/config/theme_color_slot.dartapps/design_system_gallery/lib/config/theme_configuration.dartapps/design_system_gallery/lib/config/theme_export_configuration.dartapps/design_system_gallery/lib/config/theme_studio_sections.dartapps/design_system_gallery/lib/core/theme_code_generator.dartapps/design_system_gallery/lib/widgets/theme_export/export_column_row.dartapps/design_system_gallery/lib/widgets/theme_export/link_toggle_button.dartapps/design_system_gallery/lib/widgets/theme_export/linked_color_rows.dartapps/design_system_gallery/lib/widgets/theme_export/message_bubble_preview.dartapps/design_system_gallery/lib/widgets/theme_export/theme_export_code_pane.dartapps/design_system_gallery/lib/widgets/theme_export/theme_export_preview_bar.dartapps/design_system_gallery/lib/widgets/theme_export/theme_export_widgets.dartapps/design_system_gallery/lib/widgets/theme_studio/add_component_theme_button.dartapps/design_system_gallery/lib/widgets/theme_studio/color_picker_tile.dartapps/design_system_gallery/lib/widgets/theme_studio/theme_customization_panel.dartapps/design_system_gallery/lib/widgets/theme_studio/theme_studio_widgets.dartapps/design_system_gallery/lib/widgets/toolbar/toolbar.dartapps/design_system_gallery/macos/Runner.xcodeproj/project.pbxprojapps/design_system_gallery/macos/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcschemeapps/design_system_gallery/pubspec.yamlapps/design_system_gallery/test/app/theme_export_page_test.dartapps/design_system_gallery/test/config/component_theme_test.dartapps/design_system_gallery/test/config/theme_color_slot_test.dartapps/design_system_gallery/test/config/theme_export_configuration_test.dartapps/design_system_gallery/test/core/theme_code_generator_test.dartapps/design_system_gallery/test/widgets/theme_studio/color_picker_tile_test.dart
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Submit a pull request
CLA
Description of the pull request
This is a new feature that adds an export button which gives you code for the theme settings.
It's also possible to manage a couple of component themes, but we can add more components in a next PR.
It also refactors the regular Theme Studio panel to make it more maintainable.
Screenshots / Videos
(export in top right)
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests