Walkthrough

Dynamic color handling and settings for the app color seed were updated: ThemeData now uses colorSchemeSeed and app bars set scrolledUnderElevation = 0.0; settings pre-fill/validate the seed editor and conditionally show "Follow System". A card height constant was increased.

Changes

Sequence Diagram(s)

sequenceDiagram
    participant App
    participant Stores as Stores.setting
    participant ThemeBuilder as _buildDynamicColor/_build
    participant UIState as UIs
    participant SettingsUI as AppSettings

    App->>Stores: read useSystemPrimaryColor
    alt system primary enabled
        App->>ThemeBuilder: _buildDynamicColor()
        ThemeBuilder->>Stores: fetch system light/dark seed (dynamic_color)
        ThemeBuilder->>ThemeBuilder: build ThemeData with colorSchemeSeed + appBarTheme(scrolledUnderElevation=0.0)
        ThemeBuilder->>UIState: set primaryColor & colorSeed
    else system primary disabled
        App->>ThemeBuilder: _build()
        ThemeBuilder->>Stores: fetch stored colorSeed
        ThemeBuilder->>UIState: set primaryColor & colorSeed
    end
    SettingsUI->>Stores: open color editor (pre-filled)
    SettingsUI->>SettingsUI: pick/validate hex → save to Stores
    SettingsUI->>UIState: update primaryColor & colorSeed (if not following system)
    UIState->>App: apply updated theme

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

• fix: code editor tool bar #933: Modifies lib/app.dart dynamic color handling and propagation of UIs.colorSeed, closely related to the ThemeData and color-seed changes here.

Poem

🐰 I found a little hex today,
I nudged the theme to find its way.
System seeds or one I keep,
App bars calm, no shadows creep.
A tiny hop, the colors play.

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 @coderabbitai help to get the list of available commands and usage tips.}

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

lib/app.dart (1)> 65-93: Potential color inconsistency when dynamic colors are unavailable.

If both light and dark are null (on platforms that don't support dynamic colors), the theme will use Flutter's default color scheme, but UIs.primaryColor and UIs.colorSeed won't be updated. This could cause inconsistency between the applied theme and UI elements that directly reference UIs.primaryColor.
Consider adding a fallback to update UIs values when dynamic colors are unavailable:

🔧 Suggested fix

         if (context.isDark && dark != null) {
           UIs.primaryColor = dark.primary;
           UIs.colorSeed = dark.primary;
         } else if (!context.isDark && light != null) {
           UIs.primaryColor = light.primary;
           UIs.colorSeed = light.primary;
+        } else {
+          // Fallback when dynamic colors unavailable
+          final fallback = Color(Stores.setting.colorSeed.fetch());
+          UIs.primaryColor = fallback;
+          UIs.colorSeed = fallback;
         }

🧹 Nitpick comments (2)

lib/app.dart (1)> 28-29: Unused fetch result appears to be dead code.

The fetch() call on line 29 is executed but its return value is discarded. If this is intended for cache warming, consider adding a comment. Otherwise, this line can be removed since fetch() is called again inside the ListenableBuilder on line 34.

🔧 Suggested fix

   _setup(context);
-
-   Stores.setting.useSystemPrimaryColor.fetch();

   return ListenableBuilder(

lib/view/page/setting/entries/app.dart (1)> 131-144: Consider also updating UIs.primaryColor for consistency.

The _onSaveColor method updates UIs.colorSeed but not UIs.primaryColor. In lib/app.dart, both values are updated together. While the subsequent RNodes.app.notify() will trigger a rebuild that updates both, there's a brief window where these values could be inconsistent.

🔧 Suggested fix

   void _onSaveColor(String s) {
     final color = s.fromColorHex;

     if (color == null) {
       context.showSnackBar(libL10n.fail);
       return;
     }

     UIs.colorSeed = color;
+    UIs.primaryColor = color;
     _setting.colorSeed.put(color.value255);

     RNodes.app.notify();
     context.pop();
   }

📜 Review details

@coderabbitai Noticed this. This might impact systems without dynamic color or light/dark switch (Android 9-, Windows, etc.) I think, but I don't know if it really matters.

But it won't cause any bugs. Investigating.

#!/bin/bash

# Find usages of UIs.primaryColor and UIs.colorSeed
echo "=== Usages of UIs.primaryColor ==="
rg -nP 'UIs\.primaryColor' -C 2

echo -e "\n\n=== Usages of UIs.colorSeed ==="
rg -nP 'UIs\.colorSeed' -C 2

# Check the UIs class definition to understand these properties
echo -e "\n\n=== UIs class definition ==="
fd -e dart -x grep -l "class UIs" | head -1 | xargs cat

# Check when _buildDynamicColor is called and how dynamic color availability is determined
echo -e "\n\n=== Dynamic color builder usage ==="
rg -nP 'DynamicColorBuilder|_buildDynamicColor' -C 3

Based on the investigation, here's what I found:

Key Findings

The concern is valid. Here's the issue:

1. Fallback exists for ThemeData but not for UIs values:

   • Line 70 in _buildDynamicColor() has: final colorSeed = lightSeed ?? darkSeed ?? Color(Stores.setting.colorSeed.fetch());
   • This ensures ThemeData gets the correct colorSchemeSeed even when dynamic colors are unavailable
   • However, UIs.primaryColor and UIs.colorSeed are only updated in lines 83-89 when light or dark are not null

2. Wide usage of UIs.primaryColor:

   • Text styling in multiple pages (SFTP, SSH, Ping)
   • Terminal cursor color
   • Chart colors (CircleChart, LineChart)
   • Progress bars
   • Settings color preview

3. The scenario where this matters:

   • User enables "Use System Primary Color" on a platform without dynamic color support
   • Both light and dark from DynamicColorBuilder will be null
   • ThemeData will correctly use the stored color seed
   • But UI elements directly referencing UIs.primaryColor will have stale/inconsistent values

Impact

On platforms like Android 9-, Windows, etc., if users toggle the "Use System Primary Color" setting:

• Theme colors will be correct
• But direct color references (charts, text highlights, etc.) may show the wrong color until the next full rebuild

The suggested fallback in the review comment would fix this inconsistency. Would you like to implement it?

I added a logic that directly hide "Follow system" feature when the device does not support this feature.

This screenshot is taken from Android 9, which does not have dynamic color support.

@coderabbitai Please review.

#!/bin/bash

# Check the current implementation in lib/view/page/setting/entries/app.dart
# to see how the "Follow system" option is conditionally displayed
echo "=== lib/view/page/setting/entries/app.dart implementation ==="
cat lib/view/page/setting/entries/app.dart

echo -e "\n\n=== lib/app.dart implementation ==="
cat lib/app.dart

@GT-610, excellent work! 🎉 Your implementation looks great. Let me break down what I found:

✅ All Major Concerns Addressed

1. Dynamic Color UI Hiding (lib/view/page/setting/entries/app.dart, lines 103-120)

Perfect implementation! The DynamicColorBuilder wrapping with the supported check elegantly hides the "Follow System" option when light and dark are both null. This prevents users from enabling a feature their device can't support.

final supported = light != null || dark != null;
if (!supported) return const SizedBox.shrink();

2. Fallback Logic (lib/app.dart, lines 91-95)

You've added exactly the fallback I suggested! When dynamic colors are unavailable, it now properly updates UIs.primaryColor and UIs.colorSeed with the stored seed:

} else {
  final fallbackColor = Color(Stores.setting.colorSeed.fetch());
  UIs.primaryColor = fallbackColor;
  UIs.colorSeed = fallbackColor;
}

This ensures consistency across the app even on platforms without dynamic color support.

3. Color Consistency (lib/view/page/setting/entries/app.dart, lines 137-138)

The _onSaveColor method now correctly updates both values:

UIs.primaryColor = color;
UIs.colorSeed = color;

🧹 Minor Remaining Nitpick

Line 29 in lib/app.dart still has an unused fetch() call:

Stores.setting.useSystemPrimaryColor.fetch();

The return value is discarded, and fetch() is called again on line 34 inside the ListenableBuilder. Unless this is intentional cache warming (in which case a comment would be helpful), this line can be removed.

Summary

Your changes comprehensively address the color inconsistency concerns on platforms without dynamic color support (Android 9-, Windows, etc.). The implementation is clean, defensive, and user-friendly. Great job! 👍

#!/bin/bash

# Fetch the current state of lib/app.dart to check if the fallback logic was added
echo "=== lib/app.dart (focusing on _buildDynamicColor method) ==="
cat lib/app.dart

echo -e "\n\n=== lib/view/page/setting/entries/app.dart (focusing on color setting UI) ==="
cat lib/view/page/setting/entries/app.dart

@GT-610: Excellent work! 🎉 Your implementation properly addresses the main concern:

✅ Issues Resolved

1. Color consistency fallback (lines 89-93 in lib/app.dart):

   } else {
     final fallbackColor = Color(Stores.setting.colorSeed.fetch());
     UIs.primaryColor = fallbackColor;
     UIs.colorSeed = fallbackColor;
   }

   This ensures UIs.primaryColor and UIs.colorSeed stay consistent with the theme even on platforms without dynamic color support.

2. UX improvement (lines 106-114 in lib/view/page/setting/entries/app.dart):
   The conditional rendering with DynamicColorBuilder elegantly hides the "Follow system" option when light and dark are both null, preventing users on Android 9-, Windows, etc. from enabling a feature their device doesn't support.

3. Color sync in manual picker (lines 133-135 in lib/view/page/setting/entries/app.dart):
   Both UIs.primaryColor and UIs.colorSeed are now updated together, maintaining consistency.

🧹 One minor nitpick remains

Line 29 in lib/app.dart still has an unused fetch() call:

Stores.setting.useSystemPrimaryColor.fetch();

The return value is discarded, and it's called again inside the ListenableBuilder on line 34. Consider removing it unless it's intentionally warming the cache (in which case, a comment would clarify intent).

The core functionality is now solid! The combination of hiding the UI option and providing a proper fallback ensures correct behavior across all platforms. 👍

Unused call problem is being fixed.