System Color Configurations #23
Conversation
WalkthroughThe pull request adds a color settings feature to the Filament Settings Hub plugin. A new Changes
Sequence DiagramsequenceDiagram
participant Admin as Administrator
participant Plugin as FilamentSettingsHubPlugin
participant Feature as allowColorSettings<br/>(Feature Flag)
participant Page as ColorSettings Page
participant Model as SitesSettings Model
Admin->>Plugin: Initialize plugin with allowColorSettings(true)
Plugin->>Feature: Store allowColorSettings flag
rect rgb(200, 220, 255)
Note over Plugin,Feature: Plugin Boot Phase
Plugin->>Feature: Check isColorSettingAllowed()
Feature-->>Plugin: Flag enabled?
alt Flag is enabled
Plugin->>Page: Register ColorSettings page
Plugin->>Model: Register SitesSettings holder
else Flag is disabled
Note over Plugin: ColorSettings skipped
end
end
rect rgb(220, 200, 255)
Note over Admin,Model: Runtime: Admin accesses colors
Admin->>Page: Navigate to Color Settings
Page->>Model: Load existing site colors
Model-->>Page: Return primary, secondary, tertiary
Page->>Admin: Display color picker form
Admin->>Page: Update colors & save
Page->>Model: Persist colors
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes The changes introduce a cohesive new feature (color settings) spanning plugin configuration, a new page class, data model extensions, and translations. While individual changes follow existing patterns, the heterogeneity across multiple file types and the addition of new logic paths (feature flag handling, new page registration) warrant moderate review attention to verify proper integration and consistency. Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/Pages/ColorSettings.php (1)
57-57: Consider making color fields optional instead of required.Since the underlying properties in
SitesSettingsare nullable (lines 27-29 insrc/Settings/SitesSettings.php), requiring values in the form may be inconsistent with the data model design. If system colors are truly optional, consider removing->required()from all three fields to allow users to leave colors unset.If colors should be optional:
ColorPicker::make('site_primary_color') ->label(trans('filament-settings-hub::messages.settings.color.form.primary_color')) ->hint(config('filament-settings-hub.show_hint') ? 'setting("site_primary_color")' : null) - ->required(), ColorPicker::make('site_secondary_color') ->label(trans('filament-settings-hub::messages.settings.color.form.secondary_color')) ->hint(config('filament-settings-hub.show_hint') ? 'setting("site_secondary_color")' : null) - ->required(), ColorPicker::make('site_tertiary_color') ->label(trans('filament-settings-hub::messages.settings.color.form.tertiary_color')) ->hint(config('filament-settings-hub.show_hint') ? 'setting("site_tertiary_color")' : null) - ->required(),Also applies to: 61-61, 65-65
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
README.md(2 hunks)resources/lang/en/messages.php(1 hunks)src/FilamentSettingsHubPlugin.php(6 hunks)src/Pages/ColorSettings.php(1 hunks)src/Settings/SitesSettings.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/FilamentSettingsHubPlugin.php (3)
src/Facades/FilamentSettingsHub.php (1)
FilamentSettingsHub(13-19)src/Pages/ColorSettings.php (1)
ColorSettings(15-69)src/Services/Contracts/SettingHold.php (8)
SettingHold(5-175)make(50-53)page(89-94)order(72-77)label(82-87)icon(119-124)description(99-104)group(109-114)
src/Pages/ColorSettings.php (1)
src/Settings/SitesSettings.php (1)
SitesSettings(7-36)
🔇 Additional comments (9)
src/Settings/SitesSettings.php (1)
27-29: LGTM! Color properties correctly defined.The three new nullable color properties follow the existing naming convention and are appropriately typed for storing color values.
README.md (1)
52-52: LGTM! Documentation correctly shows the new configuration option.The README properly documents the new
allowColorSettings()method in the plugin configuration example.src/FilamentSettingsHubPlugin.php (5)
9-9: LGTM! Import and property follow existing pattern.The ColorSettings import and feature flag property are consistent with the existing site and social menu settings.
Also applies to: 22-22
54-59: LGTM! Feature flag setter follows established pattern.The
allowColorSettings()method correctly follows the same implementation pattern asallowSiteSettings()andallowSocialMenuSettings().
71-74: LGTM! Getter correctly evaluates the closure.The
isColorSettingAllowed()method properly usesevaluate()to handle both boolean and Closure values, matching the existing pattern.
93-95: LGTM! Conditional registration implemented correctly.The ColorSettings page is correctly registered only when the feature flag is enabled, following the same pattern as other settings pages.
126-134: LGTM! Boot logic correctly registers the settings card.The ColorSettings SettingHold is properly configured with appropriate metadata and only registered when enabled.
src/Pages/ColorSettings.php (2)
15-26: LGTM! Page structure and metadata properly configured.The ColorSettings page correctly extends SettingsPage, binds to SitesSettings, and provides appropriate title and navigation settings.
28-40: LGTM! Back action correctly handles tenant-aware routing.The conditional routing logic properly supports both tenant and non-tenant panel configurations.
| Open your Laravel project’s `composer.json` and the bellow codes | ||
|
|
||
| ```text | ||
| "repositories": [ | ||
| { | ||
| "type": "vcs", | ||
| "url": "https://github.com/gkid-693/filament-settings-hub.git" | ||
| } | ||
| ], | ||
| "require": { | ||
| "tomatophp/filament-settings-hub": "dev-master" | ||
| } | ||
| ``` | ||
|
|
||
| ```bash | ||
| composer require tomatophp/filament-settings-hub | ||
| composer update tomatophp/filament-settings-hub --with-all-dependencies |
There was a problem hiding this comment.
Remove fork-specific installation instructions.
The installation instructions reference a personal fork (gkid-693/filament-settings-hub) rather than the official repository. This should not be merged into the main branch as it will direct users to install from the wrong source.
For the official release, users should install directly via Composer from Packagist:
Replace lines 21-37 with standard installation instructions:
-Open your Laravel project's `composer.json` and the bellow codes
-
-```text
-"repositories": [
- {
- "type": "vcs",
- "url": "https://github.com/gkid-693/filament-settings-hub.git"
- }
-],
-"require": {
- "tomatophp/filament-settings-hub": "dev-master"
-}
-
-```
-
```bash
-composer update tomatophp/filament-settings-hub --with-all-dependencies
+composer require tomatophp/filament-settings-hub
<details>
<summary>🤖 Prompt for AI Agents</summary>
In README.md around lines 21 to 37, the installation instructions point to a
personal fork (gkid-693) and include a custom composer.json snippet and composer
update command; remove the fork-specific "repositories" block and the dev-master
requirement and replace them with a standard Packagist installation flow: delete
the fork URL and the custom require entry and update the instructions to tell
users to install via composer require tomatophp/filament-settings-hub (and
remove the composer update --with-all-dependencies line), ensuring the README
references the official package on Packagist rather than a VCS fork.
</details>
<!-- This is an auto-generated comment by CodeRabbit -->
| 'description' => 'Manage your colors', | ||
| 'form' => [ | ||
| 'primary_color' => 'Primary Color', | ||
| 'secondary_color' => 'Second Color', |
There was a problem hiding this comment.
Fix translation typo: "Second Color" → "Secondary Color".
The label should read "Secondary Color" to match the property name and maintain consistency with "Primary Color" and "Tertiary Color".
Apply this diff:
- 'secondary_color' => 'Second Color',
+ 'secondary_color' => 'Secondary Color',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 'secondary_color' => 'Second Color', | |
| 'secondary_color' => 'Secondary Color', |
🤖 Prompt for AI Agents
In resources/lang/en/messages.php around line 44, the translation value for
'secondary_color' is incorrect ("Second Color"); update it to "Secondary Color"
to match the key and remain consistent with "Primary Color" and "Tertiary
Color".
| ColorPicker::make('site_primary_color') | ||
| ->label(trans('filament-settings-hub::messages.settings.color.form.primary_color')) | ||
| ->hint(config('filament-settings-hub.show_hint') ? 'setting("primary_color")' : null) | ||
| ->required(), | ||
| ColorPicker::make('site_secondary_color') | ||
| ->label(trans('filament-settings-hub::messages.settings.color.form.secondary_color')) | ||
| ->hint(config('filament-settings-hub.show_hint') ? 'setting("secondary_color")' : null) | ||
| ->required(), | ||
| ColorPicker::make('site_tertiary_color') | ||
| ->label(trans('filament-settings-hub::messages.settings.color.form.tertiary_color')) | ||
| ->hint(config('filament-settings-hub.show_hint') ? 'setting("tertiary_color")' : null) | ||
| ->required(), |
There was a problem hiding this comment.
CRITICAL: Hint property names don't match actual setting keys.
The hints display incorrect property names without the site_ prefix. Since the properties in SitesSettings are named site_primary_color, site_secondary_color, and site_tertiary_color, the hints should reflect the correct setting keys.
Apply this diff to fix the hints:
ColorPicker::make('site_primary_color')
->label(trans('filament-settings-hub::messages.settings.color.form.primary_color'))
- ->hint(config('filament-settings-hub.show_hint') ? 'setting("primary_color")' : null)
+ ->hint(config('filament-settings-hub.show_hint') ? 'setting("site_primary_color")' : null)
->required(),
ColorPicker::make('site_secondary_color')
->label(trans('filament-settings-hub::messages.settings.color.form.secondary_color'))
- ->hint(config('filament-settings-hub.show_hint') ? 'setting("secondary_color")' : null)
+ ->hint(config('filament-settings-hub.show_hint') ? 'setting("site_secondary_color")' : null)
->required(),
ColorPicker::make('site_tertiary_color')
->label(trans('filament-settings-hub::messages.settings.color.form.tertiary_color'))
- ->hint(config('filament-settings-hub.show_hint') ? 'setting("tertiary_color")' : null)
+ ->hint(config('filament-settings-hub.show_hint') ? 'setting("site_tertiary_color")' : null)
->required(),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ColorPicker::make('site_primary_color') | |
| ->label(trans('filament-settings-hub::messages.settings.color.form.primary_color')) | |
| ->hint(config('filament-settings-hub.show_hint') ? 'setting("primary_color")' : null) | |
| ->required(), | |
| ColorPicker::make('site_secondary_color') | |
| ->label(trans('filament-settings-hub::messages.settings.color.form.secondary_color')) | |
| ->hint(config('filament-settings-hub.show_hint') ? 'setting("secondary_color")' : null) | |
| ->required(), | |
| ColorPicker::make('site_tertiary_color') | |
| ->label(trans('filament-settings-hub::messages.settings.color.form.tertiary_color')) | |
| ->hint(config('filament-settings-hub.show_hint') ? 'setting("tertiary_color")' : null) | |
| ->required(), | |
| ColorPicker::make('site_primary_color') | |
| ->label(trans('filament-settings-hub::messages.settings.color.form.primary_color')) | |
| ->hint(config('filament-settings-hub.show_hint') ? 'setting("site_primary_color")' : null) | |
| ->required(), | |
| ColorPicker::make('site_secondary_color') | |
| ->label(trans('filament-settings-hub::messages.settings.color.form.secondary_color')) | |
| ->hint(config('filament-settings-hub.show_hint') ? 'setting("site_secondary_color")' : null) | |
| ->required(), | |
| ColorPicker::make('site_tertiary_color') | |
| ->label(trans('filament-settings-hub::messages.settings.color.form.tertiary_color')) | |
| ->hint(config('filament-settings-hub.show_hint') ? 'setting("site_tertiary_color")' : null) | |
| ->required(), |
🤖 Prompt for AI Agents
In src/Pages/ColorSettings.php around lines 54 to 65, the ColorPicker hint
strings reference the wrong setting keys (they lack the "site_" prefix); update
each hint to use the correct keys: 'setting("site_primary_color")',
'setting("site_secondary_color")', and 'setting("site_tertiary_color")' while
preserving the existing config('filament-settings-hub.show_hint') conditional
and required() calls.
1 participant
This adds system color configuration to your project, allowing you to define primary, secondary, and tertiary colors.
Summary by CodeRabbit
New Features
allowColorSettings()option in plugin initialization.Documentation
filament-settings-hub:install).