Skip to content

Feat/wysiwyg dynamic values#71

Open
lloydsamson wants to merge 3 commits intomainfrom
feat/wysiwyg-dynamic-values
Open

Feat/wysiwyg dynamic values#71
lloydsamson wants to merge 3 commits intomainfrom
feat/wysiwyg-dynamic-values

Conversation

@lloydsamson
Copy link
Copy Markdown
Contributor

Add dynamic values support to WYSIWYG editor fields

Summary

Extends the dynamic values system to support WYSIWYG editor fields (both ProseMirror and TinyMCE). Previously,
dynamic values were limited to text, color-picker, date-picker, number, and conditional-panel field types. This
is particularly useful for email editing workflows where field content needs to include dynamic tokens like
[[post-title]] or [[author-name]].

Changes

Feature

  • Register editor and wysiwyg as allowed dynamic value types (insert mode only, replace mode doesn't apply to
    rich text)
  • ProseMirror: wrap editor with BaseWrapper; token is appended to the content value
  • TinyMCE: wrap editor with BaseWrapper; token is inserted at cursor position via
    tinyMCE.activeEditor.insertContent(), falling back to string append
  • Fix List.updateItem — state update was wrapped in setTimeout, causing the visibility toggle to not reflect
    immediately
  • Add live examples for both ProseMirror and TinyMCE editor dynamic value fields on the example page
  • Fix npm run test script to quote the path, so it works in environments with spaces (e.g. Local Sites)

Tests

  • Jest: use getByRole('button', { name: 'Insert' }) instead of getByText to avoid false matches against
    ProseMirror's own toolbar text
  • PHPUnit: add tests for dynamic token rendering inside HTML content and for wysiwyg store/strip behaviour
  • PHPUnit: fix enqueue test regex — WP 7.1-alpha appends a sourceURL comment to inline scripts, breaking the
    single-line match
  • E2E: fix Tangible Fields plugin basename (tangible-fields/plugin → fields/plugin) so the activation test
    correctly finds and activates the plugin in a fresh environment

…ditor fields

- Register 'editor' and 'wysiwyg' types in allowedTypes and defaultConfig (insert mode only)
- Wrap ProseMirror editor with BaseWrapper; append token via setValue
- Wrap TinyMCE with BaseWrapper; insert at cursor via tinyMCE.activeEditor.insertContent()
- Fix BaseWrapper hasClear logic to require props.remove !== undefined
- Fix List.updateItem: remove setTimeout so visibility toggle updates synchronously
- Add editor dynamic value examples (ProseMirror and TinyMCE) to example page
- Fix npm test script to quote path for environments with spaces (e.g. Local Sites)
…ssues

- Jest: use getByRole('button') for Insert/Clear to avoid matching ProseMirror toolbar text
- Jest: add editor/wysiwyg to insert-only types; fix Clear button expectations
- PHPUnit: add tests for dynamic token rendering inside HTML and wysiwyg store/strip logic
- PHPUnit: fix enqueue regex multiline flag for WP 7.1-alpha inline script sourceURL suffix
- E2E: fix Tangible Fields plugin basename from tangible-fields/plugin to fields/plugin
Depend on test_dynamic_value_render_in_html_content instead of
test_dynamic_value_category_registration so test-value-html is
registered before the store strips-tokens test runs.
Copy link
Copy Markdown
Contributor

@nicolas-jaussaud nicolas-jaussaud left a comment

Choose a reason for hiding this comment

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

Hi Lloyd!

Thanks for the PR and the tests

I left a few comment, mostly about the ProseMirror implementation as I think we have a small re-render issue. The TinyMCE part looks good to me!

? mapResults(results, props.mapResults)
: results

debounced.current.status = false
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.

Suggested change
debounced.current.status = false

Unfortunately I don't think we can add this line here, it prevent the debounced delay to be applied. Did you encounter some issues with it while testing?

If for a reason or another you have a specific case and want the result request to be sent right away you can pass a debounceTime prop and set it to 0

onChange={ setValue }
rawView={ props.rawView ?? true }
/>
<BaseWrapper
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.

We might want to move the BaseWrapper here into the child component (ProseMirror.tsx)

Currently the layout would look like this:

Image

By moving it into ProseMirror.tsx, it could look like this instead, which will keep all our action button in the same place:

Image

<BaseWrapper /> will still work if we use it without any children, so we can do something like this:

+import { BaseWrapper } from '../../../dynamic/'
 
 // ...
 
 const ProseMirror = forwardRef(({
   rawView = true,
+  dynamic = false,
   ...props
 }, ref) => {
 
   // ...
 
   return <div className="tf-editor-content">
-    { rawView && <div className="tf-editor-view-toggle">
-      <ButtonGroup
-        label={ 'Switch view' }
-        labelVisuallyHidden={ true }
-        value={ view }
-        onChange={ view => {
-          setView(view)
-          if ( view === 'raw' ) ref.current = null
-        }}
-        choices={{
-          visual : 'Visual',
-          raw    : 'Raw'
-        }}
-      />
+    { ( rawView || dynamic ) && 
+      <div className="tf-editor-view-toggle">
+        { dynamic &&
+          <BaseWrapper
+            config={ dynamic ?? false }
+            onValueSelection={ dynamic => {
+              setValue( value + dynamic )
+              setMountKey(k => k + 1)
+            } }
+            buttonType="outside"
+          /> }
+        { rawView &&
+          <ButtonGroup
+            label={ 'Switch view' }
+            labelVisuallyHidden={ true }
+            value={ view }
+            onChange={ view => {
+              setView(view)
+              if ( view === 'raw' ) ref.current = null
+            }}
+            choices={{
+              visual : 'Visual',
+              raw    : 'Raw'
+            }}
+          /> }

   // ...
})

export default ProseMirror

Which will also require to pass a dynamic props on <ProseMirror />:

 <ProseMirror
     ref={ editorRef }
     value={ value }
     onChange={ setValue }
     rawView={ props.rawView ?? true }
+    dynamic={ props.dynamic ?? false }
   />

props.onChange && props.onChange(value)
}, [value])

const insertDynamicValue = token => setValue(prev => (prev ?? '') + token)
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.

it looks like there is a small issue currently

If I insert a dynamic value, the state will update but it won't show up in the ProseMirror editor

It might be easier to fix if we move the wrapper inside the child component (as suggested in my other comment)

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.

2 participants