feat: Implement device-specific dynamic theme switching - #620
Merged
Conversation
Add reactive theme system that automatically switches themes based on router model number. Key changes: - Add deviceThemeConfigProvider for reactive theme loading based on modelNumber - Extend BrandUtils with getDeviceTheme() method using model-to-suffix mapping - Integrate device theme provider in app.dart with AsyncValue error handling - Support forcedSource priority for CI/CD theme overrides (THEME_SOURCE) - Add theme_m60.json with glass style for M60 series devices - Update build_web.sh to support THEME_SOURCE and THEME_JSON parameters - Migrate from static GetIt theme loading to reactive Riverpod system - Preserve backward compatibility with existing theme infrastructure Technical implementation: - lib/providers/device_theme_config_provider.dart: FutureProvider with forcedSource priority - lib/utils.dart: BrandUtils.getDeviceTheme() with TB/CF/M60 model mapping - lib/app.dart: AsyncValue.when() handling for loading/error/data states - lib/main.dart: Simplified to register default theme as fallback only - lib/di.dart: Updated documentation explaining theme migration - assets/theme/: New directory structure for device-specific themes Testing: - Add 19 unit tests (11 BrandUtils + 8 Provider tests) - All tests passing with comprehensive coverage - test/utils/brand_utils_theme_test.dart: Theme loading and error handling - test/providers/device_theme_config_provider_test.dart: Provider behavior Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Replace M60-specific theme mapping with DU series mapping for broader device support. Changes: - Update _modelSuffixMap: 'M60' -> 'DU' in BrandUtils - Remove temporary theme_m60.json test file - DU mapping will match M60DU, M80DU, and other DU series devices This change aligns theme mapping with actual device model naming conventions where 'DU' is the consistent identifier across the product line. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Review Summary by QodoImplement device-specific dynamic theme switching with Riverpod
WalkthroughsDescription• Implement reactive device-specific theme switching via Riverpod provider • Extend BrandUtils with getDeviceTheme() method for model-based theme loading • Integrate deviceThemeConfigProvider in app.dart with AsyncValue state handling • Support CI/CD theme overrides via THEME_SOURCE environment variable • Add comprehensive unit tests for theme loading and provider behavior Diagramflowchart LR
A["sessionProvider<br/>modelNumber"] -->|"watch"| B["deviceThemeConfigProvider"]
C["ThemeConfigLoader<br/>forcedSource"] -->|"Priority 1"| B
B -->|"Priority 2"| D["BrandUtils<br/>getDeviceTheme"]
D -->|"model mapping"| E["Theme Assets<br/>TB/CF/DU"]
E -->|"fallback"| F["Default Theme"]
B -->|"AsyncValue"| G["app.dart<br/>_buildMaterialApp"]
G -->|"loading/error/data"| H["MaterialApp<br/>with Theme"]
File Changes1. lib/providers/device_theme_config_provider.dart
|
Code Review by Qodo
✅ 1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| - assets/icons/ | ||
| - assets/resources/ | ||
| - assets/a2ui/widgets/ | ||
| - assets/theme/ |
There was a problem hiding this comment.
1. Theme assets outside allowed dirs 📘 Rule violation ⛯ Reliability
• pubspec.yaml adds assets/theme/, introducing a new asset directory outside the designated assets/resources/ (and assets/icons/) locations. • This breaks the documented asset organization policy and can lead to inconsistent asset placement and harder-to-manage builds.
Agent Prompt
## Issue description
The PR adds a new Flutter asset directory `assets/theme/`, but the compliance checklist requires general resources to live under `assets/resources/`.
## Issue Context
Theme JSON files are general resources and should follow the repo’s asset directory conventions to keep asset management consistent.
## Fix Focus Areas
- pubspec.yaml[132-138]
- lib/utils.dart[769-776]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+59
to
+76
| // Check for forced theme source (CI/CD, testing) | ||
| final forcedSource = ThemeConfigLoader.forcedSource; | ||
|
|
||
| // Priority 1: Use forced source if set (environment variable override) | ||
| if (forcedSource != ThemeSource.normal) { | ||
| logger.i('[DeviceThemeConfig] Using forcedSource: $forcedSource'); | ||
| return ThemeConfigLoader.load(); | ||
| } | ||
|
|
||
| // Priority 2: Device-specific theme based on model number | ||
| final modelNumber = ref.watch( | ||
| sessionProvider.select((state) => state.modelNumber), | ||
| ); | ||
|
|
||
| logger.d('[DeviceThemeConfig] Loading device theme for model: $modelNumber'); | ||
|
|
||
| // Load device theme via BrandUtils (reuses brand asset mapping logic) | ||
| final themeConfig = await BrandUtils.getDeviceTheme(modelNumber); |
There was a problem hiding this comment.
2. Theme_json ignored in normal 🐞 Bug ✓ Correctness
• In the new flow, deviceThemeConfigProvider only calls ThemeConfigLoader.load() when THEME_SOURCE is *not* normal; otherwise it always uses BrandUtils.getDeviceTheme(modelNumber). • ThemeConfigLoader still supports the prior “priority” behavior where THEME_JSON alone (non-empty) wins even when THEME_SOURCE=normal. • Result: builds that previously set only THEME_JSON (without forcing THEME_SOURCE=cicd) will silently stop applying the override theme.
Agent Prompt
### Issue description
`deviceThemeConfigProvider` ignores `THEME_JSON` unless `THEME_SOURCE` is set to a non-normal value. Previously, `ThemeConfigLoader` would apply `THEME_JSON` in normal mode via priority resolution.
### Issue Context
This is a behavioral regression for CI/CD and local builds that set only `--dart-define=THEME_JSON=...` (and rely on default `THEME_SOURCE=normal`).
### Fix Focus Areas
- lib/providers/device_theme_config_provider.dart[59-76]
- lib/theme/theme_config_loader.dart[28-36]
### Suggested implementation
1. Add public getters in `ThemeConfigLoader` for env values (e.g., `static String get themeJsonEnv => _themeJsonEnv;` and similar for network url/asset path).
2. In `deviceThemeConfigProvider`, update the branch condition to also use `ThemeConfigLoader.themeJsonEnv.isNotEmpty` (and optionally other env signals) to decide when to call `ThemeConfigLoader.load()`.
3. (Optional) Add a short migration note in docs/build scripts stating that THEME_JSON-only builds must set THEME_SOURCE=cicd if you choose not to support the old behavior.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+116
to
+121
| return deviceThemeConfigAsync.when( | ||
| loading: () => MaterialApp( | ||
| home: Scaffold( | ||
| body: Center(child: CircularProgressIndicator()), | ||
| ), | ||
| ), |
There was a problem hiding this comment.
3. Loading swaps out router 🐞 Bug ⛯ Reliability
• While the theme provider is loading, app.dart returns a plain MaterialApp (no go_router) rather than MaterialApp.router. • deviceThemeConfigProvider reloads whenever sessionProvider.modelNumber changes (e.g., during prepare/login flows), so the app can transiently replace the router-based app with a different MaterialApp after startup. • This can reset navigation state/deeplinks and cause user-visible flicker; it also temporarily drops localization/theme configuration present in _buildMaterialApp.
Agent Prompt
### Issue description
The root `build()` returns `MaterialApp` (non-router) during theme loading, and `MaterialApp.router` otherwise. Since the theme provider can reload when `modelNumber` changes, this can reset routing/navigation state and cause UI flicker.
### Issue Context
`deviceThemeConfigProvider` watches `sessionProvider.modelNumber`, which is updated during prepare/login flows. That means theme reloads are not only at startup.
### Fix Focus Areas
- lib/app.dart[113-142]
- lib/providers/device_theme_config_provider.dart[68-77]
### Suggested implementation
1. Always return `_buildMaterialApp(...)` (i.e., always use `MaterialApp.router`).
2. Choose a themeConfig even while loading (use `themeAsync.value ?? ThemeJsonConfig.defaultConfig()`).
3. Show loading UI as an overlay in the `builder` rather than swapping out the entire app widget.
4. Consider `AsyncValue.when(skipLoadingOnReload: true)` (or equivalent) to keep the previous theme during refresh.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
This commit resolves issues identified in PR #620 code review: 1. Fix THEME_JSON ignored in normal mode - Add shouldUseDeviceTheme() method to ThemeConfigLoader - Check all environment overrides (THEME_SOURCE, THEME_JSON, THEME_NETWORK_URL) - Fixes regression where THEME_SOURCE=normal + THEME_JSON was ignored 2. Fix navigation reset during theme loading - Always use MaterialApp.router in all AsyncValue states - Use default theme during loading instead of non-router MaterialApp - Prevents navigation state loss when modelNumber changes 3. Standardize log format consistency - Update all theme-related logs to [TAG]: message format - Aligns with logger.dart regex pattern for proper parsing 4. Update documentation for build_web.sh parameters - Add themeSource and themeJson parameter documentation - Include usage examples in README.md and CLAUDE.md Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
… provider (#611) - Convert ThemeConfigLoader from static-only class to instance class with forTesting() constructor for dependency injection in tests - Move BrandUtils.getDeviceTheme() and _modelSuffixMap into ThemeConfigLoader as the single entry point for theme resolution - Rename deviceThemeConfigProvider to themeConfigProvider - Remove ThemeConfigLoader.load() calls from demo entry points (main_demo.dart, main_usp_demo.dart); theme now loads via provider - Simplify di.dart: remove themeConfig parameter from dependencySetup() - Remove unused BrandUtils.getDeviceTheme() from utils.dart - Update tests to use ThemeConfigLoader.forTesting() constructor - Delete obsolete brand_utils_theme_test.dart and device_theme_config_provider_test.dart
- Document the new theme loading flow and architecture - Explain ThemeConfigLoader and themeConfigProvider roles - Diagram the decision tree and override priorities
PeterJhongLinksys
requested review from
AustinChangLinksys and
HankYuLinksys
February 10, 2026 08:57
Closed
AustinChangLinksys
approved these changes
Feb 11, 2026
AustinChangLinksys
left a comment
AustinChangLinksys
left a comment
Collaborator
There was a problem hiding this comment.
looks good to me
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
2 participants
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.
Summary
Implement reactive theme system that automatically switches themes based on router model number. The system dynamically loads device-specific themes when users log into different router models, with support for CI/CD environment overrides.
Changes
Core Implementation
Technical Details
lib/providers/device_theme_config_provider.dart: FutureProvider with forcedSource priority checklib/utils.dart: BrandUtils.getDeviceTheme() with TB/CF/DU model mappinglib/app.dart: AsyncValue.when() for loading states, extracted _buildMaterialApp helperlib/main.dart: Simplified to register default theme as fallback onlylib/di.dart: Updated documentation explaining theme system migrationbuild_web.sh: Support THEME_SOURCE and THEME_JSON parameters for CI/CDTheme Mappings
assets/theme/theme_tb.jsonassets/theme/theme_cf.jsonassets/theme/theme_du.jsonTest Plan
Architecture Compliance
✅ All changes comply with project constitution:
Breaking Changes
None. The implementation preserves backward compatibility:
Migration Notes
🤖 Generated with Claude Code