Interaction Library for Builders - #41
Conversation
📝 WalkthroughWalkthroughThe editor integration adds Interaction Library apply/insert workflows for builder editors, introduces builder-specific preset schemas and target mappings, implements insertion adapters for Gutenberg, Elementor, Bricks, and Divi, and adds Divi Visual Builder loading, selection, insertion, and persistence support. ChangesBuilder-aware interaction library
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant EditorUI
participant ConfigureModal
participant EditorAdapter
participant Builder
EditorUI->>ConfigureModal: open apply or insert mode
ConfigureModal->>EditorAdapter: insertLibraryPreset(selectedPreset)
EditorAdapter->>Builder: insert normalized preset
Builder-->>EditorAdapter: inserted elements and IDs
EditorAdapter-->>ConfigureModal: resolve target mappings
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
🤖 Pull request artifacts
|
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/editor/components/interaction-panel/index.js (1)
225-237: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winMissing error handling for
saveCurrentEditor()rejection.
saveCurrentEditor()'s rejection isn't caught anywhere in this chain..finally()still resets UI state on failure (good), but the underlying rejection propagates out ofonPublishHandleruncaught (this function's return value isn't awaited by its callers — the buttononClickand theinteract/save-interactionevent listener), producing a silent unhandled promise rejection with no user feedback if saving fails.Suggested fix
const onPublishHandler = useCallback( callback => { setStatus( 'publishing' ) // TODO: if publishing and then we are missing a target, we should show a notice. onChange( editedInteraction ).then( () => { return Promise.resolve( saveCurrentEditor() ).finally( () => { setStatus( 'idle' ) setIsDirty( false ) if ( callback ) { setTimeout( callback, 1 ) // Need a timeout here because re-publishing may be too fast. } } ) + } ).catch( error => { + console.error( 'Interactions: failed to save the current editor.', error ) // eslint-disable-line no-console } ) }, [ editedInteraction, onChange ] )The ast-grep hint about "setState with initial state variable" on
setTimeout( callback, 1 )looks like a false positive and isn't related to state usage here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/editor/components/interaction-panel/index.js` around lines 225 - 237, Handle rejection from saveCurrentEditor within onPublishHandler’s promise chain so it cannot become an unhandled rejection. Preserve the existing finally cleanup that restores idle status and clears the dirty state, and provide appropriate user-facing feedback for the save failure using the surrounding interaction-panel error handling conventions.Source: Linters/SAST tools
🧹 Nitpick comments (6)
src/editor/editors/divi.js (2)
800-806: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider guarding the external builder dispatches.
copyModuleFromPayload/copyModulesFromPayloadare Divi-internal APIs invoked with a hand-built payload; a throw here rejectsinsertPresetContentand surfaces as an unhandled rejection in the library modal flow. Atry/catchthat returnsnull(falling back to theaddModulepath where applicable) would degrade more gracefully across Divi versions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/editor/editors/divi.js` around lines 800 - 806, Wrap the external dispatches in the single-root and multi-root branches of insertPresetContent with try/catch, catching failures from copyModuleFromPayload and copyModulesFromPayload. Return null on dispatch failure so the caller can use the existing addModule fallback where applicable, while preserving normal successful dispatch behavior.
626-630: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the
console.warndebug tracing before merge.
insertPresetContentlogs eightconsole.warntraces (each with an eslint-disable) on every insertion, including the success paths. These are development artifacts and will spam the browser console for end users. Drop them, or route them through a debug-gated logger.Also applies to: 668-674, 696-703, 742-748, 768-772, 791-798, 819-826, 836-848
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/editor/editors/divi.js` around lines 626 - 630, Remove all eight development-only console.warn traces and their eslint-disable comments from insertPresetContent, including the start, success, and related insertion-path logs identified in the comment. Leave the surrounding insertion behavior unchanged and do not add replacement logging unless an existing debug-gated logger is already available.src/editor/editors/bricks.js (2)
176-203: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winPrefer
window.bricksDataand cache the parsed payload instead of regex-scraping the inline script on every call.The greedy
(\{[\s\S]*\});capture will over-match if any further};-terminated statement followsbricksDatain that script tag, makingJSON.parsefail and silently disabling every AJAX path.refreshBuilderPreview(Line 273) already readswindow.bricksDatafirst — use that same precedence here and memoize the fallback parse, since this runs on eachgetBuilderPostId/getBuilderArea/runBricksAjaxActioncall.♻️ Proposed change
getBricksBootData() { + if ( window.bricksData ) { + return window.bricksData + } + + if ( this.cachedBricksBootData !== undefined ) { + return this.cachedBricksBootData + } + const dataScript = document.querySelector( '`#bricks-builder-js-extra`' ) const scriptContent = dataScript?.textContent || '' const match = scriptContent.match( /var\s+bricksData\s*=\s*(\{[\s\S]*\});/ ) if ( ! match ) { + this.cachedBricksBootData = null return null } try { - return JSON.parse( match[ 1 ] ) + this.cachedBricksBootData = JSON.parse( match[ 1 ] ) } catch ( error ) { - return null + this.cachedBricksBootData = null } + + return this.cachedBricksBootData }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/editor/editors/bricks.js` around lines 176 - 203, Update getBricksBootData to first return the existing window.bricksData payload, then parse the inline script only as a fallback using a non-greedy, properly bounded match. Cache the parsed fallback result so repeated calls from getBuilderPostId, getBuilderArea, and runBricksAjaxAction do not re-query or re-parse the script, while preserving null behavior when no valid payload exists.
206-237: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winHarden the AJAX helper: no
response.okcheck, no timeout, andresponse.json()can throw.A non-JSON error page (nonce failure, 500, plugin conflict) rejects inside
insertPresetContentwith an unhandled rejection instead of a clean failure, and there's no abort on a hanging request.♻️ Proposed hardening
- const response = await window.fetch( ajaxUrl, { - method: 'POST', - credentials: 'same-origin', - body: formData, - } ) - - return response.json() + try { + const response = await window.fetch( ajaxUrl, { + method: 'POST', + credentials: 'same-origin', + body: formData, + signal: window.AbortSignal?.timeout?.( 30000 ), + } ) + + if ( ! response.ok ) { + return null + } + + return await response.json() + } catch ( error ) { + return null + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/editor/editors/bricks.js` around lines 206 - 237, Harden runBricksAjaxAction by adding an abort timeout for the fetch request, checking response.ok before parsing, and safely handling JSON parse failures or fetch/abort errors. Ensure failures return a consistent null or error result so callers such as insertPresetContent do not receive unhandled rejections, while preserving the existing successful response behavior.src/editor/editors/elementor.js (2)
309-317: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated widget→frontend-selector mapping.
The
button./icon.selector rules here duplicategetWidgetContentTargetSelector(Lines 139-146). Extract a sharedgetWidgetSelectorForType( widgetType, elementId )so both paths stay in sync when new widget types are added.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/editor/editors/elementor.js` around lines 309 - 317, Extract the duplicated widget-to-frontend-selector logic from the current target-value calculation and the existing getWidgetContentTargetSelector path into a shared getWidgetSelectorForType(widgetType, elementId) helper. Update both call sites to use this helper while preserving the existing button, icon, and default selector behavior.
240-245: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueWidget selection with no resolvable parent aborts insertion.
When
elType === 'widget'andcontainer.parentis undefined, the method returnsnullandinsertPresetContentbails without feedback. Consider falling back to the preview container instead of failing.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/editor/editors/elementor.js` around lines 240 - 245, Update the widget branch in the container-resolution method around elementType and insertPresetContent so an unresolved container.parent falls back to the preview container rather than returning null. Preserve returning the resolved parent when available, and ensure widget selection can continue insertion without silently aborting.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/editor/editors/bricks.js`:
- Around line 361-385: Update insertPresetContent to use the live builder
element tree from window.bricksData.loadData when available, falling back to
getBuilderElements only when no in-memory state exists. Merge the preset into
that authoritative current state before saveBuilderElements and
refreshBuilderPreview, preserving unsaved user edits.
In `@src/editor/editors/divi.js`:
- Around line 941-949: Debounce the MutationObserver callback used by
syncBindings so frequent builder mutations schedule only one synchronization
within a short interval, while preserving immediate initial synchronization and
iframe replacement detection. Keep the observer behavior scoped to the existing
builder observation unless a reliable app-window container is already available.
- Around line 316-328: Update saveEditor() to locate Divi’s save control using a
stable, non-localized selector instead of matching the visible “Save” text. Make
it await Divi’s actual save-completed signal before resolving, rather than
resolving immediately after click or relying on the local saving class, so
saveCurrentEditor() only transitions publishing and dirty state after
persistence completes.
- Around line 397-418: Update ensureInteractionTarget to verify the
divi/edit-post store and editModuleAttribute are available before dispatching;
return an empty string when persistence cannot occur, and return the generated
ID only after a successful editModuleAttribute call. Also remove the redundant
10 argument from generateInteractionTargetId, since its configured alphabet
generator already defines the size.
In `@src/editor/editors/elementor.js`:
- Around line 398-419: Update insertPresetContent around the
document/elements/paste command to catch rejected paste errors and return null
consistently with other failure paths. After a successful paste, call
document/save/set-is-modified before getFirstInsertedContainer can return early,
while preserving the existing fallback-target construction for successful
insertions.
- Around line 219-222: Update canInsertPreset to accept both array and
single-object examples supported by normalizeElementorExample, while still
rejecting empty or invalid examples. Align its validation with
normalizeElementorExample so a single copied Elementor element is insertable.
In `@src/editor/editors/gutenberg.js`:
- Around line 135-144: Update insertPresetContent to preserve all blocks
returned by parse() rather than destructuring only the first block. Insert the
complete parsed block collection through
dispatch('core/block-editor').insertBlocks, while retaining the existing
empty-result handling and ensuring targetMappingsSource continues to reference
the appropriate inserted root block.
In `@src/editor/interaction-library/configure-modal.js`:
- Around line 195-218: Update the skip-config rendering branch in the configure
modal to show the loading state only when both selectedPreset.skipConfig and
isApplying are true. When insertLibraryPreset() returns null and clears
isApplying, fall through to the normal apply-button view so the user can retry.
In `@src/editor/interaction-library/util.js`:
- Around line 154-186: Update resolveTargetMappingTarget so that when an
editor-specific resolver is provided but returns a falsy result, it returns null
immediately instead of falling through to the generic blockOrTarget fallback.
Preserve the existing fallback behavior only when no resolver is supplied,
allowing applyTargetMappings to emit its unresolved-target warning.
---
Outside diff comments:
In `@src/editor/components/interaction-panel/index.js`:
- Around line 225-237: Handle rejection from saveCurrentEditor within
onPublishHandler’s promise chain so it cannot become an unhandled rejection.
Preserve the existing finally cleanup that restores idle status and clears the
dirty state, and provide appropriate user-facing feedback for the save failure
using the surrounding interaction-panel error handling conventions.
---
Nitpick comments:
In `@src/editor/editors/bricks.js`:
- Around line 176-203: Update getBricksBootData to first return the existing
window.bricksData payload, then parse the inline script only as a fallback using
a non-greedy, properly bounded match. Cache the parsed fallback result so
repeated calls from getBuilderPostId, getBuilderArea, and runBricksAjaxAction do
not re-query or re-parse the script, while preserving null behavior when no
valid payload exists.
- Around line 206-237: Harden runBricksAjaxAction by adding an abort timeout for
the fetch request, checking response.ok before parsing, and safely handling JSON
parse failures or fetch/abort errors. Ensure failures return a consistent null
or error result so callers such as insertPresetContent do not receive unhandled
rejections, while preserving the existing successful response behavior.
In `@src/editor/editors/divi.js`:
- Around line 800-806: Wrap the external dispatches in the single-root and
multi-root branches of insertPresetContent with try/catch, catching failures
from copyModuleFromPayload and copyModulesFromPayload. Return null on dispatch
failure so the caller can use the existing addModule fallback where applicable,
while preserving normal successful dispatch behavior.
- Around line 626-630: Remove all eight development-only console.warn traces and
their eslint-disable comments from insertPresetContent, including the start,
success, and related insertion-path logs identified in the comment. Leave the
surrounding insertion behavior unchanged and do not add replacement logging
unless an existing debug-gated logger is already available.
In `@src/editor/editors/elementor.js`:
- Around line 309-317: Extract the duplicated widget-to-frontend-selector logic
from the current target-value calculation and the existing
getWidgetContentTargetSelector path into a shared
getWidgetSelectorForType(widgetType, elementId) helper. Update both call sites
to use this helper while preserving the existing button, icon, and default
selector behavior.
- Around line 240-245: Update the widget branch in the container-resolution
method around elementType and insertPresetContent so an unresolved
container.parent falls back to the preview container rather than returning null.
Preserve returning the resolved parent when available, and ensure widget
selection can continue insertion without silently aborting.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4862e162-5bcd-4529-a831-83ccd687a528
📒 Files selected for processing (75)
interactions.phpsrc/editor/app.jssrc/editor/components/interaction-panel/index.jssrc/editor/components/target-selector/index.jssrc/editor/editor.phpsrc/editor/editor.scsssrc/editor/editors/abstract.jssrc/editor/editors/bricks.jssrc/editor/editors/divi.jssrc/editor/editors/elementor.jssrc/editor/editors/gutenberg.jssrc/editor/editors/index.jssrc/editor/interaction-library/bricks-example.jssrc/editor/interaction-library/configure-modal.jssrc/editor/interaction-library/elementor-example.jssrc/editor/interaction-library/index.jssrc/editor/interaction-library/library/background-color-transition.jsonsrc/editor/interaction-library/library/bento-entrance-video.jsonsrc/editor/interaction-library/library/bouncing-button.jsonsrc/editor/interaction-library/library/button-underline.jsonsrc/editor/interaction-library/library/card-1.jsonsrc/editor/interaction-library/library/card-2.jsonsrc/editor/interaction-library/library/card-3.jsonsrc/editor/interaction-library/library/card-4.jsonsrc/editor/interaction-library/library/card-5.jsonsrc/editor/interaction-library/library/card-6.jsonsrc/editor/interaction-library/library/card-7.jsonsrc/editor/interaction-library/library/card-8.jsonsrc/editor/interaction-library/library/card-9.jsonsrc/editor/interaction-library/library/card-flip.jsonsrc/editor/interaction-library/library/columns-fade-in-stagger.jsonsrc/editor/interaction-library/library/columns-slide-in-stagger.jsonsrc/editor/interaction-library/library/confetti.jsonsrc/editor/interaction-library/library/content-stagger.jsonsrc/editor/interaction-library/library/copy-text.jsonsrc/editor/interaction-library/library/counting-up.jsonsrc/editor/interaction-library/library/details-slide-in-open.jsonsrc/editor/interaction-library/library/filling-button.jsonsrc/editor/interaction-library/library/flickering-text.jsonsrc/editor/interaction-library/library/glowing-button.jsonsrc/editor/interaction-library/library/grow-on-scroll.jsonsrc/editor/interaction-library/library/growing-button.jsonsrc/editor/interaction-library/library/hero-1.jsonsrc/editor/interaction-library/library/hero-2.jsonsrc/editor/interaction-library/library/hero-3.jsonsrc/editor/interaction-library/library/hero-4.jsonsrc/editor/interaction-library/library/hero-5.jsonsrc/editor/interaction-library/library/hero-6.jsonsrc/editor/interaction-library/library/hero-7.jsonsrc/editor/interaction-library/library/hero-8.jsonsrc/editor/interaction-library/library/hero-9.jsonsrc/editor/interaction-library/library/image-3d-mouse-tilt.jsonsrc/editor/interaction-library/library/image-blur.jsonsrc/editor/interaction-library/library/image-fade-in-entrance.jsonsrc/editor/interaction-library/library/image-slide-in-entrance.jsonsrc/editor/interaction-library/library/image-zoom-out-rotate.jsonsrc/editor/interaction-library/library/image-zoom.jsonsrc/editor/interaction-library/library/logo-carousel.jsonsrc/editor/interaction-library/library/parallax.jsonsrc/editor/interaction-library/library/play-video.jsonsrc/editor/interaction-library/library/price-calculator.jsonsrc/editor/interaction-library/library/pulsing-button.jsonsrc/editor/interaction-library/library/revealing-text.jsonsrc/editor/interaction-library/library/rotating-image.jsonsrc/editor/interaction-library/library/scroll-to-top.jsonsrc/editor/interaction-library/library/scrub-video.jsonsrc/editor/interaction-library/library/shaking-button.jsonsrc/editor/interaction-library/library/sliding-text.jsonsrc/editor/interaction-library/library/spreading-text.jsonsrc/editor/interaction-library/library/text-fade-in-entrance.jsonsrc/editor/interaction-library/library/text-slide-in-entrance.jsonsrc/editor/interaction-library/library/toggle-details.jsonsrc/editor/interaction-library/preset-schema.jssrc/editor/interaction-library/select-modal.jssrc/editor/interaction-library/util.js
| async insertPresetContent( preset ) { | ||
| if ( ! this.canInsertPreset( preset ) ) { | ||
| return null | ||
| } | ||
|
|
||
| const postId = this.getBuilderPostId() | ||
| const area = this.getBuilderArea() | ||
| if ( ! postId ) { | ||
| return null | ||
| } | ||
|
|
||
| const currentElements = await this.getBuilderElements( postId, area ) | ||
| const inserted = cloneBricksExample( getPresetBuilderExample( preset, 'bricks' ) ) | ||
| const mergedElements = this.insertBricksElements( | ||
| currentElements, | ||
| inserted.elements, | ||
| inserted.rootElementIds | ||
| ) | ||
|
|
||
| const response = await this.saveBuilderElements( postId, area, mergedElements ) | ||
| if ( ! response?.success ) { | ||
| return null | ||
| } | ||
|
|
||
| await this.refreshBuilderPreview( postId, area, mergedElements ) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Insertion reads server-saved elements, so unsaved builder edits are overwritten on save.
getBuilderElements fetches the persisted bricks_get_partial_builder_data payload, but the builder holds the user's live (unsaved) element tree in window.bricksData.loadData. Merging into the server copy and then calling bricks_save_post with it discards every change made since the last save — including the possibility that the freshly refreshed preview no longer matches what the user had on screen. Prefer reading the in-memory builder state (with the AJAX load only as fallback), or route the insert through Bricks' own store so the save path stays authoritative.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/editor/editors/bricks.js` around lines 361 - 385, Update
insertPresetContent to use the live builder element tree from
window.bricksData.loadData when available, falling back to getBuilderElements
only when no in-memory state exists. Merge the preset into that authoritative
current state before saveBuilderElements and refreshBuilderPreview, preserving
unsaved user edits.
| saveEditor() { | ||
| // Reuse Divi's own save button so the builder persists the current page | ||
| // after an interaction modifies module attributes such as target IDs. | ||
| const saveButton = Array.from( document.querySelectorAll( '.et-vb-page-bar-action-button' ) ) | ||
| .find( button => button.textContent?.trim() === 'Save' ) | ||
|
|
||
| if ( ! saveButton || saveButton.disabled ) { | ||
| return Promise.resolve() | ||
| } | ||
|
|
||
| saveButton.click() | ||
| return Promise.resolve() | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how other builder adapters implement saveEditor and whether callers await it.
rg -nP -C5 '\bsaveEditor\s*\(' src/editorRepository: gambitph/Interactions
Length of output: 3146
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== saveCurrentEditor call sites =="
rg -n -C 4 '\bsaveCurrentEditor\s*\(' .
echo
echo "== divi saveEditor region =="
sed -n '1,380p' src/editor/editors/divi.js | nl -ba | sed -n '260,345p'
echo
echo "== editor interface around saveEditor in abstract/gutenberg =="
sed -n '1,170p' src/editor/editors/abstract.js | nl -ba
sed -n '90,145p' src/editor/editors/gutenberg.js | nl -ba
echo
echo "== editor selection tracking region =="
sed -n '1,110p' src/editor/editors/index.js | nl -baRepository: gambitph/Interactions
Length of output: 1199
🌐 Web query:
Divi Visual Builder save button .et-vb-page-bar-action-button English save localization
💡 Result:
The Divi Visual Builder save functionality, including the button associated with the.et-vb-page-bar-action-button class, is part of the core Divi Builder interface and is managed through WordPress internationalization (i18n) systems [1][2]. If you are encountering issues with this button's localization or wish to change its label, you can manage it through the following methods: 1. Translation Files (.po/.mo): Divi uses the et_builder text domain for its builder interface strings [2]. You can locate the official translation files in the Divi/includes/builder/languages directory [1][3]. To create a custom translation or fix errors, copy the.po and.mo files to a child theme's language directory (maintaining the same path structure) and edit the strings using software like Poedit [1][3]. 2. Localization Plugins: For a simpler approach that does not require manual file management, you can use a plugin like Say What [2]. This allows you to override specific strings by providing the original English text and the correct text domain (et_builder for Visual Builder elements) [2]. 3. Multilingual Environments: If you are using a translation plugin like WPML, ensure your WordPress user language settings and site language settings are correctly configured [4]. If the button label appears in the wrong language or remains in English despite your site's language settings, you may need to rescan the theme strings within the WPML plugin settings to ensure all builder interface labels are correctly registered and translated [4]. Note that the Visual Builder interface language is typically determined by the language of the current WordPress editing session [4]. If you are performing technical customizations, ensure you are not modifying core files directly, as these changes will be overwritten during Divi theme updates [3]. Always use a child theme or appropriate localization plugins to ensure your changes persist [1][3].
Citations:
- 1: https://help.elegantthemes.com/en/articles/2434045-how-to-translate-the-divi-theme
- 2: https://help.elegantthemes.com/en/articles/2716960-how-to-correct-translation-errors
- 3: https://divitheme.net/modify-divi-translation/
- 4: https://wpml.org/forums/topic/divi-labels-wrong-language/
🏁 Script executed:
#!/bin/bash
set -u
echo "== available tools =="
command -v sed awk cat || true
echo
echo "== divi saveEditor region =="
sed -n '260,350p' src/editor/editors/divi.js
echo
echo "== editor base files =="
sed -n '1,170p' src/editor/editors/abstract.js
sed -n '90,160p' src/editor/editors/gutenberg.js
sed -n '1,80p' src/editor/editors/index.js
echo
echo "== call sites =="
sed -n '200,260p' src/editor/components/interaction-panel/index.js
rg -n -C 3 '\bsaveCurrentEditor\s*\(|\.saveEditor\s*\(' src/editor || true
echo
echo "== selectors/usages =="
rg -n "\.et-vb-page-bar-action-button|textContent|disabled|Promise\.resolve|savePost" src/editor -g '*.js' || trueRepository: gambitph/Interactions
Length of output: 18117
Use a stable Divi save button selector and await the save cycle.
button.textContent?.trim() === 'Save' relies on an English label that can change under Divi/WordPress localization. Also, the call site sets publishing to idle and clears the dirty state immediately after saveCurrentEditor() resolves, so this resolves with no wait for Divi persistence; gate these transitions on an actual save-completed signal instead of the local saving class.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/editor/editors/divi.js` around lines 316 - 328, Update saveEditor() to
locate Divi’s save control using a stable, non-localized selector instead of
matching the visible “Save” text. Make it await Divi’s actual save-completed
signal before resolving, rather than resolving immediately after click or
relying on the local saving class, so saveCurrentEditor() only transitions
publishing and dirty state after persistence completes.
| ensureInteractionTarget( moduleId ) { | ||
| if ( ! moduleId ) { | ||
| return '' | ||
| } | ||
|
|
||
| const existingTarget = this.getStoredInteractionTarget( moduleId ) | ||
| if ( existingTarget ) { | ||
| return existingTarget | ||
| } | ||
|
|
||
| // Persist the target on the Divi module itself so the same identifier is | ||
| // rendered in both the builder and the frontend output. | ||
| const targetId = generateInteractionTargetId( 10 ) | ||
| this.getDiviDataApi()?.dispatch?.( 'divi/edit-post' )?.editModuleAttribute?.( { | ||
| id: moduleId, | ||
| attrName: 'module.decoration.interactionTarget', | ||
| value: targetId, | ||
| caller: 'user', | ||
| subName: false, | ||
| } ) | ||
| return targetId | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
ensureInteractionTarget returns an ID it may never have persisted.
Every hop is optional-chained, so if the divi/edit-post store or editModuleAttribute is unavailable the dispatch is a no-op, yet the generated targetId is still returned. Callers (buildTargetFromElement, buildTargetFromModuleId) then produce a [data-interaction-target="…"] selector that matches nothing, and the interaction silently does nothing. Return '' when the dispatch isn't available so the caller can bail out.
Also, generateInteractionTargetId( 10 ) is redundant — customAlphabet( alphabet, 10 ) already fixes the default size (Line 20).
🐛 Proposed fix
- const targetId = generateInteractionTargetId( 10 )
- this.getDiviDataApi()?.dispatch?.( 'divi/edit-post' )?.editModuleAttribute?.( {
+ const editModuleAttribute = this.getDiviDataApi()?.dispatch?.( 'divi/edit-post' )?.editModuleAttribute
+ if ( typeof editModuleAttribute !== 'function' ) {
+ return ''
+ }
+
+ const targetId = generateInteractionTargetId()
+ editModuleAttribute( {
id: moduleId,
attrName: 'module.decoration.interactionTarget',
value: targetId,
caller: 'user',
subName: false,
} )
return targetId📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ensureInteractionTarget( moduleId ) { | |
| if ( ! moduleId ) { | |
| return '' | |
| } | |
| const existingTarget = this.getStoredInteractionTarget( moduleId ) | |
| if ( existingTarget ) { | |
| return existingTarget | |
| } | |
| // Persist the target on the Divi module itself so the same identifier is | |
| // rendered in both the builder and the frontend output. | |
| const targetId = generateInteractionTargetId( 10 ) | |
| this.getDiviDataApi()?.dispatch?.( 'divi/edit-post' )?.editModuleAttribute?.( { | |
| id: moduleId, | |
| attrName: 'module.decoration.interactionTarget', | |
| value: targetId, | |
| caller: 'user', | |
| subName: false, | |
| } ) | |
| return targetId | |
| } | |
| ensureInteractionTarget( moduleId ) { | |
| if ( ! moduleId ) { | |
| return '' | |
| } | |
| const existingTarget = this.getStoredInteractionTarget( moduleId ) | |
| if ( existingTarget ) { | |
| return existingTarget | |
| } | |
| // Persist the target on the Divi module itself so the same identifier is | |
| // rendered in both the builder and the frontend output. | |
| const editModuleAttribute = this.getDiviDataApi()?.dispatch?.( 'divi/edit-post' )?.editModuleAttribute | |
| if ( typeof editModuleAttribute !== 'function' ) { | |
| return '' | |
| } | |
| const targetId = generateInteractionTargetId() | |
| editModuleAttribute( { | |
| id: moduleId, | |
| attrName: 'module.decoration.interactionTarget', | |
| value: targetId, | |
| caller: 'user', | |
| subName: false, | |
| } ) | |
| return targetId | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/editor/editors/divi.js` around lines 397 - 418, Update
ensureInteractionTarget to verify the divi/edit-post store and
editModuleAttribute are available before dispatching; return an empty string
when persistence cannot occur, and return the generated ID only after a
successful editModuleAttribute call. Also remove the redundant 10 argument from
generateInteractionTargetId, since its configured alphabet generator already
defines the size.
| syncBindings() | ||
|
|
||
| // Keep watching for iframe replacement because Divi can recreate the app | ||
| // window during builder navigation without reloading the top document. | ||
| observer = new MutationObserver( syncBindings ) | ||
| observer.observe( document.body, { | ||
| childList: true, | ||
| subtree: true, | ||
| } ) |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Unthrottled MutationObserver over the whole builder DOM.
syncBindings runs on every mutation batch anywhere under document.body with subtree: true, and each run does a document.querySelector plus getCanvasDocument() lookup. Divi's Visual Builder mutates the DOM constantly while dragging/editing, so this is a hot path. Debounce it (or narrow the observed subtree to the app-window container).
♻️ Debounce the observer callback
- observer = new MutationObserver( syncBindings )
+ let syncFrame = null
+ const scheduleSync = () => {
+ if ( syncFrame ) {
+ return
+ }
+ syncFrame = window.requestAnimationFrame( () => {
+ syncFrame = null
+ syncBindings()
+ } )
+ }
+ observer = new MutationObserver( scheduleSync )
observer.observe( document.body, {
childList: true,
subtree: true,
} )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| syncBindings() | |
| // Keep watching for iframe replacement because Divi can recreate the app | |
| // window during builder navigation without reloading the top document. | |
| observer = new MutationObserver( syncBindings ) | |
| observer.observe( document.body, { | |
| childList: true, | |
| subtree: true, | |
| } ) | |
| syncBindings() | |
| // Keep watching for iframe replacement because Divi can recreate the app | |
| // window during builder navigation without reloading the top document. | |
| let syncFrame = null | |
| const scheduleSync = () => { | |
| if ( syncFrame ) { | |
| return | |
| } | |
| syncFrame = window.requestAnimationFrame( () => { | |
| syncFrame = null | |
| syncBindings() | |
| } ) | |
| } | |
| observer = new MutationObserver( scheduleSync ) | |
| observer.observe( document.body, { | |
| childList: true, | |
| subtree: true, | |
| } ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/editor/editors/divi.js` around lines 941 - 949, Debounce the
MutationObserver callback used by syncBindings so frequent builder mutations
schedule only one synchronization within a short interval, while preserving
immediate initial synchronization and iframe replacement detection. Keep the
observer behavior scoped to the existing builder observation unless a reliable
app-window container is already available.
| canInsertPreset( preset ) { | ||
| const example = getPresetBuilderExample( preset, 'elementor' ) | ||
| return Array.isArray( example ) && example.length > 0 | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
canInsertPreset rejects single-object examples that normalizeElementorExample explicitly supports.
normalizeElementorExample wraps a non-array example into an array, but this gate requires Array.isArray(...), so a preset storing a single copied Elementor element is silently un-insertable. Align the two contracts.
🔧 Proposed fix
canInsertPreset( preset ) {
const example = getPresetBuilderExample( preset, 'elementor' )
- return Array.isArray( example ) && example.length > 0
+ if ( ! example ) {
+ return false
+ }
+ return Array.isArray( example ) ? example.length > 0 : typeof example === 'object'
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| canInsertPreset( preset ) { | |
| const example = getPresetBuilderExample( preset, 'elementor' ) | |
| return Array.isArray( example ) && example.length > 0 | |
| } | |
| canInsertPreset( preset ) { | |
| const example = getPresetBuilderExample( preset, 'elementor' ) | |
| if ( ! example ) { | |
| return false | |
| } | |
| return Array.isArray( example ) ? example.length > 0 : typeof example === 'object' | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/editor/editors/elementor.js` around lines 219 - 222, Update
canInsertPreset to accept both array and single-object examples supported by
normalizeElementorExample, while still rejecting empty or invalid examples.
Align its validation with normalizeElementorExample so a single copied Elementor
element is insertable.
| const inserted = await window.$e.run( 'document/elements/paste', { | ||
| container, | ||
| rebuild: true, | ||
| storageType: 'json', | ||
| data: JSON.stringify( { | ||
| type: 'elementor', | ||
| elements: normalizedExample, | ||
| } ), | ||
| options: { | ||
| at: container.view?.collection?.length, | ||
| }, | ||
| } ) | ||
|
|
||
| // The first inserted container becomes the fallback target for presets | ||
| // that do not declare target mappings. | ||
| const firstInsertedContainer = this.getFirstInsertedContainer( inserted ) | ||
| if ( ! firstInsertedContainer ) { | ||
| return null | ||
| } | ||
| const defaultTarget = this.buildTargetFromContainer( firstInsertedContainer ) | ||
|
|
||
| window.$e.internal?.( 'document/save/set-is-modified', { status: true } ) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Paste is unguarded, and the dirty flag is set after an early return.
Two issues in this block:
window.$e.run( 'document/elements/paste', ... )has notry/catch. Every other failure path returnsnull, but a rejected paste rejectsinsertPresetContent, which the shared library flow likely does not expect.- If
getFirstInsertedContainerreturns nothing (Line 414), the method returns early after the document was already mutated, sodocument/save/set-is-modifiedis never called and the user's inserted content can be lost on navigation.
Set the modified flag immediately after a successful paste and wrap the command.
🐛 Proposed fix
- const inserted = await window.$e.run( 'document/elements/paste', {
- container,
- rebuild: true,
- storageType: 'json',
- data: JSON.stringify( {
- type: 'elementor',
- elements: normalizedExample,
- } ),
- options: {
- at: container.view?.collection?.length,
- },
- } )
+ let inserted = null
+ try {
+ inserted = await window.$e.run( 'document/elements/paste', {
+ container,
+ rebuild: true,
+ storageType: 'json',
+ data: JSON.stringify( {
+ type: 'elementor',
+ elements: normalizedExample,
+ } ),
+ options: {
+ at: container.view?.collection?.length,
+ },
+ } )
+ } catch ( error ) {
+ console.error( 'Interactions: Elementor preset paste failed', error ) // eslint-disable-line no-console
+ return null
+ }
+
+ window.$e.internal?.( 'document/save/set-is-modified', { status: true } )
// The first inserted container becomes the fallback target for presets
// that do not declare target mappings.
const firstInsertedContainer = this.getFirstInsertedContainer( inserted )
if ( ! firstInsertedContainer ) {
return null
}
const defaultTarget = this.buildTargetFromContainer( firstInsertedContainer )
-
- window.$e.internal?.( 'document/save/set-is-modified', { status: true } )
const targetRefs = getPresetBuilderTargetRefs( preset, 'elementor' )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const inserted = await window.$e.run( 'document/elements/paste', { | |
| container, | |
| rebuild: true, | |
| storageType: 'json', | |
| data: JSON.stringify( { | |
| type: 'elementor', | |
| elements: normalizedExample, | |
| } ), | |
| options: { | |
| at: container.view?.collection?.length, | |
| }, | |
| } ) | |
| // The first inserted container becomes the fallback target for presets | |
| // that do not declare target mappings. | |
| const firstInsertedContainer = this.getFirstInsertedContainer( inserted ) | |
| if ( ! firstInsertedContainer ) { | |
| return null | |
| } | |
| const defaultTarget = this.buildTargetFromContainer( firstInsertedContainer ) | |
| window.$e.internal?.( 'document/save/set-is-modified', { status: true } ) | |
| let inserted = null | |
| try { | |
| inserted = await window.$e.run( 'document/elements/paste', { | |
| container, | |
| rebuild: true, | |
| storageType: 'json', | |
| data: JSON.stringify( { | |
| type: 'elementor', | |
| elements: normalizedExample, | |
| } ), | |
| options: { | |
| at: container.view?.collection?.length, | |
| }, | |
| } ) | |
| } catch ( error ) { | |
| console.error( 'Interactions: Elementor preset paste failed', error ) // eslint-disable-line no-console | |
| return null | |
| } | |
| window.$e.internal?.( 'document/save/set-is-modified', { status: true } ) | |
| // The first inserted container becomes the fallback target for presets | |
| // that do not declare target mappings. | |
| const firstInsertedContainer = this.getFirstInsertedContainer( inserted ) | |
| if ( ! firstInsertedContainer ) { | |
| return null | |
| } | |
| const defaultTarget = this.buildTargetFromContainer( firstInsertedContainer ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/editor/editors/elementor.js` around lines 398 - 419, Update
insertPresetContent around the document/elements/paste command to catch rejected
paste errors and return null consistently with other failure paths. After a
successful paste, call document/save/set-is-modified before
getFirstInsertedContainer can return early, while preserving the existing
fallback-target construction for successful insertions.
| insertPresetContent( preset ) { | ||
| const [ block ] = parse( getPresetBuilderExample( preset, 'gutenberg' ) ?? '' ) | ||
| if ( ! block ) { | ||
| return null | ||
| } | ||
|
|
||
| dispatch( 'core/block-editor' ).insertBlocks( block ) | ||
|
|
||
| return { | ||
| targetMappingsSource: block, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Preserve every parsed Gutenberg root block.
parse() can return multiple blocks, but only block is inserted here. Multi-root preset examples will be truncated.
Proposed fix
- const [ block ] = parse( getPresetBuilderExample( preset, 'gutenberg' ) ?? '' )
+ const blocks = parse( getPresetBuilderExample( preset, 'gutenberg' ) ?? '' )
+ const [ block ] = blocks
if ( ! block ) {
return null
}
- dispatch( 'core/block-editor' ).insertBlocks( block )
+ dispatch( 'core/block-editor' ).insertBlocks( blocks )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| insertPresetContent( preset ) { | |
| const [ block ] = parse( getPresetBuilderExample( preset, 'gutenberg' ) ?? '' ) | |
| if ( ! block ) { | |
| return null | |
| } | |
| dispatch( 'core/block-editor' ).insertBlocks( block ) | |
| return { | |
| targetMappingsSource: block, | |
| insertPresetContent( preset ) { | |
| const blocks = parse( getPresetBuilderExample( preset, 'gutenberg' ) ?? '' ) | |
| const [ block ] = blocks | |
| if ( ! block ) { | |
| return null | |
| } | |
| dispatch( 'core/block-editor' ).insertBlocks( blocks ) | |
| return { | |
| targetMappingsSource: block, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/editor/editors/gutenberg.js` around lines 135 - 144, Update
insertPresetContent to preserve all blocks returned by parse() rather than
destructuring only the first block. Insert the complete parsed block collection
through dispatch('core/block-editor').insertBlocks, while retaining the existing
empty-result handling and ensuring targetMappingsSource continues to reference
the appropriate inserted root block.
| // Skip-config presets apply immediately and do not render controls. | ||
| if ( selectedPreset.skipConfig ) { | ||
| return ( | ||
| <div | ||
| className="interact-interaction-library__configure-loading" | ||
| style={ { | ||
| minHeight: '260px', | ||
| display: 'flex', | ||
| flexDirection: 'column', | ||
| alignItems: 'center', | ||
| justifyContent: 'center', | ||
| gap: '12px', | ||
| textAlign: 'center', | ||
| } } | ||
| > | ||
| <Spinner /> | ||
| <p style={ { margin: 0 } }> | ||
| { mode === 'insert' | ||
| ? __( 'Inserting interaction…', 'interactions' ) | ||
| : __( 'Applying interaction…', 'interactions' ) | ||
| } | ||
| </p> | ||
| </div> | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not render a permanent loader after insertion fails.
When insertLibraryPreset() returns null, Line 110 clears isApplying, but this branch still renders the spinner because it only checks skipConfig. The user cannot retry from this view. Gate the loader on isApplying so the normal apply button is restored after the handled failure.
Proposed fix
- if ( selectedPreset.skipConfig ) {
+ if ( selectedPreset.skipConfig && isApplying ) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Skip-config presets apply immediately and do not render controls. | |
| if ( selectedPreset.skipConfig ) { | |
| return ( | |
| <div | |
| className="interact-interaction-library__configure-loading" | |
| style={ { | |
| minHeight: '260px', | |
| display: 'flex', | |
| flexDirection: 'column', | |
| alignItems: 'center', | |
| justifyContent: 'center', | |
| gap: '12px', | |
| textAlign: 'center', | |
| } } | |
| > | |
| <Spinner /> | |
| <p style={ { margin: 0 } }> | |
| { mode === 'insert' | |
| ? __( 'Inserting interaction…', 'interactions' ) | |
| : __( 'Applying interaction…', 'interactions' ) | |
| } | |
| </p> | |
| </div> | |
| ) | |
| // Skip-config presets apply immediately and do not render controls. | |
| if ( selectedPreset.skipConfig && isApplying ) { | |
| return ( | |
| <div | |
| className="interact-interaction-library__configure-loading" | |
| style={ { | |
| minHeight: '260px', | |
| display: 'flex', | |
| flexDirection: 'column', | |
| alignItems: 'center', | |
| justifyContent: 'center', | |
| gap: '12px', | |
| textAlign: 'center', | |
| } } | |
| > | |
| <Spinner /> | |
| <p style={ { margin: 0 } }> | |
| { mode === 'insert' | |
| ? __( 'Inserting interaction…', 'interactions' ) | |
| : __( 'Applying interaction…', 'interactions' ) | |
| } | |
| </p> | |
| </div> | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/editor/interaction-library/configure-modal.js` around lines 195 - 218,
Update the skip-config rendering branch in the configure modal to show the
loading state only when both selectedPreset.skipConfig and isApplying are true.
When insertLibraryPreset() returns null and clears isApplying, fall through to
the normal apply-button view so the user can retry.
| /** | ||
| * Resolve a mapping entry into an interaction target object. | ||
| * | ||
| * @param {Object} blockOrTarget - Inserted Gutenberg block tree or direct target. | ||
| * @param {Object} mapping - Target mapping definition for one assignment. | ||
| * @param {Object} targetRefs - Preset target ref definitions. | ||
| * @param {?Function} resolver - Optional editor-specific resolver. | ||
| * | ||
| * @return {?Object} The resolved interaction target object. | ||
| */ | ||
| const resolveTargetMappingTarget = ( blockOrTarget, mapping = {}, targetRefs = {}, resolver = null ) => { | ||
| if ( typeof resolver === 'function' ) { | ||
| const resolvedTarget = resolver( mapping ) | ||
| if ( resolvedTarget ) { | ||
| return resolvedTarget | ||
| } | ||
| } | ||
|
|
||
| if ( ! blockOrTarget?.clientId ) { | ||
| return blockOrTarget | ||
| } | ||
|
|
||
| const resolvedBlockPath = Array.isArray( mapping.blockPath ) | ||
| ? mapping.blockPath | ||
| : getTargetRefPath( targetRefs, mapping.targetRef ) | ||
|
|
||
| if ( ! Array.isArray( resolvedBlockPath ) ) { | ||
| return null | ||
| } | ||
|
|
||
| const block = getValueAtPath( blockOrTarget, resolvedBlockPath ) | ||
| return block?.clientId ? createTargetObj( block ) : null | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Resolver failure silently falls back to the wrong target instead of null.
When resolver is provided (builder insert-mode path) but returns falsy for a given mapping, execution falls through to if ( ! blockOrTarget?.clientId ) return blockOrTarget. Since non-Gutenberg blockOrTarget values never have .clientId, this branch always fires on resolver failure, silently substituting the generic inserted-content descriptor for that mapping's target instead of triggering the existing "target mapping could not be resolved" warning further up the call chain (applyTargetMappings, Lines 220-228). For presets with multiple targetMappings entries, this can make unrelated interaction paths collapse to the same (likely malformed) value with no diagnostic.
🐛 Proposed fix
const resolveTargetMappingTarget = ( blockOrTarget, mapping = {}, targetRefs = {}, resolver = null ) => {
if ( typeof resolver === 'function' ) {
- const resolvedTarget = resolver( mapping )
- if ( resolvedTarget ) {
- return resolvedTarget
- }
+ // A resolver was supplied specifically to resolve this mapping; if it can't,
+ // treat it as unresolved instead of silently reusing blockOrTarget.
+ return resolver( mapping ) || null
}
if ( ! blockOrTarget?.clientId ) {
return blockOrTarget
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * Resolve a mapping entry into an interaction target object. | |
| * | |
| * @param {Object} blockOrTarget - Inserted Gutenberg block tree or direct target. | |
| * @param {Object} mapping - Target mapping definition for one assignment. | |
| * @param {Object} targetRefs - Preset target ref definitions. | |
| * @param {?Function} resolver - Optional editor-specific resolver. | |
| * | |
| * @return {?Object} The resolved interaction target object. | |
| */ | |
| const resolveTargetMappingTarget = ( blockOrTarget, mapping = {}, targetRefs = {}, resolver = null ) => { | |
| if ( typeof resolver === 'function' ) { | |
| const resolvedTarget = resolver( mapping ) | |
| if ( resolvedTarget ) { | |
| return resolvedTarget | |
| } | |
| } | |
| if ( ! blockOrTarget?.clientId ) { | |
| return blockOrTarget | |
| } | |
| const resolvedBlockPath = Array.isArray( mapping.blockPath ) | |
| ? mapping.blockPath | |
| : getTargetRefPath( targetRefs, mapping.targetRef ) | |
| if ( ! Array.isArray( resolvedBlockPath ) ) { | |
| return null | |
| } | |
| const block = getValueAtPath( blockOrTarget, resolvedBlockPath ) | |
| return block?.clientId ? createTargetObj( block ) : null | |
| } | |
| /** | |
| * Resolve a mapping entry into an interaction target object. | |
| * | |
| * `@param` {Object} blockOrTarget - Inserted Gutenberg block tree or direct target. | |
| * `@param` {Object} mapping - Target mapping definition for one assignment. | |
| * `@param` {Object} targetRefs - Preset target ref definitions. | |
| * `@param` {?Function} resolver - Optional editor-specific resolver. | |
| * | |
| * `@return` {?Object} The resolved interaction target object. | |
| */ | |
| const resolveTargetMappingTarget = ( blockOrTarget, mapping = {}, targetRefs = {}, resolver = null ) => { | |
| if ( typeof resolver === 'function' ) { | |
| // A resolver was supplied specifically to resolve this mapping; if it can't, | |
| // treat it as unresolved instead of silently reusing blockOrTarget. | |
| return resolver( mapping ) || null | |
| } | |
| if ( ! blockOrTarget?.clientId ) { | |
| return blockOrTarget | |
| } | |
| const resolvedBlockPath = Array.isArray( mapping.blockPath ) | |
| ? mapping.blockPath | |
| : getTargetRefPath( targetRefs, mapping.targetRef ) | |
| if ( ! Array.isArray( resolvedBlockPath ) ) { | |
| return null | |
| } | |
| const block = getValueAtPath( blockOrTarget, resolvedBlockPath ) | |
| return block?.clientId ? createTargetObj( block ) : null | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/editor/interaction-library/util.js` around lines 154 - 186, Update
resolveTargetMappingTarget so that when an editor-specific resolver is provided
but returns a falsy result, it returns null immediately instead of falling
through to the generic blockOrTarget fallback. Preserve the existing fallback
behavior only when no resolver is supplied, allowing applyTargetMappings to emit
its unresolved-target warning.
fixes #42
Summary by CodeRabbit
New Features
Bug Fixes