improve visual configurator for better elementor compatibility (WP-1018) - #633
Conversation
…onfigurator (WP-1018) Rules can be limited to an Elementor widgetType and to text properties of enclosing objects, and use recursive $..key paths so custom widgets nested at any depth can be ingested (e.g. sovos accordions, skipping stale placeholder data). Extended rules are matched in PHP by JsonLeafMatcher because JSONPath filters with recursive descent are unreliable in the bundled library. The rule editor shows a live preview of matches. Rules can be exported to and imported from JSON; import only adds rules and never changes existing ones. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…s (WP-1018) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
| */ | ||
| public function isExtended(): bool | ||
| { | ||
| return $this->widgetType !== '' || $this->conditions !== []; |
There was a problem hiding this comment.
A rule counts as "extended" only when it has a widget or conditions. The UI's "Anywhere" mode (visual-configurator.js#L261) always produces a $..a.b path, with or without them.
So $..items.title with no conditions goes to the JSONPath library, which most likely won't step through the items[n] array. Add one condition and the same rule goes to JsonLeafMatcher, which ignores array indexes (JsonLeafMatcher.php#L86) and does match. Adding a filter can therefore increase the matches.
Repeater fields are the main use case for this. Suggest an explicit matchMode (position / anywhere) stored on the rule instead of inferring it. That also means the EXTENDED_PATH_PATTERN check covers every anywhere rule. Please add a test for "anywhere" with no widget or conditions against a repeater.
There was a problem hiding this comment.
Added matchMode
| if ($rule->isExtended()) { | ||
| $data = &$jsonObject->getValue(); | ||
| $changed = false; | ||
| foreach ($this->matcher->match($data, $rule) as $leaf) { |
There was a problem hiding this comment.
On download, the matcher runs on $data, which comes from $translation['meta'][$metaKey] (L153) and has usually been translated by the Elementor handler already. On upload it ran on the source.
A condition on a translatable sibling (e.g. title = "Hello") matches on upload, fails on download, and the translation is silently dropped. The UI makes this easy, because it offers every string property as a condition, including the leaf's own value (visual-configurator.js#L255).
Suggest matching against $original['meta'][$metaKey] to collect segments, then calling setValue on the translated copy.
| if ($rule->isExtended()) { | ||
| $data = &$jsonObject->getValue(); | ||
| $changed = false; | ||
| foreach ($this->matcher->match($data, $rule) as $leaf) { |
There was a problem hiding this comment.
The same source-vs-translated problem, for related rules. Conditions and widget scope should be evaluated on the original JSON. Separately, IDs the Elementor handler has already remapped get passed through processAttributeOnDownload a second time. In multisite, a target ID can collide with an unrelated source ID.
| */ | ||
| private function buildRule(array $data): JsonFieldRule | ||
| { | ||
| $rule = JsonFieldRule::fromArray($data); |
There was a problem hiding this comment.
Import goes straight to fromArray(), which skips the checks readRule() does: sanitize_text_field, rejecting empty metaKey/propertyPath/replacerId, and the 512-character limit on propertyPath. So an imported file can store rules the UI would reject. Suggest moving those checks into the JsonFieldRule constructor so every path shares them.
- store explicit matchMode so "anywhere" rules always use JsonLeafMatcher and match repeater items, with or without widget/conditions - match extended rules against the source JSON and write into the translated copy, so conditions on translatable siblings keep working on download - replace related ids from the source id to avoid remapping twice - validate rule fields in JsonFieldRule and share sanitizing between UI save and import Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…ted copy (WP-1018) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
|
||
| public function getPluginId(): string | ||
| { | ||
| return 'elementor4'; |
There was a problem hiding this comment.
This is more than a log label: ExternalContentManager (L39) stores the uploaded fields under getPluginId(), and ExternalContentElementorAbstract::setContentFields (L212) reads $translation[$this->getPluginId()].
Elementor 4 content uploaded before the upgrade has its strings under elementor. After the upgrade, download looks under elementor4, finds nothing, and the Elementor strings stay untranslated with no error. The Elementor 4 handler has been on master since WP-1000, so jobs can be in progress across the upgrade.
Suggest keeping elementor as the data key and changing only the log name, or falling back to $translation['elementor'] when elementor4 is missing.
There was a problem hiding this comment.
Reworked to use dedicated log method
| const [replacerId, setReplacerId] = useState('copy'); | ||
| const [refType, setRefType] = useState('attachment'); | ||
| if (!draft) return null; | ||
| const [mode, setMode] = useState('position'); |
There was a problem hiding this comment.
RuleEditor stays mounted (it returns null when there is no draft), so mode, keyCount, selected and limitToWidget carry over to the next "Add rule".
For example, save an "anywhere" rule, then add a rule on a leaf where canGeneralize is false. The Match select is hidden but mode is still anywhere, so the path becomes $.. (or a wrong key suffix) and saving fails with a confusing error. A checked condition id such as 0|kind can also stay applied to a different leaf without the user seeing it.
Suggest resetting this state when draft changes, or rendering el(RuleEditor, { key: draft path, ... }).
| for (let distance = 0; distance < ancestors.length; distance++) { | ||
| const object = ancestors[ancestors.length - 1 - distance]; | ||
| Object.entries(object).forEach(([key, val]) => { | ||
| if (typeof val === 'string' && key !== 'elType' && key !== 'widgetType') { |
There was a problem hiding this comment.
Distance-0 options include the leaf's own key and value. For title.text the list offers text = "Hello" (same object), and selecting it limits the rule to that one source string, which is almost never what the user wants.
Suggest excluding the leaf's own key at distance 0 (the last element of keys).
| $this->wpProxy->wp_send_json_error(['message' => $e->getMessage()], 400); | ||
| return; | ||
| } | ||
| $value = $this->wpProxy->getPostMeta($id, $rule->getMetaKey(), true); |
There was a problem hiding this comment.
Preview reads any metaKey from any post id the caller sends. The capability check limits who can call it, and only JSON-decodable values come back, but a user with the Smartling capability could still read JSON meta from posts they cannot edit.
Suggest checking current_user_can('edit_post', $id) or restricting to meta keys the configurator lists.
There was a problem hiding this comment.
Added check
- keep elementor as the data key for Elementor 3 and 4 and add getLogName() to Pluggable so logs can tell the handlers apart (elementor3, elementor4) - require edit_post capability for the configurator preview - remount the rule editor for every opened draft so state does not leak - do not offer the leaf's own value as a condition Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
PavelLoparev
left a comment
There was a problem hiding this comment.
We need internal demo of this change.
No description provided.