Replace icon strings with enum #2113
Open
+6,719
−515
Conversation
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
📝 WalkthroughWalkthroughAdds a new Artisan command to generate a TablerIcon PHP enum from vendor SVGs and converts existing string-based Tabler icon usages to a BackedEnum-based Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer (runs Artisan)
participant CLI as Artisan Console
participant Vendor as vendor SVG dir
participant FS as Filesystem
participant Enum as app/Enums/TablerIcon.php
Dev->>CLI: php artisan dev:generate-tabler-icons-enum
CLI->>Vendor: scan for .svg files
CLI->>CLI: filter duplicates and variants\nsanitize & build enum cases
CLI->>FS: write enum content to `app/Enums/TablerIcon.php`
FS-->>CLI: write result
CLI->>Dev: info("Wrote app/Enums/TablerIcon.php")
Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 4
🤖 Fix all issues with AI agents
In `@app/Enums/BackupStatus.php`:
- Line 5: The BackupStatus enum references TablerIcon but does not import it;
update the BackupStatus.php enum to either add an explicit import for
App\Enums\TablerIcon (so BackupStatus can use TablerIcon by name) or change the
TablerIcon references to the fully-qualified name (\App\Enums\TablerIcon);
locate the enum declaration (BackupStatus) and adjust the use/import accordingly
so the TablerIcon symbol resolves at runtime.
In `@app/Enums/ContainerStatus.php`:
- Line 5: The getIcon() implementations in the ContainerStatus enum (and
similarly in BackupStatus, ServerState, PluginStatus, PluginCategory,
WebhookType) currently return a BackedEnum instance which violates Filament's
HasIcon contract expecting string|null; update each enum's getIcon() to return
the icon's string value and adjust the method signature to public function
getIcon(): ?string (or public function getIcon(): string if null isn't used),
i.e., extract and return the enum's backed string (use ->value or return the
mapped string directly) so the return type matches HasIcon.
In `@app/Enums/SubuserPermission.php`:
- Around line 72-74: In getIcon(), remove the unused $permission binding from
the destructuring of $this->split(); replace "[$group, $permission] =
$this->split()" with a form that only captures the first value (e.g.,
destructure to [$group] = $this->split() or assign $group = $this->split()[0])
so $permission is not declared and static analysis warnings are eliminated.
In `@app/Extensions/OAuth/OAuthSchemaInterface.php`:
- Line 6: The CommonSchema::getIcon() signature is narrower than its parent
OAuthSchema; change CommonSchema::getIcon() to return null|string|BackedEnum to
match the parent. Update the method declaration in the CommonSchema class (and
any implemented interface reference in OAuthSchemaInterface if present) to use
the union return type null|string|BackedEnum and adjust any phpdoc if necessary
so the signature and docs align with OAuthSchema::getIcon().
🧹 Nitpick comments (3)
app/Enums/PluginCategory.php (1)
5-5: Consider adding explicit import forTablerIconfor consistency.While
TablerIconis in the sameApp\Enumsnamespace and will resolve correctly, other files in this PR explicitly import it. Adding the import would improve consistency and make the dependency more obvious.Suggested change
use BackedEnum; +use App\Enums\TablerIcon; use Filament\Support\Contracts\HasIcon;app/Filament/Admin/Resources/Nodes/Pages/CreateNode.php (1)
43-53: Inconsistent icon string in Blade template.The wizard navigation icons are correctly migrated to
TablerIconenum constants, but thesubmitActionstill uses a hardcoded string'tabler-file-plus'in the Blade template. Consider extracting this to maintain consistency with the enum migration.♻️ Suggested approach
Since this is an inline Blade template, you could pass the icon as a variable:
- ->submitAction(new HtmlString(Blade::render(<<<'BLADE' + ->submitAction(new HtmlString(Blade::render(<<<'BLADE' <x-filament::icon-button type="submit" iconSize="xl" - icon="tabler-file-plus" + :icon="$icon" > {{ trans('admin/node.create') }} </x-filament::icon-button> - BLADE))), + BLADE, ['icon' => TablerIcon::FilePlus->value]))),Alternatively, keep the string if Blade components don't support enum icons directly.
app/Filament/Admin/Resources/Servers/Pages/EditServer.php (1)
263-265: Remaining string literal for dynamic dice icon is acceptable.The dynamic icon construction
'tabler-dice-' . random_int(1, 6)cannot easily use the enum since it requires runtime concatenation. If you want full enum consistency in the future, consider using a match expression with individualTablerIcon::Dice1throughTablerIcon::Dice6cases, but the current approach is simpler and works correctly.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
No description provided.