feat: implement dark/light mode theme selection and admin configuration section #131
Conversation
There was a problem hiding this comment.
Pull request overview
Adds configurable dark/light/auto theming for the MageForge frontend Inspector, with an admin configuration section and frontend propagation so injected UI elements render consistently across themes.
Changes:
- Introduces a new admin configuration section (with ACL) to select the Inspector theme (dark/light/auto) and sets a default theme in module config.
- Exposes the configured theme via the Inspector block and passes it to the template as
data-theme. - Updates Inspector CSS/JS to use theme-aware CSS variables and propagate theme to dynamically injected UI elements.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/view/frontend/web/js/inspector.js | Propagates data-theme to injected DOM elements; updates floating button icon. |
| src/view/frontend/web/css/inspector.css | Refactors overlay styling to CSS variables and adds light/auto theme overrides. |
| src/view/frontend/templates/inspector.phtml | Adds data-theme attribute sourced from the block config. |
| src/etc/config.xml | Adds default config value for mageforge/inspector/theme. |
| src/etc/adminhtml/system.xml | Adds admin config UI for enabling Inspector and selecting theme. |
| src/etc/acl.xml | Adds ACL resource to control access to the new config section. |
| src/Model/Config/Source/InspectorTheme.php | Provides select options for theme (dark/light/auto). |
| src/Block/Inspector.php | Adds getTheme() method to read configured theme for frontend rendering. |
Comments suppressed due to low confidence (2)
src/view/frontend/web/js/inspector.js:321
- Unused variable gradientId.
const gradientId = 'mageforge-gradient-' + Math.random().toString(36).substr(2, 9);
src/view/frontend/web/js/inspector.js:942
- Unused variable lazyIcon.
const { lazyIcon, lazyColor } = this.getLazyLoadingStyle(a11yData.lazyLoading);
| --mageforge-border-glass: rgba(255, 255, 255, 0.1); | ||
| --mageforge-shadow-glow: rgba(255, 255, 255, 0.05); | ||
| } | ||
|
|
There was a problem hiding this comment.
The new CSS custom property --mageforge-shadow-glow is defined (and overridden for themes) but never used anywhere in this stylesheet. Consider removing it until it’s needed, or apply it where appropriate to avoid accumulating unused variables.
| .mageforge-inspector--glow { | |
| box-shadow: 0 0 0 1px var(--mageforge-shadow-glow); | |
| } |
| --mageforge-color-slate-100: #0f172a; /* Slate 900 */ | ||
| --mageforge-color-slate-200: #1e293b; /* Slate 800 */ | ||
| --mageforge-color-slate-300: #334155; /* Slate 700 */ | ||
| --mageforge-color-slate-400: #64748b; /* Slate 500 */ | ||
| --mageforge-color-slate-500: #94a3b8; /* Slate 400 */ | ||
|
|
There was a problem hiding this comment.
In the light theme override, --mageforge-color-slate-400 is assigned a darker value than --mageforge-color-slate-500 (effectively swapping the 400/500 shades vs the :root definitions). This makes the variable naming inconsistent and can lead to unexpected colors where these variables are used; consider keeping shade ordering consistent and adjusting specific usages instead.
| <field id="theme" translate="label" type="select" sortOrder="20" showInDefault="1" showInWebsite="1" showInStore="1"> | ||
| <label>Theme</label> | ||
| <source_model>OpenForgeProject\MageForge\Model\Config\Source\InspectorTheme</source_model> | ||
| <config_path>mageforge/inspector/theme</config_path> | ||
| <comment>Choose between Dark, Light, or Auto (System Preference) theme.</comment> | ||
| </field> |
There was a problem hiding this comment.
The field includes a <comment> string, but the translate attribute is only set to label. In Magento system.xml, comments won’t be picked up for translation unless you include comment in the translate list (e.g., translate="label comment").
| public function getTheme(): string | ||
| { | ||
| return (string) $this->scopeConfig->getValue('mageforge/inspector/theme') ?: 'dark'; | ||
| } |
There was a problem hiding this comment.
getTheme() reads mageforge/inspector/theme from the default scope only; this means website/store-view overrides configured in system.xml won’t be reflected on the storefront. Read this value using store scope (e.g., pass ScopeInterface::SCOPE_STORE to getValue()) so the frontend honors per-store configuration.
| // Generate unique ID for SVG gradient to avoid collisions | ||
| const gradientId = 'mageforge-gradient-' + Math.random().toString(36).substr(2, 9); | ||
|
|
||
| // Icon + Text with unique gradient ID |
There was a problem hiding this comment.
gradientId is still generated but no longer used by the updated SVG markup. This leaves dead code and an unnecessary Math.random() call on every button creation; remove the unused variable or reintroduce a use for it.
1 participant
This pull request adds feature-request #130 for theme support (dark, light, auto) to the MageForge Inspector component, making the inspector's appearance configurable from the Magento admin and ensuring the theme is consistently applied across the frontend UI. The changes include new configuration options, backend logic, frontend template updates, and extensive CSS and JS improvements for theming.
Configuration and Backend Integration
config.xmland exposed agetTheme()method in the Inspector block to retrieve the configured theme for frontend use. [1] [2]Theme support (dark, light, auto)
Frontend Template and Data Flow
inspector.phtml) to pass the configured theme as adata-themeattribute to the root inspector element, enabling dynamic theming on the frontend.JavaScript Theme Propagation
data-themeattribute to all dynamically injected UI elements (highlight box, info badge, floating button), ensuring consistent theming even for elements rendered outside the root component. [1] [2] [3]UI/UX Improvements