[WC-3464] Rich text Tiptapeditor

[gjulivan]

Pull request type

---

Description

[yordan-st]

Would have a look at the high and medium flagged CC review comments

[github-actions]

🔶 Medium — Hardcoded waitForTimeout delay

page.waitForTimeout(500) is a hardcoded sleep. Per e2e-test-guidelines, replace it with a web-first assertion that resolves as soon as the condition is met.

After pressing Tab to blur, the editor's empty-content normalisation is synchronous within Tiptap's state flush. A reliable wait is:

Suggested change

	await page.waitForTimeout(500);
	await expect(editor).toHaveAttribute("data-placeholder");

Or, if you want to assert the save has propagated to the Mendix attribute, wait for the text-content assertion itself (Playwright retries automatically):

        await page.keyboard.press("Tab");
        await expect(editor).toContainText("");

Either avoids the 500 ms wall-clock cost and the flakiness risk of a fixed sleep.

[github-actions]

🔶 Medium — Missing screenshot baselines for new class-mode tests

classModeEditor.png and classModeViewCodeDialog.png (line 170) are referenced by toHaveScreenshot but their baseline PNGs are not committed to the snapshots directory. Playwright will fail on first run because there is nothing to compare against.

Per the E2E guidelines, baseline images must be committed with the spec. Generate and commit them before merging:

cd packages/pluggableWidgets/rich-text-web
pnpm run e2e --update-snapshots
git add e2e/RichText.spec.js-snapshots/classModeEditor-chromium-linux.png \
         e2e/RichText.spec.js-snapshots/classModeViewCodeDialog-chromium-linux.png

[iobuhov]

Can we use different selector?

[gjulivan]

this selector is .tiptap-toolbar button, so, it won't affect any other buttons.

[iobuhov]

I just looked at Rich Text - looks fine to me, but I did half only. My only concern is drop down and some styling in toolbar.

[rahmanunver]

What are we doing with the states? I see store is gone. Is tiptap handling it itself?

[gjulivan]

What are we doing with the states? I see store is gone. Is tiptap handling it itself?

editorcontext is still there, just not on folder

[github-actions]

AI Code Review

🔶 Changes requested — one or more medium-severity items must be addressed

---

What was reviewed

File	Change
src/components/Editor.tsx	Core Tiptap editor component, extension wiring, action refs
src/components/EditorWrapper.tsx	Mendix EditableValue adapter, debounced write-back
src/components/EditorContext.tsx	Code-view state machine via useReducer
src/components/toolbars/components/ImageDialog.tsx	Image insert dialog with URL/upload/entity tabs
src/components/toolbars/components/VideoDialog.tsx	Video/embed insert dialog
src/components/toolbars/components/LinkDialog.tsx	Link insert/edit dialog
src/components/LinkBubbleMenu.tsx	Contextual link bubble menu
src/components/toolbars/Toolbar.tsx	Toolbar rendering and group filtering
src/components/toolbars/components/ToolbarButton.tsx	Per-command toolbar button
src/components/toolbars/components/ToolbarDefaultButton.tsx	Shared button primitive
src/components/YouTubeResize.tsx	YouTube node view with drag-resize
src/components/HighlightedCodeEditor.tsx	hljs-based code editor for HTML view
src/extensions/WordPaste.ts	ProseMirror plugin wiring for Word paste
src/extensions/GenericEmbed.ts	Sandboxed iframe embed node with domain allowlist
src/utils/wordPaste.ts	Pure Word-HTML sanitiser (marker flattening, indent levels)
src/utils/helpers.ts	CSS/URL injection guards
src/utils/tableStyle.ts	Validated CSS segment builders for table cells
src/utils/embedCodeParser.ts	Embed-code parser with domain allowlist
src/extensions/tableCellStyling.ts	Shared cell-styling attributes and renderCellHTML
src/components/DynamicTableStyles.tsx / DynamicTextColorStyles.tsx	Class-mode dynamic style appliers
src/utils/i18n/context.tsx	Translation provider + useT with English fallback
src/__tests__/RichText.spec.tsx	Widget-level snapshot suite
src/__tests__/wordPaste.spec.ts	Word sanitiser unit tests
e2e/RichText.spec.js	Playwright screenshot baseline tests
RichText.xml	Widget manifest
CHANGELOG.md	Unreleased entries for this change

Skipped (out of scope): dist/, pnpm-lock.yaml, openspec/ documents

---

Findings

🔶 Medium — useEffect depends on dialogRef.current (invalid dependency)

File: src/components/toolbars/components/ImageDialog.tsx line 188
Problem: dialogRef.current is a mutable ref value, not a reactive value — React never schedules re-runs because of it, so the // eslint-disable-next-line react-hooks/exhaustive-deps is hiding a real bug. If the div mounts after the effect fires (always possible in Strict Mode), the addEventListener call is skipped and the imageSelected custom event is never heard.
Fix: Drop dialogRef.current from the array; the ref stabilises after first render and the listener only needs to be registered once:

useEffect(() => {
    const imgRef = dialogRef.current;
    if (imgRef !== null) {
        imgRef.addEventListener("imageSelected", handleImageSelected);
    }
    return () => {
        imgRef?.removeEventListener("imageSelected", handleImageSelected);
    };
    // eslint-disable-next-line react-hooks/exhaustive-deps
}, []); // mount-once; dialogRef is stable, handler captured via closure is intentional

Remove the eslint-disable line once the dependency array is correct.

---

🔶 Medium — VideoDialog closes unconditionally even on validation error

File: src/components/toolbars/components/VideoDialog.tsx lines 152-166
Problem: handleSubmit calls onClose() after handleUrlSubmit/handleEmbedSubmit regardless of whether they set a validationError and returned early. The user sees the dialog close but the video is never inserted, with no error feedback visible.
Fix:

const handleSubmit = (e: FormEvent): void => {
    e.preventDefault();
    if (!editor) return;

    const w = parseInt(width, 10);
    const h = parseInt(height, 10);

    let submitted = false;
    if (activeTab === "url") {
        submitted = handleUrlSubmit(w, h);
    } else {
        submitted = handleEmbedSubmit(w, h);
    }

    if (submitted) {
        onClose();
    }
};

handleUrlSubmit and handleEmbedSubmit should return true on success and false on error (currently they return void).

---

🔶 Medium — External-value update guard in Editor.tsx is inverted

File: src/components/Editor.tsx lines 319-329
Problem: The comment says "only do update if editor not focused, otherwise it will override the user input" but the guard does the opposite: it returns early when the editor is not focused and only runs setContent when the editor is focused — directly clobbering the user's in-progress typing. External Mendix attribute changes therefore have no effect in view/read contexts, and do clobber typing in edit contexts.
Fix:

useEffect(() => {
    if (!editor || editor.isFocused) {
        // Don't clobber the user's in-progress typing.
        return;
    }
    const newContent = editor.getHTML();
    if (newContent !== defaultValue) {
        editor.commands.setContent(defaultValue || "");
    }
}, [editor, defaultValue]);

---

⚠️ Low — Snapshot test exercises a non-existent enum value

File: src/__tests__/RichText.spec.tsx line 96
Problem: "both" is not a valid StatusBarContentEnum value (the XML defines wordCount, characterCount, characterCountHtml). The test casts with as StatusBarContentEnum to silence TypeScript, meaning it hits the default branch of the switch and renders identically to wordCount. The test title is misleading and the cast masks a type error.
Fix: Remove the test or replace it with a valid enum value that is not already covered by other tests.

---

⚠️ Low — Dead commented-out code in DynamicTextColorStyles.tsx

File: src/components/DynamicTextColorStyles.tsx line 17
Problem: // if (!styleElementRef.current) return; references a styleElementRef that does not exist in the component. This is a leftover from a previous implementation and is confusing to future readers.
Fix: Delete the commented line.

---

⚠️ Low — XML property key OverflowY is not lowerCamelCase

File: src/RichText.xml line 127
Problem: All XML key attributes must be lowerCamelCase per AGENTS.md. OverflowY starts with an uppercase O. The TypeScript typings (RichTextContainerProps) will follow this casing and OverflowY as a prop name is non-standard.
Fix:

<property key="overflowY" type="enumeration" defaultValue="auto">

(and update the corresponding TypeScript references from OverflowY to overflowY). If this key was introduced in a previous release, note it for the next major-version migration to avoid a breaking Studio Pro config change.

---

Positives

• The suite of injection guards (isSafeCssColor, isSafeLinkUrl, isSafeCssBorderStyle, isSafeCssSize, isSafeCssFontFamily) is thorough and correctly applied at every point values are interpolated into style strings. CSS injection surface is well covered.
• isSafeLinkUrl strips ASCII control characters (U+0000–U+0020) before scheme detection — this correctly blocks java\tscript: bypass vectors that browsers resolve after stripping those chars.
• GenericEmbed.ts re-validates src on both parseHTML and renderHTML against the same domain allowlist, so stored-data tampering after paste cannot produce an un-sandboxed iframe.
• sanitizeWordHtml is a pure string → string function fully independent of an editor instance — the comprehensive fixture-driven test suite (wordPaste.spec.ts) directly tests the transform and is much easier to reason about than integration tests.
• The TranslationProvider / useT pattern provides a clean English fallback when no provider is mounted, preventing missing-translation crashes in isolated tests and the editor preview.
• The useReducer-based code-view state machine in EditorContext.tsx cleanly separates the enter / edit / save / cancel / confirm states rather than using scattered boolean flags.