-
-
Notifications
You must be signed in to change notification settings - Fork 257
Fix oauth providers with no color #2044
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
📝 WalkthroughWalkthroughThree files handling OAuth actions now compute color values once before use. A $color variable pre-processes hex color strings into Color objects if valid, or null otherwise, replacing inline Color::hex() calls in action configurations across user and authentication pages. Changes
Pre-merge checks❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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: 0
🧹 Nitpick comments (1)
app/Filament/Pages/Auth/Login.php (1)
82-86: Consider extracting the duplicated color-handling pattern.This color computation pattern is duplicated across three files:
app/Filament/Pages/Auth/Login.php(lines 79-80)app/Filament/Pages/Auth/EditProfile.php(lines 184-185)app/Filament/Admin/Resources/Users/UserResource.php(lines 303-304)💡 Optional refactor to reduce duplication
Consider adding a helper method to the
OAuthSchemaInterfaceor a utility class:// In OAuthSchemaInterface or a helper class public function getColorForAction(): ?Color { $hexColor = $this->getHexColor(); return is_string($hexColor) ? Color::hex($hexColor) : null; }Then usage becomes:
-$color = $schema->getHexColor(); -$color = is_string($color) ? Color::hex($color) : null; - $actions[] = Action::make("oauth_$id") ->label($schema->getName()) ->icon($schema->getIcon()) - ->color($color) + ->color($schema->getColorForAction()) ->url(route('auth.oauth.redirect', ['driver' => $id], false));
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/Filament/Admin/Resources/Users/UserResource.phpapp/Filament/Pages/Auth/EditProfile.phpapp/Filament/Pages/Auth/Login.php
🧰 Additional context used
🧬 Code graph analysis (3)
app/Filament/Admin/Resources/Users/UserResource.php (1)
app/Filament/Components/Tables/Columns/Concerns/HasProgress.php (1)
color(97-102)
app/Filament/Pages/Auth/EditProfile.php (8)
app/Filament/Components/Tables/Columns/Concerns/HasProgress.php (1)
color(97-102)app/Extensions/OAuth/Schemas/AuthentikSchema.php (1)
getHexColor(79-82)app/Extensions/OAuth/OAuthSchemaInterface.php (1)
getHexColor(32-32)app/Extensions/OAuth/Schemas/CommonSchema.php (1)
getHexColor(35-38)app/Extensions/OAuth/Schemas/DiscordSchema.php (1)
getHexColor(50-53)app/Extensions/OAuth/Schemas/GithubSchema.php (1)
getHexColor(50-53)app/Extensions/OAuth/Schemas/GitlabSchema.php (1)
getHexColor(61-64)app/Extensions/OAuth/Schemas/SteamSchema.php (1)
getHexColor(67-70)
app/Filament/Pages/Auth/Login.php (4)
app/Filament/Components/Tables/Columns/Concerns/HasProgress.php (1)
color(97-102)app/Extensions/OAuth/OAuthSchemaInterface.php (3)
getHexColor(32-32)getName(12-12)getIcon(30-30)app/Extensions/OAuth/Schemas/CommonSchema.php (3)
getHexColor(35-38)getName(20-23)getIcon(30-33)app/Extensions/OAuth/Schemas/GithubSchema.php (2)
getHexColor(50-53)getIcon(45-48)
🔇 Additional comments (3)
app/Filament/Pages/Auth/EditProfile.php (1)
184-185: LGTM! Consistent null-safety handling.The color handling here matches the pattern introduced in
Login.php, providing the same null-safety protection for OAuth schemas without colors.Also applies to: 192-192
app/Filament/Admin/Resources/Users/UserResource.php (1)
303-304: LGTM! Completes the null-safety fix across all OAuth actions.This implementation matches the pattern in the other files, ensuring OAuth providers without colors are handled safely in the admin user resource as well.
Also applies to: 310-310
app/Filament/Pages/Auth/Login.php (1)
79-80: Good null-safety fix for OAuth providers without colors.The precomputed
$colorvariable properly handles cases wheregetHexColor()returns null. Filament'sAction::color()method accepts null values, so passing null when no color is available is safe and correct.
No description provided.