Refactor: add FilamentServiceProvider & globally make Select native(false)
#1836
Conversation
📝 WalkthroughWalkthroughCentralizes Filament/Livewire integrations in a new FilamentServiceProvider, removes individual Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Laravel as Laravel Boot
participant Providers as Registered Providers
participant FilamentProv as FilamentServiceProvider
participant AppProv as AppServiceProvider
participant Livewire as Livewire Runtime
participant Forms as Filament Forms
Note over Laravel,Providers: Application boot sequence
Laravel->>Providers: instantiate & boot providers
Providers->>FilamentProv: call boot()
activate FilamentProv
FilamentProv->>FilamentProv: register color mappings
FilamentProv->>FilamentProv: register Blade render hooks (page start/footer/styles/scripts)
FilamentProv->>Livewire: attach dehydrate listener (alertBanner dispatch)
FilamentProv->>FilamentProv: Field::macro(hintCopy)
FilamentProv->>FilamentProv: Select::configureUsing(native(false))
deactivate FilamentProv
Providers->>AppProv: call boot()
activate AppProv
AppProv->>AppProv: (no Filament/Livewire hooks present)
deactivate AppProv
Note over Forms,FilamentProv: Runtime form rendering
Forms->>FilamentProv: resolve Select config
FilamentProv-->>Forms: apply global Select config (non-native by default)
Livewire->>Forms: dehydrate -> Livewire listener may dispatch alertBanner
Pre-merge checks❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)bootstrap/providers.php (1)
🔇 Additional comments (1)
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/Extensions/Features/Schemas/JavaVersionSchema.php (1)
54-55: Options mapping likely reversed (bug).mapWithKeys closure args are ($value, $key). Current fn ($key, $value) flips label/value, breaking default/notIn matching.
Apply one of these fixes:
- ->options(fn () => collect($server->egg->docker_images)->mapWithKeys(fn ($key, $value) => [$key => $value])) + ->options(fn () => collect($server->egg->docker_images)->mapWithKeys(fn ($value, $key) => [$key => $value]))Or simplify if already associative:
- ->options(fn () => collect($server->egg->docker_images)->mapWithKeys(fn ($key, $value) => [$key => $value])) + ->options(fn () => $server->egg->docker_images)
🧹 Nitpick comments (3)
app/Filament/Admin/Resources/Users/UserResource.php (1)
254-276: Optional: align avatar upload constraints with EditProfile.EditProfile restricts to PNG via acceptedFileTypes(['image/png']) and sets a deterministic filename; here it’s looser and visible only when a PNG already exists. If consistency is desired, mirror the restriction.
Example:
FileUpload::make('avatar') + ->acceptedFileTypes(['image/png']) ->directory('avatars') ->disk('public')app/Providers/Filament/FilamentServiceProvider.php (2)
38-40: Prefer component alias in Livewire render hook for compatibility.@livewire(\FQCN::class) can be brittle across Livewire versions/parsers. Use the component alias/tag.
- fn () => Blade::render('@livewire(\App\Livewire\AlertBannerContainer::class)'), + fn () => Blade::render("@livewire('alert-banner-container')"), + // or: Blade::render('<livewire:alert-banner-container />')
57-71: Dehydrate listener is fine; small guard suggestion.Dispatching for every component may be redundant. Optionally gate to root component to reduce event noise.
Example:
- on('dehydrate', function (Component $component) { + on('dehydrate', function (Component $component) { + if (method_exists($component, 'isRoot') && !$component->isRoot()) { + return; + } // existing checks...
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
app/Extensions/Features/Schemas/JavaVersionSchema.php(1 hunks)app/Filament/Admin/Pages/Settings.php(0 hunks)app/Filament/Admin/Resources/Eggs/Pages/CreateEgg.php(0 hunks)app/Filament/Admin/Resources/Eggs/Pages/EditEgg.php(0 hunks)app/Filament/Admin/Resources/Users/UserResource.php(2 hunks)app/Filament/Components/Forms/Fields/CopyFrom.php(0 hunks)app/Filament/Pages/Auth/EditProfile.php(2 hunks)app/Filament/Server/Resources/Files/Pages/ListFiles.php(0 hunks)app/Filament/Server/Resources/Schedules/ScheduleResource.php(1 hunks)app/Livewire/Installer/PanelInstaller.php(0 hunks)app/Providers/AppServiceProvider.php(0 hunks)app/Providers/Filament/FilamentServiceProvider.php(1 hunks)bootstrap/providers.php(1 hunks)
💤 Files with no reviewable changes (7)
- app/Filament/Components/Forms/Fields/CopyFrom.php
- app/Filament/Admin/Resources/Eggs/Pages/EditEgg.php
- app/Filament/Admin/Pages/Settings.php
- app/Livewire/Installer/PanelInstaller.php
- app/Filament/Admin/Resources/Eggs/Pages/CreateEgg.php
- app/Providers/AppServiceProvider.php
- app/Filament/Server/Resources/Files/Pages/ListFiles.php
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-07T14:44:18.583Z
Learnt from: notAreYouScared
PR: pelican-dev/panel#1779
File: app/Filament/Admin/Resources/Users/Pages/EditUser.php:51-51
Timestamp: 2025-10-07T14:44:18.583Z
Learning: In the Pelican Panel codebase, when using Filament's FileUpload component for the avatar field in UserResource, the 'avatar' key is intentionally unset from the data array in EditUser::handleRecordUpdate before passing to UserUpdateService. This is by design because the avatar is not stored in the database directly—Filament's FileUpload component handles file storage, retrieval, and deletion through its own lifecycle hooks (formatStateUsing, deleteUploadedFileUsing, etc.) independently of the database update service.
Applied to files:
app/Filament/Admin/Resources/Users/UserResource.php
🧬 Code graph analysis (3)
bootstrap/providers.php (1)
app/Providers/Filament/FilamentServiceProvider.php (1)
FilamentServiceProvider(20-82)
app/Filament/Pages/Auth/EditProfile.php (1)
app/Services/Helpers/LanguageService.php (2)
LanguageService(8-30)getAvailableLanguages(22-29)
app/Filament/Admin/Resources/Users/UserResource.php (1)
app/Services/Helpers/LanguageService.php (2)
LanguageService(8-30)getAvailableLanguages(22-29)
🔇 Additional comments (7)
app/Extensions/Features/Schemas/JavaVersionSchema.php (1)
59-59: LGTM: relying on global Select native(false).Keeping preload() here is good for large option sets. No further changes needed.
app/Filament/Pages/Auth/EditProfile.php (1)
135-135: LGTM: native(false) removal aligns with global config.Selects remain searchable; behavior stays consistent via FilamentServiceProvider.
Also applies to: 146-146
app/Filament/Admin/Resources/Users/UserResource.php (1)
239-239: LGTM: Selects rely on global native(false).No functional change; searchable and options logic preserved.
Also applies to: 254-254
app/Filament/Server/Resources/Schedules/ScheduleResource.php (1)
258-258: LGTM: consistent with global Select configuration.selectablePlaceholder(false) remains; UX unchanged except for unified non‑native rendering.
bootstrap/providers.php (1)
8-8: No duplicate registrations found—placement is correct.The verification confirms all Filament global configurations (color scheme, render hooks, macros, Select settings) are properly centralized in the single
FilamentServiceProvider::boot()method with no redundant registrations elsewhere. Loading this provider early ensures hooks and macros initialize before panel providers depend on them.app/Providers/Filament/FilamentServiceProvider.php (2)
78-78: Global Select native(false) set correctly — but manual verification needed on opt-in capability.The search for
native(true)overrides returned no matches. This may be expected if no fields currently require native behavior, or if the opt-in mechanism hasn't been exercised yet. Confirm that:
- Fields can still opt-in via
->native(true)when needed (e.g., mobile, large lists)- The codebase is in the intended state post-deployment
48-55: Resolve — Vite hooks won’t double‑load app assets.Search found only the @Vite calls for app.css/js in app/Providers/Filament/FilamentServiceProvider.php (the two render hooks) and a docs view (resources/views/docs/api-index.blade.php); Filament render hooks are additive — no other inclusions of resources/css/app.css or resources/js/app.js were found, so these hooks will not cause duplicate loads in panel layouts.
Closes #1441