Skip to content

Interaction Library for Builders - #41

Open
Arukuen wants to merge 6 commits into
developfrom
feat/builder-library
Open

Interaction Library for Builders#41
Arukuen wants to merge 6 commits into
developfrom
feat/builder-library

Conversation

@Arukuen

@Arukuen Arukuen commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

fixes #42

Summary by CodeRabbit

  • New Features

    • Added Interaction Library support for applying and inserting presets directly within builder editors.
    • Added Divi Visual Builder integration, including element selection, preset insertion, and target synchronization.
    • Added preset support for Elementor, Bricks, Gutenberg, and Divi with builder-specific examples and target mapping.
    • Added builder-aware Interaction Library controls with Apply and Insert actions.
  • Bug Fixes

    • Improved target selection and compatibility messaging across editors.
    • Ensured editor changes complete saving before status cleanup and callbacks run.

@Arukuen Arukuen changed the title feat/builder library Interaction Library for Builders Jul 28, 2026
@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The 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.

Changes

Builder-aware interaction library

Layer / File(s) Summary
Editor entry points and shared contracts
interactions.php, src/editor/app.js, src/editor/components/..., src/editor/editor.php, src/editor/editors/abstract.js, src/editor/editors/index.js
Builder launch controls, modal state, target selection, save handling, Divi loading, and shared editor adapter methods now support preset insertion.
Preset schemas and target mapping
src/editor/interaction-library/preset-schema.js, src/editor/interaction-library/util.js, src/editor/interaction-library/*example.js, src/editor/interaction-library/library/*
Presets now provide builder-specific examples and semantic target references, with normalization and mapping support across editor formats.
Gutenberg, Elementor, and Bricks insertion
src/editor/editors/gutenberg.js, src/editor/editors/elementor.js, src/editor/editors/bricks.js
The shared library is mounted in each editor and preset insertion returns builder-specific content and target mappings.
Divi Visual Builder adapter
src/editor/editors/divi.js, src/editor/editor.scss
Divi receives iframe selection tracking, module insertion, target persistence, element picking, and full-height panel/launcher styling.

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
Loading

Possibly related PRs

Suggested reviewers: bfintal

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title is concise and matches the main change: expanding the interaction library for builder editors.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/builder-library

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown

🤖 Pull request artifacts

file commit
pr41-interactions-41-merge.zip 95083e8

github-actions Bot added a commit that referenced this pull request Jul 28, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 win

Missing 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 of onPublishHandler uncaught (this function's return value isn't awaited by its callers — the button onClick and the interact/save-interaction event 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 win

Consider guarding the external builder dispatches.

copyModuleFromPayload / copyModulesFromPayload are Divi-internal APIs invoked with a hand-built payload; a throw here rejects insertPresetContent and surfaces as an unhandled rejection in the library modal flow. A try/catch that returns null (falling back to the addModule path 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 win

Remove the console.warn debug tracing before merge.

insertPresetContent logs eight console.warn traces (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 win

Prefer window.bricksData and 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 follows bricksData in that script tag, making JSON.parse fail and silently disabling every AJAX path. refreshBuilderPreview (Line 273) already reads window.bricksData first — use that same precedence here and memoize the fallback parse, since this runs on each getBuilderPostId/getBuilderArea/runBricksAjaxAction call.

♻️ 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 win

Harden the AJAX helper: no response.ok check, no timeout, and response.json() can throw.

A non-JSON error page (nonce failure, 500, plugin conflict) rejects inside insertPresetContent with 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 win

Duplicated widget→frontend-selector mapping.

The button. / icon. selector rules here duplicate getWidgetContentTargetSelector (Lines 139-146). Extract a shared getWidgetSelectorForType( 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 value

Widget selection with no resolvable parent aborts insertion.

When elType === 'widget' and container.parent is undefined, the method returns null and insertPresetContent bails 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

📥 Commits

Reviewing files that changed from the base of the PR and between 3e07105 and 95083e8.

📒 Files selected for processing (75)
  • interactions.php
  • src/editor/app.js
  • src/editor/components/interaction-panel/index.js
  • src/editor/components/target-selector/index.js
  • src/editor/editor.php
  • src/editor/editor.scss
  • src/editor/editors/abstract.js
  • src/editor/editors/bricks.js
  • src/editor/editors/divi.js
  • src/editor/editors/elementor.js
  • src/editor/editors/gutenberg.js
  • src/editor/editors/index.js
  • src/editor/interaction-library/bricks-example.js
  • src/editor/interaction-library/configure-modal.js
  • src/editor/interaction-library/elementor-example.js
  • src/editor/interaction-library/index.js
  • src/editor/interaction-library/library/background-color-transition.json
  • src/editor/interaction-library/library/bento-entrance-video.json
  • src/editor/interaction-library/library/bouncing-button.json
  • src/editor/interaction-library/library/button-underline.json
  • src/editor/interaction-library/library/card-1.json
  • src/editor/interaction-library/library/card-2.json
  • src/editor/interaction-library/library/card-3.json
  • src/editor/interaction-library/library/card-4.json
  • src/editor/interaction-library/library/card-5.json
  • src/editor/interaction-library/library/card-6.json
  • src/editor/interaction-library/library/card-7.json
  • src/editor/interaction-library/library/card-8.json
  • src/editor/interaction-library/library/card-9.json
  • src/editor/interaction-library/library/card-flip.json
  • src/editor/interaction-library/library/columns-fade-in-stagger.json
  • src/editor/interaction-library/library/columns-slide-in-stagger.json
  • src/editor/interaction-library/library/confetti.json
  • src/editor/interaction-library/library/content-stagger.json
  • src/editor/interaction-library/library/copy-text.json
  • src/editor/interaction-library/library/counting-up.json
  • src/editor/interaction-library/library/details-slide-in-open.json
  • src/editor/interaction-library/library/filling-button.json
  • src/editor/interaction-library/library/flickering-text.json
  • src/editor/interaction-library/library/glowing-button.json
  • src/editor/interaction-library/library/grow-on-scroll.json
  • src/editor/interaction-library/library/growing-button.json
  • src/editor/interaction-library/library/hero-1.json
  • src/editor/interaction-library/library/hero-2.json
  • src/editor/interaction-library/library/hero-3.json
  • src/editor/interaction-library/library/hero-4.json
  • src/editor/interaction-library/library/hero-5.json
  • src/editor/interaction-library/library/hero-6.json
  • src/editor/interaction-library/library/hero-7.json
  • src/editor/interaction-library/library/hero-8.json
  • src/editor/interaction-library/library/hero-9.json
  • src/editor/interaction-library/library/image-3d-mouse-tilt.json
  • src/editor/interaction-library/library/image-blur.json
  • src/editor/interaction-library/library/image-fade-in-entrance.json
  • src/editor/interaction-library/library/image-slide-in-entrance.json
  • src/editor/interaction-library/library/image-zoom-out-rotate.json
  • src/editor/interaction-library/library/image-zoom.json
  • src/editor/interaction-library/library/logo-carousel.json
  • src/editor/interaction-library/library/parallax.json
  • src/editor/interaction-library/library/play-video.json
  • src/editor/interaction-library/library/price-calculator.json
  • src/editor/interaction-library/library/pulsing-button.json
  • src/editor/interaction-library/library/revealing-text.json
  • src/editor/interaction-library/library/rotating-image.json
  • src/editor/interaction-library/library/scroll-to-top.json
  • src/editor/interaction-library/library/scrub-video.json
  • src/editor/interaction-library/library/shaking-button.json
  • src/editor/interaction-library/library/sliding-text.json
  • src/editor/interaction-library/library/spreading-text.json
  • src/editor/interaction-library/library/text-fade-in-entrance.json
  • src/editor/interaction-library/library/text-slide-in-entrance.json
  • src/editor/interaction-library/library/toggle-details.json
  • src/editor/interaction-library/preset-schema.js
  • src/editor/interaction-library/select-modal.js
  • src/editor/interaction-library/util.js

Comment on lines +361 to +385
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 )

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ 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.

Comment on lines +316 to +328
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()
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 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/editor

Repository: 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 -ba

Repository: 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:


🏁 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' || true

Repository: 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.

Comment on lines +397 to +418
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
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 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.

Suggested change
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.

Comment on lines +941 to +949
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,
} )

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀 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.

Suggested change
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.

Comment on lines +219 to +222
canInsertPreset( preset ) {
const example = getPresetBuilderExample( preset, 'elementor' )
return Array.isArray( example ) && example.length > 0
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 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.

Suggested change
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.

Comment on lines +398 to +419
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 } )

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Paste is unguarded, and the dirty flag is set after an early return.

Two issues in this block:

  1. window.$e.run( 'document/elements/paste', ... ) has no try/catch. Every other failure path returns null, but a rejected paste rejects insertPresetContent, which the shared library flow likely does not expect.
  2. If getFirstInsertedContainer returns nothing (Line 414), the method returns early after the document was already mutated, so document/save/set-is-modified is 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.

Suggested change
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.

Comment on lines +135 to +144
insertPresetContent( preset ) {
const [ block ] = parse( getPresetBuilderExample( preset, 'gutenberg' ) ?? '' )
if ( ! block ) {
return null
}

dispatch( 'core/block-editor' ).insertBlocks( block )

return {
targetMappingsSource: block,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 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.

Suggested change
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.

Comment on lines +195 to +218
// 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>
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 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.

Suggested change
// 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.

Comment on lines +154 to +186
/**
* 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
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ 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.

Suggested change
/**
* 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Interaction Library for Builders

1 participant