feat: data-mageforge attribute handling#189
Open
dermatz wants to merge 5 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Replaces HTML-comment-based inspector markers with data-mageforge-* attributes on the first root element of each rendered block, so block output can be safely embedded inside attribute values (notably PageBuilder URLs) without corrupting markup. Also surfaces a CMS block identifier and adapts the frontend DOM/picker/tabs code to the new attribute-based discovery, including a PageBuilder multi-root fallback.
Changes:
- Backend: inject
data-mageforge-id/data-mageforge-blockJSON via regex on the first opening tag and addcmsBlockIdto metadata. - Frontend DOM: rewrite block discovery to query
[data-mageforge-id], parse JSON metadata, and add a PageBuilder sibling-search fallback. - Frontend UI: render a "CMS Block" section when present, and let the picker still target elements inside
[data-content-type]even when no block was resolved.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/Model/TemplateEngine/Decorator/InspectorHints.php | Switch from comment wrappers to data-attribute injection on the first root tag; add cmsBlockId metadata. |
| src/view/frontend/web/js/inspector/dom.js | Replace TreeWalker comment parsing with attribute-based discovery and add PageBuilder multi-root fallback. |
| src/view/frontend/web/js/inspector/picker.js | Return inspectable target inside [data-content-type] even when no block metadata is resolved. |
| src/view/frontend/web/js/inspector/tabs.js | Carry cmsBlockId through inherited data and render a new "CMS Block" info section. |
Comment on lines
+246
to
+249
| // Escape single quotes so JSON can be safely embedded in a single-quoted HTML attribute. | ||
| // The browser automatically decodes HTML entities when getAttribute() is called, | ||
| // so JSON.parse() on the JS side will receive the correct string. | ||
| $safeJson = str_replace("'", ''', $jsonMetadata); |
Comment on lines
+255
to
265
| $result = preg_replace_callback( | ||
| '/^(\s*<[a-zA-Z][a-zA-Z0-9]*)/s', | ||
| function (array $matches) use ($wrapperId, $safeJson, &$replaced): string { | ||
| $replaced = true; | ||
| return $matches[0] | ||
| . ' data-mageforge-id="' . $wrapperId . '"' | ||
| . ' data-mageforge-block=\'' . $safeJson . "'"; | ||
| }, | ||
| $html, | ||
| $wrapperId, | ||
| 1, | ||
| ); |
Comment on lines
+222
to
+223
| // Detect CMS block identifier (e.g. for PageBuilder blocks rendered via Magento\Cms\Block\Block) | ||
| $cmsBlockId = method_exists($block, 'getBlockId') ? (string) $block->getBlockId() : ''; |
| // embedded inside HTML attribute values (e.g. PageBuilder URL blocks in href="..."). | ||
| $replaced = false; | ||
| $result = preg_replace_callback( | ||
| '/^(\s*<[a-zA-Z][a-zA-Z0-9]*)/s', |
Comment on lines
+215
to
+218
| // Escape single quotes so JSON can be safely embedded in a single-quoted HTML attribute. | ||
| // The browser automatically decodes HTML entities when getAttribute() is called, | ||
| // so JSON.parse() on the JS side will receive the correct string. | ||
| $safeJson = str_replace("'", ''', $jsonMetadata); |
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
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.
This pull request refactors how MageForge inspector metadata is injected and parsed in both backend PHP and frontend JavaScript. The system now uses HTML data attributes (instead of HTML comments) to mark and describe rendered blocks, improving robustness—especially when block output is embedded inside attribute values (e.g., PageBuilder URLs). The inspector can now more reliably identify blocks, including special handling for CMS/PageBuilder content, and exposes new metadata such as CMS block IDs.
Inspector metadata injection and backend logic:
data-mageforge-idanddata-mageforge-blockattributes on the first root HTML element of each block, preventing markup corruption when blocks are embedded in attributes. Injection is skipped if the content doesn’t start with an HTML element. [1] [2]cmsBlockId) in the inspector metadata for blocks rendered via Magento CMS/PageBuilder. [1] [2]Frontend JavaScript inspector logic:
data-mageforge-idattributes instead of scanning for comment nodes. Added robust parsing of block metadata and improved block lookup, including fallbacks for multi-root PageBuilder content.