Open
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR implements an editor-adapter facade with Gutenberg and Elementor adapters, adds a unified React InteractionsApp, and migrates components, hooks, timeline code, plugins, and PHP enqueue logic to use the facade for editor context, canvas access, and element-picking. ChangesElementor Editor Integration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
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/location-rules/index.js (1)
99-105:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDead condition:
locations.length === 0is alwaysfalseinsideonClickAnd.The
onClickAndhandler is only reachable from insidelocations.map(...), solocations.length ≥ 1at the call site.getCurrentPostId()on line 100 is unreachable;valuewill always be''. Contrast this with line 117 (the "Add rule group" button handler), wherelocations.length === 0is genuinely possible — that usage is correct. Simplify line 100 to remove the misleading guard:🐛 Proposed fix
onClickAnd={ () => { - const value = locations.length === 0 ? getCurrentPostId() : '' onChange( addLocation( locations, i, k + 1, { param: 'post', operator: '==', - value: value || '', + value: '', } ) ) } }🤖 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/location-rules/index.js` around lines 99 - 105, In the onClickAnd handler (the callback passed as onClickAnd inside the locations.map loop), remove the dead guard locations.length === 0 and simplify the value assignment to always be an empty string; update the call site that invokes onChange(addLocation(locations, i, k + 1, { param: 'post', operator: '==', value: '' })) so getCurrentPostId() is not referenced in this handler (leave other handlers such as the "Add rule group" button untouched where locations.length === 0 is meaningful).
🧹 Nitpick comments (3)
src/editor/components/timeline/runner.js (1)
45-46: ⚡ Quick win
renderingModecomputed with bareselect()won't trigger re-renders when the store value changes.
@wordpress/data's docs explicitly warn: "Don't useuseSelectthis way when calling the selectors in the render function because your component won't re-render on a data change." SincerenderingModelives in theuseEffectdependency array (line 82), store changes to rendering mode won't fire that effect unless something else forces a re-render — meaning therequestAnimationFramerace-condition guard at line 76 can silently be skipped.♻️ Suggested fix — use
useSelectfor reactivity-import { select } from '@wordpress/data' +import { useSelect } from '@wordpress/data' import { getEditorMode } from '~interact/editor/editors' export const useTimelineRunnerRef = ( interaction, actions, timelineIndex ) => { const runnerRef = useRef( null ) const [ initialStyles, setInitialStyles ] = useState( '' ) - const renderingMode = select( 'core/editor' )?.getRenderingMode?.() || getEditorMode() + const renderingMode = useSelect( select => + select( 'core/editor' )?.getRenderingMode?.() || getEditorMode() + ) const prevRenderingMode = useRef( renderingMode )🤖 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/timeline/runner.js` around lines 45 - 46, The current code computes renderingMode with a bare select call (select('core/editor')?.getRenderingMode?.() || getEditorMode()) which won't make the component re-render when the store value changes; replace that with a reactive useSelect hook from `@wordpress/data` to subscribe to core/editor.getRenderingMode (falling back to getEditorMode()) so renderingMode updates trigger re-renders and therefore correctly run the useEffect that compares prevRenderingMode and the requestAnimationFrame race-condition guard; update references to renderingMode and prevRenderingMode accordingly so the effect dependency uses the value returned by useSelect.src/editor/components/timeline/use-initial-style-tag.js (1)
7-9: 💤 Low valueThe
getEditorElwrapper is a no-op indirection — callgetEditorCanvasElement()directly.The alias
const getEditorEl = () => getEditorCanvasElement()adds no value; all call sites can usegetEditorCanvasElement()directly.♻️ Suggested cleanup
-const getEditorEl = () => getEditorCanvasElement()Then replace each
getEditorEl()call withgetEditorCanvasElement().🤖 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/timeline/use-initial-style-tag.js` around lines 7 - 9, Remove the no-op wrapper getEditorEl and all of its usages: delete the declaration const getEditorEl = () => getEditorCanvasElement() and replace every call to getEditorEl() with a direct call to getEditorCanvasElement() (update imports/usages accordingly where getEditorEl was referenced).src/editor/editors/gutenberg.js (1)
40-63: ⚡ Quick winUse the unified
wp.editor.PluginSidebarAPI introduced in WordPress 6.6.WordPress 6.6 (June 2024) unified the editor APIs across the post and site editors under
wp.editor. Thewp.editPost.PluginSidebarandwp.editSite.PluginSidebarapproaches you're currently using were moved there and now show deprecation warnings. A cleaner approach is to check the unified API first, then fall back to the old locations for compatibility with earlier versions:♻️ Proposed refactor
- const SideEditorPluginSidebar = window.wp.editSite?.PluginSidebar - const PostEditorPluginSidebar = window.wp.editPost?.PluginSidebar - const SideBar = SideEditorPluginSidebar ? SideEditorPluginSidebar - : PostEditorPluginSidebar ? PostEditorPluginSidebar : null + const SideBar = window.wp.editor?.PluginSidebar + || window.wp.editPost?.PluginSidebar + || window.wp.editSite?.PluginSidebar + || 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/gutenberg.js` around lines 40 - 63, The code currently looks up PluginSidebar via window.wp.editSite.PluginSidebar and window.wp.editPost.PluginSidebar; update GutenbergInteractionsEditorComponent to prefer the unified API by first checking window.wp.editor?.PluginSidebar and using that as SideBar, and only if that is undefined fall back to window.wp.editSite?.PluginSidebar and window.wp.editPost?.PluginSidebar for older WP versions; keep the rest of the component (selectedBlockAnchor, GutenbergSidebarWrapper, InteractionsApp) unchanged while ensuring the SideBar variable is assigned from window.wp.editor?.PluginSidebar when available.
🤖 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/app.js`:
- Around line 138-151: Guard the parse of
localStorage.getItem('interact-dismissed-errors') so malformed JSON cannot
throw: in the useEffect that reads into dismissedErrors (and the other site that
also parses this key) replace the direct JSON.parse call with a safe parse
(try/catch or a small helper like safeParseJson) that returns [] on error or
non-string input, then proceed to check dismissedErrors.includes(errorKey) and
call setIsShowingError accordingly; update both locations that reference the
'interact-dismissed-errors' parse to use the same safe fallback.
- Around line 245-257: The element-import path currently passes the imported
payload through unchanged while the page-import path rebuilds actions with
createNewAction, causing inconsistent action shapes; update the import handler
used by onOpenImportExportModal (the branch that calls onAddInteractionHandler)
to normalize incoming data the same way as page imports by mapping each
timeline.actions and replacing each action with a call to createNewAction (using
action.type, action.timing.start, action.target.type, and the original props) —
implement a helper normalizeImportedInteraction and call it before invoking
onAddInteractionHandler (also apply the same normalization to the other import
location around lines 317-333).
In `@src/editor/components/add-interaction-popover/index.js`:
- Around line 76-81: The state initializer calls getCurrentSelectedTarget()
multiple times and invokes getOrGenerateBlockAnchor(...) during every render;
change the useState call to use a lazy initializer function that runs only once
on mount, call getCurrentSelectedTarget() once into a local variable (e.g.,
selectedTarget) inside that function, and build the initial object using
selectedTarget, isElementor, getSelectedBlockClientId(),
getOrGenerateBlockAnchor(..., false) and getBlockNamesByClientId(...) as needed
so getOrGenerateBlockAnchor and the adapter call are only executed at
initialization and not on every render.
In `@src/editor/components/target-selector/index.js`:
- Around line 169-173: When Elementor mode removes certain options from
targetOptions (via isElementor), ensure the currently selected value.type is
normalized if it no longer exists: add a useEffect that runs when isElementor or
targetOptions change, checks if value.type (from props/state) is included in
targetOptions.map(t=>t.value), and if not calls the setter that updates
value.type to a safe default (e.g., the first allowed option such as 'trigger'
or ''), so the SelectControl and the block input rendering (the block-related
UI) stay in sync; reference targetOptions, isElementor, value.type,
SelectControl and the block input render logic when implementing.
In `@src/editor/editor.php`:
- Around line 19-25: The constructor currently registers the
elementor/editor/after_enqueue_scripts hook inside an is_admin() check which
prevents enqueue_elementor_editor() from being registered for frontend Elementor
editor requests; move the add_action('elementor/editor/after_enqueue_scripts',
array($this, 'enqueue_elementor_editor')) out of the is_admin() conditional so
the hook is always registered in __construct(), leaving the other add_action
calls (enqueue_block_editor_assets and enqueue_block_assets) inside the
is_admin() guard; ensure enqueue_elementor_editor() remains the same and relies
on Elementor's internal context to run only in the editor.
In `@src/editor/editors/elementor.js`:
- Around line 258-276: The Escape cancel path is unreliable because keyHandler
is only attached to document, so when focus is inside previewDocument the Escape
key isn't caught; update the setup to add the same keydown listener to
previewDocument (previewDocument.addEventListener('keydown', keyHandler, true))
and ensure stop() removes it as well
(previewDocument.removeEventListener('keydown', keyHandler, true)), keeping
existing cleanup for mouseMoveHandler, clickHandler and clearHighlight and still
calling onCancel when keyHandler detects Escape.
- Around line 105-107: getCanvasDocument currently returns document when the
preview iframe is missing, causing canvas consumers to operate on the admin
chrome; change getCanvasDocument() to return null if '#elementor-preview-iframe'
is missing or doesn't have contentDocument so callers can bail out (e.g.,
startElementPicker() already checks for null). Locate the getCanvasDocument
function and remove the fallback to document, ensuring it returns
iframe?.contentDocument ?? null, and update any callers if they assume a
non-null return to handle null cleanly.
---
Outside diff comments:
In `@src/editor/components/location-rules/index.js`:
- Around line 99-105: In the onClickAnd handler (the callback passed as
onClickAnd inside the locations.map loop), remove the dead guard
locations.length === 0 and simplify the value assignment to always be an empty
string; update the call site that invokes onChange(addLocation(locations, i, k +
1, { param: 'post', operator: '==', value: '' })) so getCurrentPostId() is not
referenced in this handler (leave other handlers such as the "Add rule group"
button untouched where locations.length === 0 is meaningful).
---
Nitpick comments:
In `@src/editor/components/timeline/runner.js`:
- Around line 45-46: The current code computes renderingMode with a bare select
call (select('core/editor')?.getRenderingMode?.() || getEditorMode()) which
won't make the component re-render when the store value changes; replace that
with a reactive useSelect hook from `@wordpress/data` to subscribe to
core/editor.getRenderingMode (falling back to getEditorMode()) so renderingMode
updates trigger re-renders and therefore correctly run the useEffect that
compares prevRenderingMode and the requestAnimationFrame race-condition guard;
update references to renderingMode and prevRenderingMode accordingly so the
effect dependency uses the value returned by useSelect.
In `@src/editor/components/timeline/use-initial-style-tag.js`:
- Around line 7-9: Remove the no-op wrapper getEditorEl and all of its usages:
delete the declaration const getEditorEl = () => getEditorCanvasElement() and
replace every call to getEditorEl() with a direct call to
getEditorCanvasElement() (update imports/usages accordingly where getEditorEl
was referenced).
In `@src/editor/editors/gutenberg.js`:
- Around line 40-63: The code currently looks up PluginSidebar via
window.wp.editSite.PluginSidebar and window.wp.editPost.PluginSidebar; update
GutenbergInteractionsEditorComponent to prefer the unified API by first checking
window.wp.editor?.PluginSidebar and using that as SideBar, and only if that is
undefined fall back to window.wp.editSite?.PluginSidebar and
window.wp.editPost?.PluginSidebar for older WP versions; keep the rest of the
component (selectedBlockAnchor, GutenbergSidebarWrapper, InteractionsApp)
unchanged while ensuring the SideBar variable is assigned from
window.wp.editor?.PluginSidebar when available.
🪄 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
Run ID: ff9d1130-dcbc-46a0-ae85-ab758c3a0993
📒 Files selected for processing (18)
src/editor/app.jssrc/editor/components/add-interaction-popover/index.jssrc/editor/components/import-export-modal/index.jssrc/editor/components/location-rules/index.jssrc/editor/components/target-selector/index.jssrc/editor/components/timeline/class-runner.jssrc/editor/components/timeline/runner.jssrc/editor/components/timeline/use-initial-style-tag.jssrc/editor/editor.jssrc/editor/editor.phpsrc/editor/editor.scsssrc/editor/editors/abstract.jssrc/editor/editors/elementor.jssrc/editor/editors/gutenberg.jssrc/editor/editors/index.jssrc/editor/hooks/use-interactions.jssrc/editor/plugins/index.jssrc/editor/util/index.js
Comment on lines
+169
to
+173
| if ( isElementor ) { | ||
| targetOptions = targetOptions.filter( target => | ||
| [ 'trigger', 'class', 'selector' ].includes( target.value ) | ||
| ) | ||
| } |
Contributor
There was a problem hiding this comment.
Saved value.type === 'block' breaks the SelectControl display in Elementor mode.
When Elementor is active, 'block' and 'block-name' are removed from targetOptions (lines 169–173), but there's no corresponding reset if the current value.type is already 'block' (e.g., an interaction saved in Gutenberg and then opened in Elementor). The SelectControl receives value="block" with no matching <option>, so the browser renders the first option visually while the underlying data stays 'block'. The block input section (line 233) still renders, causing a confusing UI.
A useEffect that normalises value.type when the Elementor filter doesn't include the current type would fix this:
🐛 Proposed fix
if ( isElementor ) {
targetOptions = targetOptions.filter( target =>
[ 'trigger', 'class', 'selector' ].includes( target.value )
)
}
+useEffect( () => {
+ if ( isElementor && ! [ 'trigger', 'class', 'selector' ].includes( value.type ) ) {
+ onChange( { ...value, type: 'selector', value: '' } )
+ }
+}, [ isElementor ] ) // eslint-disable-line react-hooks/exhaustive-deps📝 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
| if ( isElementor ) { | |
| targetOptions = targetOptions.filter( target => | |
| [ 'trigger', 'class', 'selector' ].includes( target.value ) | |
| ) | |
| } | |
| if ( isElementor ) { | |
| targetOptions = targetOptions.filter( target => | |
| [ 'trigger', 'class', 'selector' ].includes( target.value ) | |
| ) | |
| } | |
| useEffect( () => { | |
| if ( isElementor && ! [ 'trigger', 'class', 'selector' ].includes( value.type ) ) { | |
| onChange( { ...value, type: 'selector', value: '' } ) | |
| } | |
| }, [ isElementor ] ) // eslint-disable-line react-hooks/exhaustive-deps |
🤖 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/target-selector/index.js` around lines 169 - 173, When
Elementor mode removes certain options from targetOptions (via isElementor),
ensure the currently selected value.type is normalized if it no longer exists:
add a useEffect that runs when isElementor or targetOptions change, checks if
value.type (from props/state) is included in targetOptions.map(t=>t.value), and
if not calls the setter that updates value.type to a safe default (e.g., the
first allowed option such as 'trigger' or ''), so the SelectControl and the
block input rendering (the block-related UI) stay in sync; reference
targetOptions, isElementor, value.type, SelectControl and the block input render
logic when implementing.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/elementor.js`:
- Around line 179-189: registerSelectionTracking() currently defines const
register = action => { ... } and immediately follows with an array literal
which, without a semicolon, triggers an ASI hazard causing the array to be
parsed as a property access on the arrow function; to fix, terminate the
register declaration properly by adding a leading semicolon before the array
literal or by assigning the array to a const (e.g., const actions = [...];
actions.forEach(register)); ensure the change is applied around the register
variable and the forEach call so registerSelectionTracking(), register, and the
subsequent forEach invocation work as intended.
🪄 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
Run ID: da90847f-9cd1-4e23-a343-8775117e9a6a
📒 Files selected for processing (2)
src/editor/editors/elementor.jssrc/editor/editors/gutenberg.js
🚧 Files skipped from review as they are similar to previous changes (1)
- src/editor/editors/gutenberg.js
Comment on lines
+179
to
+189
| } | ||
|
|
||
| [ | ||
| 'panel/open_editor/section', | ||
| 'panel/open_editor/column', | ||
| 'panel/open_editor/container', | ||
| 'panel/open_editor/widget', | ||
| ].forEach( register ) | ||
|
|
||
| return NOOP | ||
| } |
Contributor
There was a problem hiding this comment.
Critical: ASI hazard breaks registerSelectionTracking() at runtime.
The arrow function assigned to register on line 171 ends with } on line 179, and the next non-blank token is [ on line 181. Without a semicolon between them, JavaScript does not insert ASI here — } followed by [ on the next line is parsed as member access on the preceding expression. So this code:
const register = action => { ... }
[ 'panel/open_editor/section', ..., 'panel/open_editor/widget' ].forEach( register )is parsed as:
const register = (action => { ... })[
'panel/open_editor/section', 'panel/open_editor/column',
'panel/open_editor/container', 'panel/open_editor/widget'
].forEach( register )The bracket contents collapse via the comma operator to 'panel/open_editor/widget', then (arrow)['panel/open_editor/widget'] is undefined, and .forEach(register) throws TypeError: Cannot read properties of undefined (reading 'forEach') the moment init() calls registerSelectionTracking(). Net effect: Elementor selection tracking never registers, this.selectedElement is never populated, and getCurrentSelectedTarget() always returns null (assuming the throw is caught upstream — otherwise the whole bootstrap crashes).
Add a leading semicolon on the array, or assign the array to a const first.
🛠️ Suggested fix
const register = action => {
window.elementor.hooks.addAction( action, ( panel, model, view ) => {
this.selectedElement = {
model,
view,
element: view?.$el?.get?.( 0 ) || null,
}
} )
}
- [
+ const actions = [
'panel/open_editor/section',
'panel/open_editor/column',
'panel/open_editor/container',
'panel/open_editor/widget',
- ].forEach( register )
+ ]
+ actions.forEach( register )🤖 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 179 - 189,
registerSelectionTracking() currently defines const register = action => { ... }
and immediately follows with an array literal which, without a semicolon,
triggers an ASI hazard causing the array to be parsed as a property access on
the arrow function; to fix, terminate the register declaration properly by
adding a leading semicolon before the array literal or by assigning the array to
a const (e.g., const actions = [...]; actions.forEach(register)); ensure the
change is applied around the register variable and the forEach call so
registerSelectionTracking(), register, and the subsequent forEach invocation
work as intended.
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n "register|forEach|actions|panel/open_editor" src/editor/editors/elementor.js -A 2 -B 2
Length of output: 2249
@Arukuen, it looks like the fix hasn't been pushed to the branch yet. The current code on feat/25-elementor-integration still shows the original pattern with the ASI hazard:
// Line 171 — arrow function closes here
const register = action => {
window.elementor.hooks.addAction( action, ( panel, model, view ) => {
this.selectedElement = { ... }
} )
}
// Line 181 — bare `[` immediately follows, triggering the ASI hazard
[
'panel/open_editor/section',
'panel/open_editor/column',
'panel/open_editor/container',
'panel/open_editor/widget',
].forEach( register )Please push the fix — for example:
const register = action => {
window.elementor.hooks.addAction( action, ( panel, model, view ) => {
this.selectedElement = {
model,
view,
element: view?.$el?.get?.( 0 ) || null,
}
} )
}
const actions = [
'panel/open_editor/section',
'panel/open_editor/column',
'panel/open_editor/container',
'panel/open_editor/widget',
]
actions.forEach( register )
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/editor/components/add-interaction-popover/index.js (1)
109-146:⚠️ Potential issue | 🔴 Critical | ⚡ Quick win
target === 'block'is alwaysfalse— block anchor (id) path is unreachableLine 121:
let pickerMode = target === 'block' ? 'id' : 'class'
targetis the state object{ type, value, blockName, options }, so comparing it to the string'block'is alwaysfalse.pickerModeis permanently'class', meaning blocks that support anchor attributes will never receive anid-based anchor — they will always get a generated class instead. The entireif ( pickerMode === 'id' )branch (lines 122–132) is dead code.The intended check is
target.type === 'block'.🐛 Proposed fix
- let pickerMode = target === 'block' ? 'id' : 'class' + let pickerMode = target.type === 'block' ? 'id' : '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/components/add-interaction-popover/index.js` around lines 109 - 146, The pickerMode check is comparing the whole target object to the string 'block', so the id-path is never taken; update the initialization of pickerMode in the BlockPickerPopover handler to use target.type (i.e. let pickerMode = target.type === 'block' ? 'id' : 'class') so the id-branch (which calls getOrGenerateBlockAnchor and checks block attributes via select('core/blocks').getBlockType) can run when the target represents a block, then keep the existing fallback to getOrGenerateBlockClass and setTarget/setHidden logic unchanged.src/editor/editor.php (1)
93-115:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse Elementor's documents API to resolve the current post in Elementor editor context.
When the
elementor/editor/after_enqueue_scriptshook fires in Elementor's frontend editor (e.g.,/?elementor=POST_ID), WordPress's global$postis not set reliably at script enqueue time. The safe defaults (0,'') prevent crashes but leave the JavaScript adapter withcurrentPostId: 0and empty post type, breaking any Elementor-specific logic that resolves targets or location rules by post context.For the
'elementor'mode branch, use Elementor's documents API to retrieve the current post reliably:Proposed fix
+ $post = get_post(); + // In Elementor editor context, get_post() is unreliable on frontend. + // Use Elementor's documents API to fetch the current document's post. + if ( $editor_mode === 'elementor' + && class_exists( '\Elementor\Plugin' ) + && \Elementor\Plugin::$instance->documents + ) { + $current_doc = \Elementor\Plugin::$instance->documents->get_current(); + if ( $current_doc ) { + $post = get_post( $current_doc->get_main_id() ); + } + } - $post = get_post();🤖 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/editor.php` around lines 93 - 115, When editorMode is 'elementor', the global $post may be unset; use Elementor's documents API to resolve the current document and populate currentPostId, currentPostType, currentPostTemplate and currentPostParent instead of relying only on get_post(). Specifically, when $editor_mode === 'elementor' call \Elementor\Plugin::instance()->documents->get( <elementor document id or current request param> ) or the appropriate documents->get_doc_for_frontend method to obtain the document object, extract the post ID, post type, template and parent from that document (falling back to the existing $post values if the document is not available), and assign those values into the array keys 'currentPostId', 'currentPostType', 'currentPostTemplate' and 'currentPostParent' before filtering with interact/localize_script.
🧹 Nitpick comments (1)
src/editor/components/add-interaction-popover/index.js (1)
68-74: ⚡ Quick winUnstable fallback functions in
useSelectcause spurious re-renders in Elementor modeWhen
blockEditorStoreresolves toundefined(Elementor), theuseSelectcallback returns freshly-created arrow functions() => []and() => nullon every invocation. TheuseSelectreturn value is always shallowly compared to the previous one, so new function instances on every call are seen as changed values, triggering unnecessary re-renders and a console warning about unstable return values.Extract the fallback no-ops as module-level constants so their references are stable:
♻️ Proposed fix
+const EMPTY_ARRAY = () => [] +const RETURN_NULL = () => null + // ... inside the component ... const { getBlockNamesByClientId, getSelectedBlockClientId, } = useSelect( select => { const blockEditorStore = select( 'core/block-editor' ) return { - getBlockNamesByClientId: blockEditorStore?.getBlockNamesByClientId || ( () => [] ), - getSelectedBlockClientId: blockEditorStore?.getSelectedBlockClientId || ( () => null ), + getBlockNamesByClientId: blockEditorStore?.getBlockNamesByClientId || EMPTY_ARRAY, + getSelectedBlockClientId: blockEditorStore?.getSelectedBlockClientId || 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/components/add-interaction-popover/index.js` around lines 68 - 74, The useSelect callback is returning new inline arrow fallbacks which are unstable and trigger re-renders; define module-level constants (e.g. NOOP_GET_BLOCK_NAMES = () => [] and NOOP_GET_SELECTED_BLOCK_ID = () => null) and use those constants as the fallback values inside the useSelect return object so getBlockNamesByClientId and getSelectedBlockClientId reference stable functions instead of creating new ones each render.
🤖 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/editor.php`:
- Around line 44-46: The code silently skips enqueuing wp-components when it's
not registered; update the block around
wp_style_is('wp-components','registered') / wp_enqueue_style('wp-components') to
emit a developer-safe warning when missing: if the style is not registered, and
WP_DEBUG is true, log a clear message (e.g., via error_log() or trigger_error())
indicating that 'wp-components' is not registered and the Elementor editor UI
may be unstyled, or optionally call wp_die() only in dev; keep the existing
wp_enqueue_style('wp-components') path unchanged when registered.
---
Outside diff comments:
In `@src/editor/components/add-interaction-popover/index.js`:
- Around line 109-146: The pickerMode check is comparing the whole target object
to the string 'block', so the id-path is never taken; update the initialization
of pickerMode in the BlockPickerPopover handler to use target.type (i.e. let
pickerMode = target.type === 'block' ? 'id' : 'class') so the id-branch (which
calls getOrGenerateBlockAnchor and checks block attributes via
select('core/blocks').getBlockType) can run when the target represents a block,
then keep the existing fallback to getOrGenerateBlockClass and
setTarget/setHidden logic unchanged.
In `@src/editor/editor.php`:
- Around line 93-115: When editorMode is 'elementor', the global $post may be
unset; use Elementor's documents API to resolve the current document and
populate currentPostId, currentPostType, currentPostTemplate and
currentPostParent instead of relying only on get_post(). Specifically, when
$editor_mode === 'elementor' call \Elementor\Plugin::instance()->documents->get(
<elementor document id or current request param> ) or the appropriate
documents->get_doc_for_frontend method to obtain the document object, extract
the post ID, post type, template and parent from that document (falling back to
the existing $post values if the document is not available), and assign those
values into the array keys 'currentPostId', 'currentPostType',
'currentPostTemplate' and 'currentPostParent' before filtering with
interact/localize_script.
---
Nitpick comments:
In `@src/editor/components/add-interaction-popover/index.js`:
- Around line 68-74: The useSelect callback is returning new inline arrow
fallbacks which are unstable and trigger re-renders; define module-level
constants (e.g. NOOP_GET_BLOCK_NAMES = () => [] and NOOP_GET_SELECTED_BLOCK_ID =
() => null) and use those constants as the fallback values inside the useSelect
return object so getBlockNamesByClientId and getSelectedBlockClientId reference
stable functions instead of creating new ones each render.
🪄 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
Run ID: f3e4a0b0-4315-4c81-bf8d-de1a8f643d86
📒 Files selected for processing (4)
src/editor/app.jssrc/editor/components/add-interaction-popover/index.jssrc/editor/editor.phpsrc/editor/editor.scss
🚧 Files skipped from review as they are similar to previous changes (2)
- src/editor/editor.scss
- src/editor/app.js
Comment on lines
+44
to
+46
| if ( wp_style_is( 'wp-components', 'registered' ) ) { | ||
| wp_enqueue_style( 'wp-components' ); | ||
| } |
Contributor
There was a problem hiding this comment.
Silent skip of wp-components may leave the Elementor editor UI unstyled.
The doc-comment on line 43 says this style is needed by the Elementor editor UI, yet if it isn't registered the skip is completely silent. At minimum a wp_die-safe debug log would surface this during development.
💡 Proposed improvement
if ( wp_style_is( 'wp-components', 'registered' ) ) {
wp_enqueue_style( 'wp-components' );
+ } else {
+ // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_error_log
+ error_log( 'Interact: wp-components style is not registered in the Elementor editor context; editor UI may appear unstyled.' );
}📝 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
| if ( wp_style_is( 'wp-components', 'registered' ) ) { | |
| wp_enqueue_style( 'wp-components' ); | |
| } | |
| if ( wp_style_is( 'wp-components', 'registered' ) ) { | |
| wp_enqueue_style( 'wp-components' ); | |
| } else { | |
| // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_error_log | |
| error_log( 'Interact: wp-components style is not registered in the Elementor editor context; editor UI may appear unstyled.' ); | |
| } |
🤖 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/editor.php` around lines 44 - 46, The code silently skips
enqueuing wp-components when it's not registered; update the block around
wp_style_is('wp-components','registered') / wp_enqueue_style('wp-components') to
emit a developer-safe warning when missing: if the style is not registered, and
WP_DEBUG is true, log a clear message (e.g., via error_log() or trigger_error())
indicating that 'wp-components' is not registered and the Elementor editor UI
may be unstyled, or optionally call wp_die() only in dev; keep the existing
wp_enqueue_style('wp-components') path unchanged when registered.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fixes #25
Summary by CodeRabbit
New Features
Improvements
Style