Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a new MageForge Toolbar UI on the storefront that hosts basic audit tools and integrates the Inspector activation button into the toolbar, alongside new backend configuration options for theme/label display and improved messaging.
Changes:
- Added a new frontend toolbar component (JS/CSS) with an audit registry and several initial WCAG/performance audits.
- Refactored the Inspector activation UI to be injected into the toolbar instead of using a standalone floating button.
- Added admin configuration for showing/hiding button labels and enabled the Inspector/Toolbar by default in config.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/view/frontend/web/js/toolbar.js | Registers Alpine toolbar component and manages lifecycle. |
| src/view/frontend/web/js/toolbar/ui.js | Builds toolbar DOM, audit menu UI, grouping, and outside-click handling. |
| src/view/frontend/web/js/toolbar/audits.js | Audit dispatcher (toggle/run/deactivate, group collapse, badge updates). |
| src/view/frontend/web/js/toolbar/audits/index.js | Audit registry and group definitions. |
| src/view/frontend/web/js/toolbar/audits/images-without-alt.js | Adds “images without alt” audit. |
| src/view/frontend/web/js/toolbar/audits/images-without-dimensions.js | Adds “images without width/height” (CLS) audit. |
| src/view/frontend/web/js/toolbar/audits/images-without-lazy-load.js | Adds “below-the-fold images without loading=lazy” audit. |
| src/view/frontend/web/js/toolbar/audits/inputs-without-label.js | Adds “inputs without accessible label” audit. |
| src/view/frontend/web/js/toolbar/audits/low-contrast-text.js | Adds “low contrast text” audit with contrast calculation. |
| src/view/frontend/web/js/toolbar/audits/tab-order.js | Adds “tab order visualization” audit with overlay + injected CSS. |
| src/view/frontend/web/js/inspector.js | Injects inspector button into toolbar and adds window-event plumbing/cleanup. |
| src/view/frontend/web/js/inspector/ui.js | Replaces floating-button state updates with toolbar event dispatch. |
| src/view/frontend/web/js/inspector/accessibility.js | Improves accessibility name resolution + focusability (hidden inputs). |
| src/view/frontend/web/css/toolbar.css | Adds full toolbar styling + shared highlight styles. |
| src/view/frontend/web/css/inspector.css | Adjusts inspector button styles to work inside toolbar layout. |
| src/view/frontend/web/css/audits/tab-order.css | Adds overlay styles injected by tab-order audit on demand. |
| src/view/frontend/templates/inspector.phtml | Loads toolbar assets and adds Alpine root node/config attributes. |
| src/Block/Inspector.php | Adds toolbar asset URLs + show_button_labels config accessor. |
| src/etc/adminhtml/system.xml | Adds/renames admin settings for toolbar + show button labels. |
| src/etc/config.xml | Enables inspector/toolbar by default and sets default show_button_labels. |
| src/Console/Command/Dev/InspectorCommand.php | Improves CLI messaging when inspector is disabled. |
src/view/frontend/web/js/toolbar/audits/inputs-without-label.js
Outdated
Show resolved
Hide resolved
| destroy() { | ||
| if (this._outsideClickHandler) { | ||
| document.removeEventListener('click', this._outsideClickHandler); | ||
| this._outsideClickHandler = null; | ||
| } |
There was a problem hiding this comment.
destroy() removes the toolbar container but doesn’t deactivate active audits first. Audits that attach observers/scroll handlers (e.g. tab-order) can keep running and leave overlays behind after the toolbar is torn down. Call deactivateAllAudits() (and close the menu) before removing listeners/DOM.
| if (typeof Alpine !== 'undefined') { | ||
| _registerMageforgeToolbar(); | ||
| } else { | ||
| document.addEventListener('alpine:init', _registerMageforgeToolbar); | ||
| } |
There was a problem hiding this comment.
Unlike inspector.js, this file doesn’t call Alpine.initTree() when Alpine is already started (common in Hyvä where Alpine loads before deferred scripts). In that scenario, registering the component alone won’t initialize existing [x-data="mageforgeToolbar"] nodes, so the toolbar may never render. Consider mirroring the mageforgeInspector pattern by initializing the tree for those elements after registration.
| // Listen for toggle event from MageForge Toolbar | ||
| this._toggleInspectorHandler = () => this.toggleInspector(); | ||
| window.addEventListener('mageforge:toolbar:toggle-inspector', this._toggleInspectorHandler); | ||
|
|
There was a problem hiding this comment.
A mageforge:toolbar:toggle-inspector window listener is registered, but no code in the PR dispatches this event (the injected inspector button calls toggleInspector() directly). This adds an unused global listener and can confuse future maintenance; either dispatch this event from the toolbar UI or remove the listener entirely.
| // Listen for toggle event from MageForge Toolbar | |
| this._toggleInspectorHandler = () => this.toggleInspector(); | |
| window.addEventListener('mageforge:toolbar:toggle-inspector', this._toggleInspectorHandler); |
| _appendInspectorButton() { | ||
| const toolbarContainer = document.querySelector('.mageforge-toolbar'); | ||
| if (toolbarContainer) { | ||
| this._inspectorFloatButton = this._createInspectorFloatButton(); | ||
| toolbarContainer.appendChild(this._inspectorFloatButton); |
There was a problem hiding this comment.
_appendInspectorButton() always appends a new .mageforge-inspector-float-button when it finds the toolbar container. If the inspector component is re-initialized (e.g. partial page updates / Alpine re-init), this can create duplicate buttons. Consider checking for an existing button in the container and reusing it (or removing any existing before appending).
This pull request introduces a new MageForge Toolbar component, refines the Inspector's activation button, and adds configuration options to improve usability and accessibility. The Inspector's floating button is now integrated into the Toolbar, and several configuration and UI improvements have been made in both the backend (admin settings) and frontend code.
Toolbar Integration and UI Refactoring:
src/view/frontend/web/js/inspector.js,src/view/frontend/web/js/inspector/ui.js,src/view/frontend/templates/inspector.phtml,src/view/frontend/web/css/inspector.css) [1] [2] [3] [4] [5] [6]Configuration and Admin UI Improvements:
src/etc/adminhtml/system.xml,src/etc/config.xml,src/Block/Inspector.php) [1] [2] [3] [4]Asset Management:
src/Block/Inspector.php,src/view/frontend/templates/inspector.phtml,src/view/frontend/web/css/audits/tab-order.css) [1] [2] [3]Accessibility and Usability Fixes:
aria-labelledbyreferences, excluding hidden inputs from focusability, and refining lazy loading status reporting. (src/view/frontend/web/js/inspector/accessibility.js) [1] [2] [3]Console Output Improvement:
src/Console/Command/Dev/InspectorCommand.php)