-
-
Notifications
You must be signed in to change notification settings - Fork 254
Replace CodeEditor with MonacoEditor #2013
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
📝 WalkthroughWalkthroughAdds a MonacoEditor Filament form field, Blade view, CSS, and config plus an EditorLanguages enum; replaces CodeEditor with MonacoEditor across Egg, Server, and File admin pages and wires Livewire/Alpine language and content synchronization. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Browser (Alpine / Monaco)
participant Livewire as Livewire Component
participant Filament as PHP (MonacoEditor / EditorLanguages)
participant DB as Database
Note over Browser: User edits content or changes language
Browser->>Livewire: update entangled state (content) / emit 'setLanguage'
Livewire->>Filament: update field state / request language resolution
Filament->>Filament: EditorLanguages::fromWithAlias(...) (resolve language)
Filament-->>Livewire: return field metadata (language, theme)
Livewire-->>Browser: push updated entangled state (language/content)
Note over Browser,Livewire: Monaco updates model language & content
Browser->>Livewire: submit form
Livewire->>Filament: handle save request
Filament->>DB: persist editor content/settings
DB-->>Filament: persist confirmation
Filament-->>Livewire: respond with success/failure
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (3)📓 Common learnings📚 Learning: 2025-12-17T21:38:45.317ZApplied to files:
📚 Learning: 2025-10-15T11:55:53.461ZApplied to files:
🔇 Additional comments (2)
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: 4
🧹 Nitpick comments (5)
app/Filament/Admin/Resources/Servers/Pages/EditServer.php (1)
5-6: Unused imports detected.
MonacoEditorandEditorLanguagesare imported but not used in this file. The install logs viewer at line 308 still usesCodeEditor. Either complete the migration to MonacoEditor or remove these unused imports.-use AbdelhamidErrahmouni\FilamentMonacoEditor\MonacoEditor; -use App\Enums\EditorLanguages;Or replace the
CodeEditorusage at line 308 withMonacoEditor.app/Enums/EditorLanguages.php (1)
94-135: Consider adding common file extension aliases.The
fromWithAliasmethod is missing some commonly used extensions:
bash→shelljsx→javascriptsass→scsscfg,conf→ini'sh', 'zsh' => self::shell, + 'bash' => self::shell, 'ts', 'tsx' => self::typescript, + 'jsx' => self::javascript, + 'cfg', 'conf' => self::ini, + + 'sass' => self::scss, + 'yml' => self::yaml,app/Filament/Admin/Resources/Eggs/Pages/EditEgg.php (1)
428-432: Consider adding->lazy()for consistency with CreateEgg.
CreateEgg.phpuses->lazy()on the equivalent MonacoEditor field, which defers state updates for large content. Consider adding it here for consistency and performance.MonacoEditor::make('script_install') ->hiddenLabel() ->showFullScreenToggle(false) // Kinda buggy with Filament's layout ->language(EditorLanguages::shell->value) - ->columnSpanFull(), + ->columnSpanFull() + ->lazy(),resources/views/filament/components/monaco-editor.blade.php (2)
96-101: Missing variable declaration and hardcoded CDN version repeated.
monacoLoaderIntervallacks alet/constdeclaration, creating an implicit global.- The Monaco CDN version
0.49.0is hardcoded in multiple places (lines 68, 100, 101). Consider extracting to a variable for easier maintenance.+const monacoVersion = '0.49.0'; +const monacoBasePath = `https://cdnjs.cloudflare.com/ajax/libs/monaco-editor/${monacoVersion}/min`; + -monacoLoaderInterval = setInterval(() => { +let monacoLoaderInterval = setInterval(() => { if(typeof _amdLoaderGlobal !== 'undefined'){ - require.config({ paths: { 'vs': 'https://cdnjs.cloudflare.com/ajax/libs/monaco-editor/0.49.0/min/vs' }}); - let proxy = URL.createObjectURL(new Blob([` self.MonacoEnvironment = { baseUrl: 'https://cdnjs.cloudflare.com/ajax/libs/monaco-editor/0.49.0/min' }; importScripts('https://cdnjs.cloudflare.com/ajax/libs/monaco-editor/0.49.0/min/vs/base/worker/workerMain.min.js');`], { type: 'text/javascript' })); + require.config({ paths: { 'vs': `${monacoBasePath}/vs` }}); + let proxy = URL.createObjectURL(new Blob([` self.MonacoEnvironment = { baseUrl: '${monacoBasePath}' }; importScripts('${monacoBasePath}/vs/base/worker/workerMain.min.js');`], { type: 'text/javascript' }));
52-58: Use strict equality for empty string comparison.Using
==for comparison is less strict. While functionally equivalent here,===is the JavaScript best practice.updatePlaceholder: function(value) { - if (value == '') { + if (value === '') { this.monacoPlaceholder = true; return; } this.monacoPlaceholder = false; },
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
app/Enums/EditorLanguages.php(1 hunks)app/Filament/Admin/Resources/Eggs/Pages/CreateEgg.php(2 hunks)app/Filament/Admin/Resources/Eggs/Pages/EditEgg.php(2 hunks)app/Filament/Admin/Resources/Servers/Pages/EditServer.php(1 hunks)app/Filament/Server/Resources/Files/Pages/EditFiles.php(3 hunks)app/Filament/Server/Resources/Files/Pages/ListFiles.php(2 hunks)composer.json(2 hunks)config/filament-monaco-editor.php(1 hunks)public/css/filament-monaco-editor/filament-monaco-editor-styles.css(1 hunks)resources/views/filament/components/monaco-editor.blade.php(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-10-15T11:55:53.461Z
Learnt from: rmartinoscar
Repo: pelican-dev/panel PR: 1801
File: app/Extensions/OAuth/Schemas/AuthentikSchema.php:7-10
Timestamp: 2025-10-15T11:55:53.461Z
Learning: In Filament v4, the Forms component Placeholder was deprecated and removed. Use TextEntry from Filament\Infolists\Components\TextEntry in forms instead, binding values with ->state(). For HTML content, use ->html().
Applied to files:
app/Filament/Server/Resources/Files/Pages/ListFiles.php
📚 Learning: 2025-10-15T11:55:53.461Z
Learnt from: rmartinoscar
Repo: pelican-dev/panel PR: 1801
File: app/Extensions/OAuth/Schemas/AuthentikSchema.php:7-10
Timestamp: 2025-10-15T11:55:53.461Z
Learning: In Filament v4, Wizard Step components use the Filament\Schemas namespace (Filament\Schemas\Components\Wizard\Step), not Filament\Forms.
Applied to files:
app/Filament/Server/Resources/Files/Pages/ListFiles.phpapp/Filament/Admin/Resources/Eggs/Pages/CreateEgg.phpapp/Filament/Admin/Resources/Eggs/Pages/EditEgg.php
📚 Learning: 2025-09-16T19:30:21.443Z
Learnt from: rmartinoscar
Repo: pelican-dev/panel PR: 1708
File: app/Filament/App/Resources/Servers/Pages/ListServers.php:115-116
Timestamp: 2025-09-16T19:30:21.443Z
Learning: The Filament `recordActions` method accepts both arrays and ActionGroup instances (signature: `array | ActionGroup $actions`), so ActionGroup can be passed directly without wrapping in an array.
Applied to files:
app/Filament/Server/Resources/Files/Pages/ListFiles.phpapp/Filament/Admin/Resources/Eggs/Pages/CreateEgg.phpapp/Filament/Admin/Resources/Eggs/Pages/EditEgg.php
📚 Learning: 2025-09-16T19:32:01.343Z
Learnt from: rmartinoscar
Repo: pelican-dev/panel PR: 1708
File: app/Filament/App/Resources/Servers/Pages/ListServers.php:228-266
Timestamp: 2025-09-16T19:32:01.343Z
Learning: ActionGroup in Filament automatically hides itself when all child actions are not visible, so additional hidden() logic to prevent empty groups is unnecessary.
Applied to files:
app/Filament/Admin/Resources/Eggs/Pages/CreateEgg.phpapp/Filament/Admin/Resources/Eggs/Pages/EditEgg.php
📚 Learning: 2025-11-04T08:38:56.562Z
Learnt from: Boy132
Repo: pelican-dev/panel PR: 1861
File: app/Filament/Server/Resources/Files/Pages/EditFiles.php:220-229
Timestamp: 2025-11-04T08:38:56.562Z
Learning: In the Pelican Panel codebase, the `AlertBanner::title()` and `body()` methods accept `string | Closure | null` types and handle Closure invocation internally, so Closures can be passed directly without manual invocation.
Applied to files:
app/Filament/Server/Resources/Files/Pages/EditFiles.php
🧬 Code graph analysis (1)
app/Filament/Server/Resources/Files/Pages/EditFiles.php (1)
app/Enums/EditorLanguages.php (1)
fromWithAlias(94-135)
🔇 Additional comments (6)
app/Filament/Admin/Resources/Eggs/Pages/CreateEgg.php (1)
272-277: LGTM!MonacoEditor integration is correctly configured with shell language and appropriate lazy loading for script content.
resources/views/filament/components/monaco-editor.blade.php (1)
107-109: Theme selection relies on localStorage without fallback validation.The theme selection reads directly from
localStorage.getItem('theme')and assumes it will only be'light'or something else. If localStorage contains an unexpected value or is unavailable (e.g., in private browsing on some browsers), behavior might be inconsistent.Consider adding a fallback or validating the theme value:
const storedTheme = localStorage.getItem('theme'); theme: storedTheme === 'light' ? 'iPlastic' : 'custom',Please verify this aligns with how themes are managed elsewhere in the application.
app/Filament/Server/Resources/Files/Pages/ListFiles.php (1)
499-510: LGTM! Clean integration of MonacoEditor for new file creation.The language selector is properly bound to the
EditorLanguagesenum with live updates dispatchingsetLanguageevents. The MonacoEditor configuration with disabled fullscreen toggle (with explanatory comment) and dynamic language binding is well-structured.config/filament-monaco-editor.php (1)
1-14: LGTM! Well-structured Monaco editor configuration.The general options and theme definitions follow Monaco's expected format. Both
blackboard(dark) andiPlastic(light) themes are comprehensively defined with appropriate token rules and editor colors.app/Filament/Server/Resources/Files/Pages/EditFiles.php (1)
140-148: LGTM! Consistent MonacoEditor integration with proper language resolution.The implementation correctly:
- Uses
EditorLanguages::fromWithAlias()to resolve language from file extension with a safe fallback toplaintext- Dispatches
setLanguageevent on language change for live updates- Configures MonacoEditor with a custom Blade view and dynamic language binding
This is consistent with the
ListFiles.phpimplementation. ThefromWithAlias()method robustly handles edge cases: files without extensions and unusual extensions both safely default toplaintextvia the fallbackself::tryFrom($match) ?? self::plaintextlogic.public/css/filament-monaco-editor/filament-monaco-editor-styles.css (1)
1-5: Hardcoded fullscreen offset assumes fixed sidebar width—may break with responsive layouts.The
.fme-full-screenclass hasleft:300pxandtop:64pxhardcoded, which locks the fullscreen editor position to a fixed sidebar offset. This will cause misalignment if:
- The sidebar width changes (collapsed, responsive breakpoints)
- The header height varies
- Custom layout adjustments are made
Since this CSS appears to be published from the
abdelhamiderrahmouni/filament-monaco-editorpackage, modifications here may be overwritten on package updates. If customization is needed, consider extracting these hardcoded values to CSS custom properties (e.g.,var(--fme-sidebar-width),var(--fme-header-height)) and managing them at the application level.
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: 1
🧹 Nitpick comments (1)
app/Enums/EditorLanguages.php (1)
94-135: Consider case-insensitive extension matching.The
fromWithAliasmethod is case-sensitive. Extensions like'JS'or'PY'would fall through to the default case and potentially returnplaintextinstead of their proper language mappings.Apply this diff to handle case-insensitive matching:
public static function fromWithAlias(string $match): self { + $match = strtolower($match); return match ($match) { 'h' => self::c,
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/Enums/EditorLanguages.php(1 hunks)app/Filament/Admin/Resources/Eggs/Pages/CreateEgg.php(2 hunks)app/Filament/Admin/Resources/Eggs/Pages/EditEgg.php(2 hunks)app/Filament/Admin/Resources/Servers/Pages/EditServer.php(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- app/Filament/Admin/Resources/Eggs/Pages/CreateEgg.php
- app/Filament/Admin/Resources/Eggs/Pages/EditEgg.php
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-09-16T19:30:21.443Z
Learnt from: rmartinoscar
Repo: pelican-dev/panel PR: 1708
File: app/Filament/App/Resources/Servers/Pages/ListServers.php:115-116
Timestamp: 2025-09-16T19:30:21.443Z
Learning: The Filament `recordActions` method accepts both arrays and ActionGroup instances (signature: `array | ActionGroup $actions`), so ActionGroup can be passed directly without wrapping in an array.
Applied to files:
app/Filament/Admin/Resources/Servers/Pages/EditServer.php
📚 Learning: 2025-10-15T11:55:53.461Z
Learnt from: rmartinoscar
Repo: pelican-dev/panel PR: 1801
File: app/Extensions/OAuth/Schemas/AuthentikSchema.php:7-10
Timestamp: 2025-10-15T11:55:53.461Z
Learning: In Filament v4, Wizard Step components use the Filament\Schemas namespace (Filament\Schemas\Components\Wizard\Step), not Filament\Forms.
Applied to files:
app/Filament/Admin/Resources/Servers/Pages/EditServer.php
📚 Learning: 2025-09-16T19:32:01.343Z
Learnt from: rmartinoscar
Repo: pelican-dev/panel PR: 1708
File: app/Filament/App/Resources/Servers/Pages/ListServers.php:228-266
Timestamp: 2025-09-16T19:32:01.343Z
Learning: ActionGroup in Filament automatically hides itself when all child actions are not visible, so additional hidden() logic to prevent empty groups is unnecessary.
Applied to files:
app/Filament/Admin/Resources/Servers/Pages/EditServer.php
🔇 Additional comments (3)
app/Enums/EditorLanguages.php (2)
9-92: LGTM! Comprehensive language support.The enum provides extensive language coverage for the Monaco editor. The typo mentioned in previous review comments (
coljure→clojure) has been properly corrected.
137-140: LGTM! Clean implementation.The
getLabel()method correctly implements theHasLabelinterface by returning the enum case name, providing human-readable labels for the editor languages.app/Filament/Admin/Resources/Servers/Pages/EditServer.php (1)
306-309: Custom Monaco editor view is properly configured.The MonacoEditor configuration correctly references the custom view at
resources/views/filament/components/monaco-editor.blade.php, which exists. The implementation properly applieshiddenLabel(), disables fullscreen toggle with a documented comment about Filament layout compatibility, and maintains all other configurations.
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: 2
♻️ Duplicate comments (3)
resources/views/filament/components/monaco-editor.blade.php (3)
97-135: Additional implicit global variables in the interval callback.Similar to the
scriptvariable issue,monacoLoaderInterval(line 97) andmonacoLoader(line 133) are assigned withoutletorconst, creating implicit globals.- monacoLoaderInterval = setInterval(() => { + let monacoLoaderInterval = setInterval(() => {And for line 133:
- clearInterval(monacoLoaderInterval); - monacoLoader = false; + clearInterval(monacoLoaderInterval); + this.monacoLoader = false;Note:
monacoLoadershould referencethis.monacoLoadersince it's defined as an Alpine data property.
61-65: CustomEvent detail should use thedetailproperty.This issue was previously flagged. Custom event data should be passed via the
detailproperty.
67-71: Missing variable declaration creates implicit global.This issue was previously flagged. The
scriptvariable needsconstdeclaration.
🧹 Nitpick comments (5)
resources/views/filament/components/monaco-editor.blade.php (1)
101-103: Consider pinning Monaco version in a single location.The Monaco version
0.49.0is hardcoded in multiple places (lines 69, 101, 102). Consider extracting this to a configuration value or PHP variable to simplify version updates.app/Filament/Components/Forms/Fields/MonacoEditor.php (1)
236-241: MethodhideFullScreenButton()is missing return type declaration.For consistency with other fluent methods, add the
: staticreturn type.- public function hideFullScreenButton() + public function hideFullScreenButton(): static { $this->showFullScreenToggle = false; return $this; }resources/css/monaco-editor.css (1)
1-5: Minified CSS for Monaco editor styling.This appears to be compiled/minified CSS output. Consider documenting the source SCSS/CSS file or build process if this is generated, to aid future maintenance.
app/Filament/Admin/Resources/Eggs/Pages/EditEgg.php (1)
428-433: Good implementation with one optional cleanup.The MonacoEditor configuration is correct and appropriate for an install script editor. The shell language selection makes sense for this context, and the comment explains why fullscreen is disabled.
The
->view('filament.components.monaco-editor')call is redundant since MonacoEditor already defines this as its default view. Consider removing it for cleaner code:MonacoEditor::make('script_install') ->hiddenLabel() - ->view('filament.components.monaco-editor') ->showFullScreenToggle(false) // Kinda buggy with Filament's layout ->language(EditorLanguages::shell->value) ->columnSpanFull(),app/Filament/Server/Resources/Files/Pages/EditFiles.php (1)
144-149: Correct reactive binding with one optional cleanup.The MonacoEditor implementation correctly uses reactive language binding with
->language(fn (Get $get) => $get('lang')), which works seamlessly with the Select field's->live()modifier above.Similar to EditEgg.php, the
->view('filament.components.monaco-editor')call is redundant since this is MonacoEditor's default view:MonacoEditor::make('editor') ->hiddenLabel() ->language(fn (Get $get) => $get('lang')) - ->view('filament.components.monaco-editor') ->showFullScreenToggle(false) // Kinda buggy with Filament's layout
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
app/Filament/Admin/Resources/Eggs/Pages/CreateEgg.php(2 hunks)app/Filament/Admin/Resources/Eggs/Pages/EditEgg.php(2 hunks)app/Filament/Admin/Resources/Servers/Pages/EditServer.php(2 hunks)app/Filament/Components/Forms/Fields/MonacoEditor.php(1 hunks)app/Filament/Server/Resources/Files/Pages/EditFiles.php(3 hunks)app/Filament/Server/Resources/Files/Pages/ListFiles.php(2 hunks)public/vendor/filament-monaco-editor/filament-monaco-editor.css(1 hunks)public/vendor/filament-monaco-editor/filament-monaco-editor.js(1 hunks)resources/css/monaco-editor.css(1 hunks)resources/views/filament/components/monaco-editor.blade.php(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- public/vendor/filament-monaco-editor/filament-monaco-editor.js
- public/vendor/filament-monaco-editor/filament-monaco-editor.css
🚧 Files skipped from review as they are similar to previous changes (1)
- app/Filament/Admin/Resources/Eggs/Pages/CreateEgg.php
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: notAreYouScared
Repo: pelican-dev/panel PR: 2013
File: app/Filament/Admin/Resources/Servers/Pages/EditServer.php:5-5
Timestamp: 2025-12-17T21:38:40.755Z
Learning: In the pelican-dev/panel repository, the team prefers using the abdelhamiderrahmouni/filament-monaco-editor package over Filament v4's built-in CodeEditor component. Do not suggest replacing MonacoEditor with the built-in CodeEditor.
📚 Learning: 2025-12-17T21:38:40.755Z
Learnt from: notAreYouScared
Repo: pelican-dev/panel PR: 2013
File: app/Filament/Admin/Resources/Servers/Pages/EditServer.php:5-5
Timestamp: 2025-12-17T21:38:40.755Z
Learning: In the pelican-dev/panel repository, the team prefers using the abdelhamiderrahmouni/filament-monaco-editor package over Filament v4's built-in CodeEditor component. Do not suggest replacing MonacoEditor with the built-in CodeEditor.
Applied to files:
resources/views/filament/components/monaco-editor.blade.phpapp/Filament/Admin/Resources/Servers/Pages/EditServer.phpapp/Filament/Server/Resources/Files/Pages/EditFiles.phpapp/Filament/Server/Resources/Files/Pages/ListFiles.phpresources/css/monaco-editor.cssapp/Filament/Admin/Resources/Eggs/Pages/EditEgg.phpapp/Filament/Components/Forms/Fields/MonacoEditor.php
📚 Learning: 2025-09-16T19:30:21.443Z
Learnt from: rmartinoscar
Repo: pelican-dev/panel PR: 1708
File: app/Filament/App/Resources/Servers/Pages/ListServers.php:115-116
Timestamp: 2025-09-16T19:30:21.443Z
Learning: The Filament `recordActions` method accepts both arrays and ActionGroup instances (signature: `array | ActionGroup $actions`), so ActionGroup can be passed directly without wrapping in an array.
Applied to files:
app/Filament/Admin/Resources/Servers/Pages/EditServer.phpapp/Filament/Server/Resources/Files/Pages/ListFiles.phpapp/Filament/Admin/Resources/Eggs/Pages/EditEgg.php
📚 Learning: 2025-10-15T11:55:53.461Z
Learnt from: rmartinoscar
Repo: pelican-dev/panel PR: 1801
File: app/Extensions/OAuth/Schemas/AuthentikSchema.php:7-10
Timestamp: 2025-10-15T11:55:53.461Z
Learning: In Filament v4, Wizard Step components use the Filament\Schemas namespace (Filament\Schemas\Components\Wizard\Step), not Filament\Forms.
Applied to files:
app/Filament/Admin/Resources/Servers/Pages/EditServer.phpapp/Filament/Server/Resources/Files/Pages/ListFiles.phpapp/Filament/Admin/Resources/Eggs/Pages/EditEgg.php
📚 Learning: 2025-09-16T19:32:01.343Z
Learnt from: rmartinoscar
Repo: pelican-dev/panel PR: 1708
File: app/Filament/App/Resources/Servers/Pages/ListServers.php:228-266
Timestamp: 2025-09-16T19:32:01.343Z
Learning: ActionGroup in Filament automatically hides itself when all child actions are not visible, so additional hidden() logic to prevent empty groups is unnecessary.
Applied to files:
app/Filament/Admin/Resources/Servers/Pages/EditServer.phpapp/Filament/Admin/Resources/Eggs/Pages/EditEgg.php
📚 Learning: 2025-11-04T08:38:56.562Z
Learnt from: Boy132
Repo: pelican-dev/panel PR: 1861
File: app/Filament/Server/Resources/Files/Pages/EditFiles.php:220-229
Timestamp: 2025-11-04T08:38:56.562Z
Learning: In the Pelican Panel codebase, the `AlertBanner::title()` and `body()` methods accept `string | Closure | null` types and handle Closure invocation internally, so Closures can be passed directly without manual invocation.
Applied to files:
app/Filament/Server/Resources/Files/Pages/EditFiles.php
📚 Learning: 2025-10-15T11:55:53.461Z
Learnt from: rmartinoscar
Repo: pelican-dev/panel PR: 1801
File: app/Extensions/OAuth/Schemas/AuthentikSchema.php:7-10
Timestamp: 2025-10-15T11:55:53.461Z
Learning: In Filament v4, the Forms component Placeholder was deprecated and removed. Use TextEntry from Filament\Infolists\Components\TextEntry in forms instead, binding values with ->state(). For HTML content, use ->html().
Applied to files:
app/Filament/Server/Resources/Files/Pages/ListFiles.php
🧬 Code graph analysis (4)
app/Filament/Admin/Resources/Servers/Pages/EditServer.php (1)
app/Filament/Components/Forms/Fields/MonacoEditor.php (2)
MonacoEditor(9-318)showFullScreenToggle(229-234)
app/Filament/Server/Resources/Files/Pages/EditFiles.php (2)
app/Filament/Components/Forms/Fields/MonacoEditor.php (3)
MonacoEditor(9-318)language(76-86)showFullScreenToggle(229-234)app/Enums/EditorLanguages.php (1)
fromWithAlias(94-135)
app/Filament/Server/Resources/Files/Pages/ListFiles.php (1)
app/Filament/Components/Forms/Fields/MonacoEditor.php (3)
MonacoEditor(9-318)showFullScreenToggle(229-234)language(76-86)
app/Filament/Admin/Resources/Eggs/Pages/EditEgg.php (1)
app/Filament/Components/Forms/Fields/MonacoEditor.php (3)
MonacoEditor(9-318)showFullScreenToggle(229-234)language(76-86)
🔇 Additional comments (9)
resources/views/filament/components/monaco-editor.blade.php (1)
138-181: LGTM for the UI structure.The fullscreen toggle, editor container, and placeholder implementation look correct with proper Alpine.js bindings for show/hide states and click handling.
app/Filament/Components/Forms/Fields/MonacoEditor.php (3)
41-52: Null-safety: config values may return null if config file is missing.If the config file isn't published or a key is missing,
config()returnsnull, which could cause type mismatches with the typed properties.Consider adding null coalescing defaults or calling
parent::setUp():protected function setUp(): void { + parent::setUp(); + - $this->showPlaceholder = config('filament-monaco-editor.general.show-placeholder'); + $this->showPlaceholder = config('filament-monaco-editor.general.show-placeholder', true);
57-69: Good validation for theme existence with clear error message.The
editorTheme()method properly validates theme configuration before use and usesJSON_THROW_ON_ERRORfor safe JSON encoding.
76-86: Good handling of Blade language fallback to HTML.Correctly normalizes 'blade' and 'blade.php' to 'html' since Monaco doesn't ship with Blade syntax support.
app/Filament/Admin/Resources/Servers/Pages/EditServer.php (1)
306-327: Clean integration of MonacoEditor for install logs.The MonacoEditor is properly configured with appropriate error handling for connection failures. The disabled fullscreen toggle is documented with a clear comment explaining the layout issue.
app/Filament/Server/Resources/Files/Pages/ListFiles.php (1)
499-510: Well-structured language selector with Monaco integration.The implementation correctly:
- Uses the
EditorLanguagesenum for consistent language options- Dispatches
setLanguageLivewire event to update the editor dynamically- Binds the editor language via closure to the selected value
app/Filament/Admin/Resources/Eggs/Pages/EditEgg.php (1)
5-5: LGTM! Clean import updates.The import changes correctly introduce EditorLanguages enum and MonacoEditor component, replacing the previous CodeEditor. This aligns with the PR objective and the team's preference for the MonacoEditor package.
Based on learnings, the team prefers using the abdelhamiderrahmouni/filament-monaco-editor package over Filament v4's built-in CodeEditor component.
Also applies to: 10-10
app/Filament/Server/Resources/Files/Pages/EditFiles.php (2)
5-5: LGTM! Consistent import pattern.The import changes match the pattern established in EditEgg.php and correctly support the MonacoEditor migration.
Also applies to: 10-10
136-143: Excellent language detection logic!The use of
EditorLanguages::fromWithAlias(pathinfo($this->path, PATHINFO_EXTENSION))provides intelligent default language selection based on the file extension. This is a great UX improvement that automatically configures the editor for the file type being edited.The reactive
->live()combined with the MonacoEditor's language binding (see line 146) ensures the editor updates when the language selection changes.
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: 1
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/Filament/Components/Forms/Fields/MonacoEditor.php(1 hunks)config/monaco-editor.php(1 hunks)resources/views/filament/components/monaco-editor.blade.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- app/Filament/Components/Forms/Fields/MonacoEditor.php
- resources/views/filament/components/monaco-editor.blade.php
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: notAreYouScared
Repo: pelican-dev/panel PR: 2013
File: app/Filament/Admin/Resources/Servers/Pages/EditServer.php:5-5
Timestamp: 2025-12-17T21:38:40.755Z
Learning: In the pelican-dev/panel repository, the team prefers using the abdelhamiderrahmouni/filament-monaco-editor package over Filament v4's built-in CodeEditor component. Do not suggest replacing MonacoEditor with the built-in CodeEditor.
🔇 Additional comments (2)
config/monaco-editor.php (2)
3-14: General settings look reasonable.The configuration structure is clean and all settings are appropriate for the Filament Monaco Editor package. The font-size value as a string (
'15px') is correct for this package as shown in the official package documentation. The default theme correctly references a valid theme definition.
15-148: The blackboard theme configuration is correct per Monaco Editor specification. Token rules correctly use hex colors without the '#' prefix (e.g., 'ffa500', '808080'), while the colors object uses hex with '#' prefix (e.g., '#000000'). The base theme 'vs-dark' is a valid Monaco built-in theme. Font style values including 'italic', 'underline', and compound values like 'italic underline' are valid Monaco fontStyle options. All editor color keys used in the configuration—editor.foreground, editor.background, editor.selectionBackground, editor.lineHighlightBackground, editorCursor.foreground, and editorWhitespace.foreground—are valid Monaco theme color properties.Likely an incorrect or invalid review 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: 3
♻️ Duplicate comments (3)
resources/views/filament/components/monaco-editor.blade.php (3)
42-46: CustomEvent detail should use thedetailproperty.Custom event data should be passed via the
detailproperty in the options object. The current implementation won't makemonacoIdaccessible to event listeners.Apply this diff to fix:
monacoEditorFocus(){ document.getElementById(this.monacoId).dispatchEvent( - new CustomEvent('monaco-editor-focused', { monacoId: this.monacoId }) + new CustomEvent('monaco-editor-focused', { detail: { monacoId: this.monacoId } }) ); },
48-52: Missing variable declaration creates implicit global.The
scriptvariable is not declared withletorconst, creating an implicit global variable. This can cause issues with strict mode and potential variable collisions.Apply this diff to fix:
monacoEditorAddLoaderScriptToHead() { - script = document.createElement('script'); + const script = document.createElement('script'); script.src = 'https://cdnjs.cloudflare.com/ajax/libs/monaco-editor/0.49.0/min/vs/loader.min.js'; document.head.appendChild(script); },
97-99: Incorrect casing for Monaco editor optionwrappingIndent.Monaco editor expects
wrappingIndent(lowercase 'w'), notWrappingIndent. This option will be silently ignored with the current casing.Apply this diff to fix:
wordWrap: 'on', - WrappingIndent: 'same', + wrappingIndent: 'same',
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/Filament/Components/Forms/Fields/MonacoEditor.php(1 hunks)resources/views/filament/components/monaco-editor.blade.php(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: notAreYouScared
Repo: pelican-dev/panel PR: 2013
File: app/Filament/Admin/Resources/Servers/Pages/EditServer.php:5-5
Timestamp: 2025-12-17T21:38:40.755Z
Learning: In the pelican-dev/panel repository, the team prefers using the abdelhamiderrahmouni/filament-monaco-editor package over Filament v4's built-in CodeEditor component. Do not suggest replacing MonacoEditor with the built-in CodeEditor.
📚 Learning: 2025-12-17T21:38:40.755Z
Learnt from: notAreYouScared
Repo: pelican-dev/panel PR: 2013
File: app/Filament/Admin/Resources/Servers/Pages/EditServer.php:5-5
Timestamp: 2025-12-17T21:38:40.755Z
Learning: In the pelican-dev/panel repository, the team prefers using the abdelhamiderrahmouni/filament-monaco-editor package over Filament v4's built-in CodeEditor component. Do not suggest replacing MonacoEditor with the built-in CodeEditor.
Applied to files:
resources/views/filament/components/monaco-editor.blade.phpapp/Filament/Components/Forms/Fields/MonacoEditor.php
📚 Learning: 2025-10-15T11:55:53.461Z
Learnt from: rmartinoscar
Repo: pelican-dev/panel PR: 1801
File: app/Extensions/OAuth/Schemas/AuthentikSchema.php:7-10
Timestamp: 2025-10-15T11:55:53.461Z
Learning: In Filament v4, the Forms component Placeholder was deprecated and removed. Use TextEntry from Filament\Infolists\Components\TextEntry in forms instead, binding values with ->state(). For HTML content, use ->html().
Applied to files:
app/Filament/Components/Forms/Fields/MonacoEditor.php
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: 3
♻️ Duplicate comments (2)
resources/views/filament/components/monaco-editor.blade.php (2)
42-46: CustomEvent detail should use thedetailproperty.Custom event data should be passed via the
detailproperty in the options object, not as a direct property. The current implementation won't makemonacoIdaccessible to event listeners.🔎 Apply this diff to fix the CustomEvent:
monacoEditorFocus(){ document.getElementById(this.monacoId).dispatchEvent( - new CustomEvent('monaco-editor-focused', { monacoId: this.monacoId }) + new CustomEvent('monaco-editor-focused', { detail: { monacoId: this.monacoId } }) ); },
48-52: Missing variable declaration creates implicit global.The
scriptvariable is not declared withletorconst, which creates an implicit global variable. This can cause issues with strict mode and potential variable collisions.🔎 Apply this diff to add proper variable declaration:
monacoEditorAddLoaderScriptToHead() { - script = document.createElement('script'); + const script = document.createElement('script'); script.src = 'https://cdnjs.cloudflare.com/ajax/libs/monaco-editor/0.49.0/min/vs/loader.min.js'; document.head.appendChild(script); },
🧹 Nitpick comments (1)
resources/views/filament/components/monaco-editor.blade.php (1)
73-73: Consider less aggressive polling interval.The 5ms polling interval is quite aggressive. A 50-100ms interval would be less resource-intensive while still providing near-instant editor initialization without noticeable delay to users.
💡 Suggested improvement:
- }, 5); " :id="monacoId" + }, 50); " :id="monacoId"
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
app/Filament/Admin/Resources/Eggs/Pages/CreateEgg.php(2 hunks)app/Filament/Admin/Resources/Eggs/Pages/EditEgg.php(2 hunks)app/Filament/Admin/Resources/Servers/Pages/EditServer.php(2 hunks)app/Filament/Components/Forms/Fields/MonacoEditor.php(1 hunks)app/Filament/Server/Resources/Files/Pages/EditFiles.php(3 hunks)app/Filament/Server/Resources/Files/Pages/ListFiles.php(2 hunks)config/monaco-editor.php(1 hunks)resources/views/filament/components/monaco-editor.blade.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- app/Filament/Server/Resources/Files/Pages/EditFiles.php
- app/Filament/Admin/Resources/Servers/Pages/EditServer.php
- config/monaco-editor.php
- app/Filament/Admin/Resources/Eggs/Pages/CreateEgg.php
- app/Filament/Components/Forms/Fields/MonacoEditor.php
- app/Filament/Server/Resources/Files/Pages/ListFiles.php
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: notAreYouScared
Repo: pelican-dev/panel PR: 2013
File: app/Filament/Admin/Resources/Servers/Pages/EditServer.php:5-5
Timestamp: 2025-12-17T21:38:40.755Z
Learning: In the pelican-dev/panel repository, the team prefers using the abdelhamiderrahmouni/filament-monaco-editor package over Filament v4's built-in CodeEditor component. Do not suggest replacing MonacoEditor with the built-in CodeEditor.
📚 Learning: 2025-12-17T21:38:40.755Z
Learnt from: notAreYouScared
Repo: pelican-dev/panel PR: 2013
File: app/Filament/Admin/Resources/Servers/Pages/EditServer.php:5-5
Timestamp: 2025-12-17T21:38:40.755Z
Learning: In the pelican-dev/panel repository, the team prefers using the abdelhamiderrahmouni/filament-monaco-editor package over Filament v4's built-in CodeEditor component. Do not suggest replacing MonacoEditor with the built-in CodeEditor.
Applied to files:
resources/views/filament/components/monaco-editor.blade.phpapp/Filament/Admin/Resources/Eggs/Pages/EditEgg.php
📚 Learning: 2025-10-15T11:55:53.461Z
Learnt from: rmartinoscar
Repo: pelican-dev/panel PR: 1801
File: app/Extensions/OAuth/Schemas/AuthentikSchema.php:7-10
Timestamp: 2025-10-15T11:55:53.461Z
Learning: In Filament v4, the Forms component Placeholder was deprecated and removed. Use TextEntry from Filament\Infolists\Components\TextEntry in forms instead, binding values with ->state(). For HTML content, use ->html().
Applied to files:
resources/views/filament/components/monaco-editor.blade.php
📚 Learning: 2025-09-16T19:30:21.443Z
Learnt from: rmartinoscar
Repo: pelican-dev/panel PR: 1708
File: app/Filament/App/Resources/Servers/Pages/ListServers.php:115-116
Timestamp: 2025-09-16T19:30:21.443Z
Learning: The Filament `recordActions` method accepts both arrays and ActionGroup instances (signature: `array | ActionGroup $actions`), so ActionGroup can be passed directly without wrapping in an array.
Applied to files:
app/Filament/Admin/Resources/Eggs/Pages/EditEgg.php
📚 Learning: 2025-09-16T19:32:01.343Z
Learnt from: rmartinoscar
Repo: pelican-dev/panel PR: 1708
File: app/Filament/App/Resources/Servers/Pages/ListServers.php:228-266
Timestamp: 2025-09-16T19:32:01.343Z
Learning: ActionGroup in Filament automatically hides itself when all child actions are not visible, so additional hidden() logic to prevent empty groups is unnecessary.
Applied to files:
app/Filament/Admin/Resources/Eggs/Pages/EditEgg.php
📚 Learning: 2025-10-15T11:55:53.461Z
Learnt from: rmartinoscar
Repo: pelican-dev/panel PR: 1801
File: app/Extensions/OAuth/Schemas/AuthentikSchema.php:7-10
Timestamp: 2025-10-15T11:55:53.461Z
Learning: In Filament v4, Wizard Step components use the Filament\Schemas namespace (Filament\Schemas\Components\Wizard\Step), not Filament\Forms.
Applied to files:
app/Filament/Admin/Resources/Eggs/Pages/EditEgg.php
🧬 Code graph analysis (1)
app/Filament/Admin/Resources/Eggs/Pages/EditEgg.php (1)
app/Filament/Components/Forms/Fields/MonacoEditor.php (2)
MonacoEditor(10-180)language(61-66)
🔇 Additional comments (11)
app/Filament/Admin/Resources/Eggs/Pages/EditEgg.php (2)
5-5: LGTM! Necessary imports for MonacoEditor integration.The imports are correctly added to support the MonacoEditor component and language configuration used in the install script field.
Based on learnings, the team prefers MonacoEditor over Filament v4's built-in CodeEditor.
Also applies to: 10-10
428-431: LGTM! Appropriate MonacoEditor configuration for install scripts.The MonacoEditor is correctly configured with shell language syntax highlighting, which is ideal for the install script field. The configuration maintains the previous layout and field behavior.
resources/views/filament/components/monaco-editor.blade.php (9)
1-11: LGTM!The Livewire event listeners for
setContentandsetLanguageare properly structured and correctly update the Monaco editor instance.
16-27: LGTM!The Alpine.js data initialization is well-structured with proper Livewire entanglement for
monacoContentand all necessary configuration variables.
28-34: LGTM!The fullscreen toggle correctly manages the component state, document body overflow, and element dimensions.
36-40: LGTM!The Monaco editor change handler properly syncs editor content to the Alpine.js state.
54-56: LGTM!The
wrapPreviewmethod correctly wraps content for preview display.
58-67: LGTM!The initialization logic correctly sets up preview content, element height, and watchers for fullscreen mode changes.
69-71: LGTM!The loader script injection and AMD configuration correctly handle dynamic loading of Monaco Editor from CDN with proper worker proxy setup.
Also applies to: 77-79
83-99: LGTM!The Monaco editor configuration is comprehensive with proper theme definition, language support, and editor options. The
wrappingIndentoption is correctly cased.
113-152: LGTM!The template markup is well-structured with proper accessibility attributes, conditional rendering, and correct integration of Alpine.js directives. The fullscreen toggle button includes appropriate ARIA labels and the editor element is correctly marked with
wire:ignore.
public/vendor/filament-monaco-editor/filament-monaco-editor.css
Outdated
Show resolved
Hide resolved
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: 2
♻️ Duplicate comments (5)
resources/views/filament/components/monaco-editor.blade.php (5)
41-45: CustomEvent detail should use thedetailproperty.Custom event data should be passed via the
detailproperty in the options object. The current implementation won't makemonacoIdaccessible to event listeners.🔎 Proposed fix
monacoEditorFocus(){ document.getElementById(this.monacoId).dispatchEvent( - new CustomEvent('monaco-editor-focused', { monacoId: this.monacoId }) + new CustomEvent('monaco-editor-focused', { detail: { monacoId: this.monacoId } }) ); },
47-51: Missing variable declaration creates implicit global.The
scriptvariable is not declared withconstorlet, creating an implicit global variable that can cause issues with strict mode and potential collisions.🔎 Proposed fix
monacoEditorAddLoaderScriptToHead() { - script = document.createElement('script'); + const script = document.createElement('script'); script.src = 'https://cdnjs.cloudflare.com/ajax/libs/monaco-editor/0.49.0/min/vs/loader.min.js'; document.head.appendChild(script); },
72-108: Missing variable declaration creates implicit global.The
monacoLoaderIntervalvariable is not declared withconstorlet, creating an implicit global variable.🔎 Proposed fix
- monacoLoaderInterval = setInterval(() => { + const monacoLoaderInterval = setInterval(() => {
101-103: Event listener not cleaned up on component destruction.The
monaco-editor-focusedevent listener is added but never removed. If the Alpine component is destroyed and recreated, this could lead to memory leaks and duplicate event handlers.
107-107: Missingthis.prefix prevents state update.The assignment
monacoLoader = falseupdates a local/global variable instead of the Alpine.js component state. This should bethis.monacoLoader = falseto properly update the reactive state.🔎 Proposed fix
clearInterval(monacoLoaderInterval); - monacoLoader = false; + this.monacoLoader = false;
🧹 Nitpick comments (4)
resources/views/filament/components/monaco-editor.blade.php (3)
53-55: Remove unnecessary wrapper function.The
wrapPreviewfunction simply returns its input without any transformation. This adds no value and can be removed.🔎 Proposed fix
- wrapPreview(value){ - return `${value}`; - },And update line 58:
- previewContent = wrapPreview(monacoContent); + previewContent = monacoContent;
68-70: Use strict equality for type safety.Use strict equality
===instead of loose equality==for more predictable behavior and to avoid type coercion issues.🔎 Proposed fix
- if(typeof _amdLoaderGlobal == 'undefined'){ + if(typeof _amdLoaderGlobal === 'undefined'){ monacoEditorAddLoaderScriptToHead(); }
85-85: Consider defensive localStorage access.The code directly accesses
localStoragewithout error handling. While rare, localStorage can throw exceptions in some browser contexts (e.g., private browsing modes with strict settings).🔎 Suggested approach
- theme: localStorage.getItem('theme') === 'light' ? 'iPlastic' : 'custom', + theme: (() => { + try { + return localStorage.getItem('theme') === 'light' ? 'iPlastic' : 'custom'; + } catch { + return 'custom'; + } + })(),app/Filament/Components/Forms/Fields/MonacoEditor.php (1)
12-23: Consider making configuration properties protected.The configuration properties (
showLoader,automaticLayout,lineNumbersMinChars,fontSize,language,theme) are declared aspublic, which exposes internal state. Filament form fields typically useprotectedproperties and expose them only through fluent setter methods, which better encapsulates the component's configuration.🔎 Proposed changes
- public bool|Closure $showLoader = true; + protected bool|Closure $showLoader = true; - public bool|Closure $automaticLayout = true; + protected bool|Closure $automaticLayout = true; - public int|Closure $lineNumbersMinChars = 3; + protected int|Closure $lineNumbersMinChars = 3; - public string|Closure $fontSize = '15px'; + protected string|Closure $fontSize = '15px'; - public EditorLanguages|Closure $language = EditorLanguages::html; + protected EditorLanguages|Closure $language = EditorLanguages::html; - public string|Closure $theme = 'blackboard'; + protected string|Closure $theme = 'blackboard';
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/Filament/Components/Forms/Fields/MonacoEditor.php(1 hunks)resources/views/filament/components/monaco-editor.blade.php(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: notAreYouScared
Repo: pelican-dev/panel PR: 2013
File: app/Filament/Admin/Resources/Servers/Pages/EditServer.php:5-5
Timestamp: 2025-12-17T21:38:40.755Z
Learning: In the pelican-dev/panel repository, the team prefers using the abdelhamiderrahmouni/filament-monaco-editor package over Filament v4's built-in CodeEditor component. Do not suggest replacing MonacoEditor with the built-in CodeEditor.
📚 Learning: 2025-12-17T21:38:40.755Z
Learnt from: notAreYouScared
Repo: pelican-dev/panel PR: 2013
File: app/Filament/Admin/Resources/Servers/Pages/EditServer.php:5-5
Timestamp: 2025-12-17T21:38:40.755Z
Learning: In the pelican-dev/panel repository, the team prefers using the abdelhamiderrahmouni/filament-monaco-editor package over Filament v4's built-in CodeEditor component. Do not suggest replacing MonacoEditor with the built-in CodeEditor.
Applied to files:
app/Filament/Components/Forms/Fields/MonacoEditor.phpresources/views/filament/components/monaco-editor.blade.php
📚 Learning: 2025-10-15T11:55:53.461Z
Learnt from: rmartinoscar
Repo: pelican-dev/panel PR: 1801
File: app/Extensions/OAuth/Schemas/AuthentikSchema.php:7-10
Timestamp: 2025-10-15T11:55:53.461Z
Learning: In Filament v4, the Forms component Placeholder was deprecated and removed. Use TextEntry from Filament\Infolists\Components\TextEntry in forms instead, binding values with ->state(). For HTML content, use ->html().
Applied to files:
app/Filament/Components/Forms/Fields/MonacoEditor.phpresources/views/filament/components/monaco-editor.blade.php
🔇 Additional comments (7)
resources/views/filament/components/monaco-editor.blade.php (3)
1-11: LGTM!The Livewire event handlers for
setContentandsetLanguageare correctly implemented and follow Filament conventions.
16-26: LGTM!Alpine.js data initialization correctly sets up all required properties with proper Livewire entanglement.
109-154: LGTM!The UI template is well-structured with proper accessibility attributes and conditional rendering logic. The fullscreen toggle and editor container are correctly implemented.
app/Filament/Components/Forms/Fields/MonacoEditor.php (4)
27-34: LGTM!The
setUp()method correctly initializes configuration defaults from the config file.
50-97: LGTM!All fluent setter methods are correctly implemented following Filament's chainable API pattern.
99-122: LGTM!The getter methods correctly evaluate closures and cast return values to the appropriate types.
1-123: Address team feedback: Consider slimming down configuration options.Based on the earlier comment from Boy132, there may be configuration options that are no longer needed. The current implementation includes loader-related methods (
showLoader(),hideLoader(),getShowLoader()). Please clarify which configuration options should be removed.Based on learnings, the team is transitioning away from the built-in CodeEditor, so ensure any removed options don't break the migration path for existing usages.
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
♻️ Duplicate comments (1)
app/Filament/Components/Forms/Fields/MonacoEditor.php (1)
35-47: Still needs evaluation of the theme property before use.This critical issue was flagged in a previous review but remains unresolved. The method accesses
$this->themedirectly without evaluating it first. If a closure is passed to thetheme()method, this will cause errors.🔎 Apply the previously suggested fix
public function editorTheme(): string { - if (!isset(config('monaco-editor.themes')[$this->theme])) { - throw new Exception("Theme {$this->theme} not found in config file."); + $themeName = $this->evaluate($this->theme); + + if (!isset(config('monaco-editor.themes')[$themeName])) { + throw new Exception("Theme {$themeName} not found in config file."); } return json_encode([ - 'base' => config("monaco-editor.themes.{$this->theme}.base"), - 'inherit' => config("monaco-editor.themes.{$this->theme}.inherit"), - 'rules' => config("monaco-editor.themes.{$this->theme}.rules"), - 'colors' => config("monaco-editor.themes.{$this->theme}.colors"), + 'base' => config("monaco-editor.themes.{$themeName}.base"), + 'inherit' => config("monaco-editor.themes.{$themeName}.inherit"), + 'rules' => config("monaco-editor.themes.{$themeName}.rules"), + 'colors' => config("monaco-editor.themes.{$themeName}.colors"), ], JSON_THROW_ON_ERROR); }
🧹 Nitpick comments (1)
app/Filament/Components/Forms/Fields/MonacoEditor.php (1)
12-12: Consider removing unused configuration options.As noted by Boy132, the file may contain configuration options that are no longer needed (e.g., loader-related properties and methods). Consider reviewing which options are actually used in the Blade template and removing any unused code to simplify the component.
Also applies to: 56-68, 103-106
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/Filament/Components/Forms/Fields/MonacoEditor.php(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: notAreYouScared
Repo: pelican-dev/panel PR: 2013
File: app/Filament/Admin/Resources/Servers/Pages/EditServer.php:5-5
Timestamp: 2025-12-17T21:38:45.317Z
Learning: In the pelican-dev/panel repository, the team prefers using the abdelhamiderrahmouni/filament-monaco-editor package over Filament v4's built-in CodeEditor component. Do not suggest replacing MonacoEditor with the built-in CodeEditor.
📚 Learning: 2025-12-17T21:38:45.317Z
Learnt from: notAreYouScared
Repo: pelican-dev/panel PR: 2013
File: app/Filament/Admin/Resources/Servers/Pages/EditServer.php:5-5
Timestamp: 2025-12-17T21:38:45.317Z
Learning: In the pelican-dev/panel repository, the team prefers using the abdelhamiderrahmouni/filament-monaco-editor package over Filament v4's built-in CodeEditor component. Do not suggest replacing MonacoEditor with the built-in CodeEditor.
Applied to files:
app/Filament/Components/Forms/Fields/MonacoEditor.php
📚 Learning: 2025-10-15T11:55:53.461Z
Learnt from: rmartinoscar
Repo: pelican-dev/panel PR: 1801
File: app/Extensions/OAuth/Schemas/AuthentikSchema.php:7-10
Timestamp: 2025-10-15T11:55:53.461Z
Learning: In Filament v4, the Forms component Placeholder was deprecated and removed. Use TextEntry from Filament\Infolists\Components\TextEntry in forms instead, binding values with ->state(). For HTML content, use ->html().
Applied to files:
app/Filament/Components/Forms/Fields/MonacoEditor.php
🔇 Additional comments (1)
app/Filament/Components/Forms/Fields/MonacoEditor.php (1)
98-121: LGTM! Getters properly evaluate closure properties.All getter methods correctly use
$this->evaluate()to handle both static values and closures, with appropriate type casting for each return type. ThegetFontSize()return type has been correctly fixed tostring.
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: 2
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/Filament/Components/Forms/Fields/MonacoEditor.php(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: notAreYouScared
Repo: pelican-dev/panel PR: 2013
File: app/Filament/Admin/Resources/Servers/Pages/EditServer.php:5-5
Timestamp: 2025-12-17T21:38:45.317Z
Learning: In the pelican-dev/panel repository, the team prefers using the abdelhamiderrahmouni/filament-monaco-editor package over Filament v4's built-in CodeEditor component. Do not suggest replacing MonacoEditor with the built-in CodeEditor.
📚 Learning: 2025-12-17T21:38:45.317Z
Learnt from: notAreYouScared
Repo: pelican-dev/panel PR: 2013
File: app/Filament/Admin/Resources/Servers/Pages/EditServer.php:5-5
Timestamp: 2025-12-17T21:38:45.317Z
Learning: In the pelican-dev/panel repository, the team prefers using the abdelhamiderrahmouni/filament-monaco-editor package over Filament v4's built-in CodeEditor component. Do not suggest replacing MonacoEditor with the built-in CodeEditor.
Applied to files:
app/Filament/Components/Forms/Fields/MonacoEditor.php
📚 Learning: 2025-10-15T11:55:53.461Z
Learnt from: rmartinoscar
Repo: pelican-dev/panel PR: 1801
File: app/Extensions/OAuth/Schemas/AuthentikSchema.php:7-10
Timestamp: 2025-10-15T11:55:53.461Z
Learning: In Filament v4, the Forms component Placeholder was deprecated and removed. Use TextEntry from Filament\Infolists\Components\TextEntry in forms instead, binding values with ->state(). For HTML content, use ->html().
Applied to files:
app/Filament/Components/Forms/Fields/MonacoEditor.php
Return of Monaco!
Replaces the filament v4 code editor with monaco, like pre-v4