[WC-3556] fix(rich-text): dialog presentation, image paste/drop, list marker formatting

[gjulivan]

Pull request type

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Test related change (New E2E test, test automation, etc.)

---

Description

Groups five related Rich Text fixes plus one File Uploader fix. All changes are
non-breaking; stored content is untouched until the user makes an edit.

Rich Text — dialogs

• New "Dialog style" setting. inline (default, unchanged behaviour) anchors a
  dialog to the toolbar button it was opened from; focused centers it over a dimmed
  page and traps the keyboard inside until it closes. Adds the dialogStyle XML
  property and a shared DialogShell component that the image, video, link, help and
  confirm dialogs now render through.
• Dialogs are no longer clipped. A dialog taller than the viewport, or one inside a
  container that clips its content (e.g. a pop-up page), used to be cut off with its
  Cancel/Insert buttons unreachable. Dialogs now render in a body-level portal, stay
  inside the viewport, and scroll internally — the action buttons sit outside the scroll
  region so they stay visible.
• Media Library buttons no longer insert prematurely. Clicking a button inside the
  image dialog's Media Library content inserted the image and closed the dialog without
  Insert being pressed.

Rich Text — images

• Drag & drop and clipboard paste for images. New ImagePasteDrop extension. Files
  that are too large or are not images are rejected with a reason shown below the editor.
  Gated on "Enable default upload", whose description was updated to cover the new paths.
• Dropping a file no longer navigates the browser away from the page.
• v4 image sizes survive the v5 upgrade. Images resized in Rich Text 4 lost their
  size and rendered at their original dimensions.

Rich Text — list markers

• Bullets and numbers now follow the formatting of the list item's first character —
  size, bold, italic, colour and font family. Each item is evaluated independently, and
  nested lists follow their own first run.

  Implementation notes for reviewers: the marker format is derived, never stored. ::marker
  inherits from its <li>, but every format the user can apply lands on an inline mark two
  levels down, and CSS has no child-to-ancestor selector — so computeMarkerFormat reads
  the first inline run and publishes the result as --rt-marker-* custom properties, which
  affect only what ::marker reads and not the item's own content. Two delivery paths are
  needed and both call the same pure function: renderHTML (feeds getHTML(), copy/paste,
  initial render) and a Decoration.node plugin (ProseMirror does not re-invoke toDOM when
  only a node's content changes, so the attribute would otherwise go stale as you type).
  No node attribute is declared, so incoming marker data is dropped on parse and recomputed.

  The list gutter scales with the marker, since an enlarged marker grows leftward out of
  padding-left. The multiplier is marker-length-aware: measured in Chrome, a flat 1.5×
  clipped three- and four-digit numbers at the maximum font size, so the gutter is derived
  from the longest counter's character count (start + childCount - 1, with lower-roman
  counted by numeral length rather than digits). With no enlarged marker present the
  computed padding is byte-identical to the previous 1.5em.

  Task lists are out of scope — taskItem renders a checkbox with list-style: none and
  has no ::marker.

• Opening a page no longer marks the value as changed. Pre-existing and not
  list-specific: the editor's value-sync effect called setContent with updates enabled,
  so on every mount the editor's own serialization was written back over any stored value
  that was not already byte-identical to getHTML() — dirtying the bound attribute and
  firing the "On change" action without a user edit. Now passes { emitUpdate: false };
  that direction is external value → editor, so echoing back is never wanted. A genuine
  edit still emits through onUpdate.

  Side effect worth a look: the status bar's "Characters (HTML)" count now reflects the
  value as stored rather than the editor's re-serialization (one snapshot moved, 82 → 49).
  Arguably the more truthful number, but it will still shift on the user's first real edit.

Rich Text — toolbar

• Header toolbar item now appears in custom mode. Its name was mapped incorrectly.

File Uploader

• Action and retry buttons no longer submit the surrounding form, which caused the
  page to submit or a containing dialog to close unexpectedly.

---

What should be covered while testing?

Rich Text

• Both styleDataFormat modes (inline and class) for every item below — class
  mode emits class + data-* attributes instead of inline styles.
• Dialog style: switch between Inline and Focused for the image, video and link
  dialogs. Check keyboard focus stays inside a Focused dialog and Escape dismisses it.
• Dialog clipping: short viewport, and a Rich Text inside a pop-up page. Cancel and
  Insert must stay reachable.
• Image dialog: Media Library tab — clicking buttons inside the content should not
  insert or close; only Insert should.
• Images: drag & drop and paste, with "Enable default upload" both on and off.
  Oversized and non-image files should be rejected with a message. Open content saved in
  Rich Text 4 with resized images and confirm sizes are preserved.
• List markers: enlarge the first character of a bullet and a numbered item and check
  the marker follows size, bold, italic, colour and font. Sibling items should format
  independently, nested lists should follow their own first run, task list checkboxes
  should be untouched, and a long numbered list (100+ items) at a large size should not
  clip its numbers.
• On change: open a page with existing Rich Text content and close it without editing.
  The "On change" action must not fire and the stored value must be byte-identical.

File Uploader

• Place the widget inside a form and inside a pop-up page. Clicking a file action button
  or the retry button must not submit the form or close the dialog.

Browser note: class-mode marker formatting relies on typed attr()
(attr(data-marker-font-size px)), which is Chrome 133+ and not yet in Safari or Firefox.
This is the same support bar the widget's existing class-mode font size and text colour
already sit on, so it is not a new limitation — but class mode is worth a look in Safari
if that matters for this release. All five ::marker properties are verified in Chrome.

Tests

• Unit: markerFormat (29), ListItemMarkerFormat (14), ImagePasteDrop, ImageResize,
  ActionButton, RetryButton, ToolbarConfig, plus the load-time no-write regression in
  RichText.spec.tsx. pnpm run test in rich-text-web: 403 passing, 26 suites.
• E2E: dialog clipping (tall viewport + pop-up page), YouTube embed URL, and one
  parameterized marker case per list type in e2e/RichText.spec.js.
• CHANGELOG.md updated under [Unreleased] in both packages. No version bumps — those
  happen at release time.

[github-actions]

AI Code Review

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

---

What was reviewed

File	Change
packages/pluggableWidgets/file-uploader-web/src/components/ActionButton.tsx	type="button" added; redundant role="button" removed
packages/pluggableWidgets/file-uploader-web/src/components/RetryButton.tsx	type="button" added
packages/pluggableWidgets/file-uploader-web/src/components/__tests__/ActionButton.spec.tsx	New unit tests for form-submission guard
packages/pluggableWidgets/file-uploader-web/src/components/__tests__/RetryButton.spec.tsx	New unit tests for form-submission guard
packages/pluggableWidgets/file-uploader-web/CHANGELOG.md	Unreleased entry added
packages/pluggableWidgets/rich-text-web/src/components/Editor.tsx	setContent now uses emitUpdate: false; ImagePasteDrop wired up
packages/pluggableWidgets/rich-text-web/src/components/toolbars/components/ImageDialog.tsx	form removed; handleInsert replaces handleSubmit; Enter per-input; stale-closure fix
packages/pluggableWidgets/rich-text-web/src/components/toolbars/components/DialogShell.tsx	New shared dialog shell — portal, inline/focused modes, viewport bounds
packages/pluggableWidgets/rich-text-web/src/components/toolbars/components/Dialog.tsx	Routing to ImageDialog/VideoDialog/LinkDialog
packages/pluggableWidgets/rich-text-web/src/extensions/ImagePasteDrop.ts	New extension — file drop/paste guard, browser-navigate prevention
packages/pluggableWidgets/rich-text-web/src/extensions/ImageResize.ts	v4 bare-number size compat
packages/pluggableWidgets/rich-text-web/src/extensions/ListItemMarkerFormat.ts	New extension — ::marker follows first-run format
packages/pluggableWidgets/rich-text-web/src/utils/imageSize.ts	New utility — toCssLength / toHtmlDimension
packages/pluggableWidgets/rich-text-web/src/utils/markerFormat.ts	New utility — CSS custom-property and class-mode attrs for ::marker
packages/pluggableWidgets/rich-text-web/src/components/toolbars/ToolbarConfig.ts	header mapped to textFormat (fix for custom mode)
packages/pluggableWidgets/rich-text-web/src/RichText.xml	dialogStyle enumeration property added
packages/pluggableWidgets/rich-text-web/typings/RichTextProps.d.ts	DialogStyleEnum + dialogStyle prop
packages/pluggableWidgets/rich-text-web/e2e/RichText.spec.js	New: dialog clipping, popup clip, list-marker, clearEditor helper; mode default
packages/pluggableWidgets/rich-text-web/e2e/RichTextWordPaste.spec.js	mode serial to default
packages/pluggableWidgets/rich-text-web/CHANGELOG.md	Unreleased entries added for all seven fixes

Skipped (out of scope): dist/, pnpm-lock.yaml, openspec/ archives, *.png E2E snapshots

---

Findings

🔶 Medium — useEffect for initial files has unstable handleFileDrop in its dependency array

File: packages/pluggableWidgets/rich-text-web/src/components/toolbars/components/ImageDialog.tsx line 215

Problem: handleFileDrop is a plain const defined inside the component, not wrapped in useCallback. Every render creates a new function reference. Listing it in the useEffect deps causes the effect to re-run on every render, including renders triggered by the setActiveTab and setSrc calls that handleFileDrop itself makes. The initial-file-drop logic will execute more than once per dialog open.

Fix: Remove handleFileDrop from the dependency array. It closes only over stable state setters and props already listed, so the omission is safe:

useEffect(() => {
    if (!initialFiles?.length) return;
    if (hasImageSource) {
        pendingEntityUploadFilesRef.current = initialFiles;
        setActiveTab("entity");
        return;
    }
    setActiveTab(showUploadTab ? "upload" : "url");
    void handleFileDrop(initialFiles);
    // eslint-disable-next-line react-hooks/exhaustive-deps
}, [initialFiles, hasImageSource, showUploadTab]);

---

🔶 Medium — DialogShell focused mode sets role="dialog" twice

File: packages/pluggableWidgets/rich-text-web/src/components/toolbars/components/DialogShell.tsx lines 150–155

Problem: The focused-mode <div> receives role="dialog" from both getFloatingProps() (injected by useRole(context, { role: "dialog" })) and an explicit role="dialog" JSX prop. The duplicate will silently mask any future change to useRole's output.

Fix: Remove the explicit role="dialog" prop — getFloatingProps() already injects it:

<div
    ref={focusedDialogRef}
    {...getFloatingProps()}
    className={dialogClassName}
    style={{ maxHeight: DEFAULT_MAX_HEIGHT }}
    aria-modal="true"
    aria-labelledby={ariaLabelledBy}
    tabIndex={-1}
>

---

⚠️ Low — Dead ternary for referenceElement in Dialog.tsx

File: packages/pluggableWidgets/rich-text-web/src/components/toolbars/components/Dialog.tsx line 18

Note: config.command === "insertImage" ? buttonRef.current : buttonRef.current — both branches are identical. Collapse to const referenceElement = buttonRef.current;.

---

⚠️ Low — Unhandled promise from insertImageFiles in drop/paste handlers

File: packages/pluggableWidgets/rich-text-web/src/extensions/ImagePasteDrop.ts lines 219, 250

Note: insertImageFiles(...) returns Promise<void> but the result is silently dropped in both the drop and paste handlers. The inner try/catch inside that function makes a rejection very unlikely, but prefixing with void documents the intentional discard and prevents strict linters from flagging it:

void insertImageFiles(files, dropPosition(view, event), context());

---

Positives

• emitUpdate: false fix (Editor.tsx line 410) is architecturally correct — the value-sync effect runs external→editor only, so echoing back dirties the attribute on load. The debounce-flushing regression test in RichText.spec.tsx (lines 106–132) makes this durable.
• Structural immunity in ImageDialog — removing the <form> entirely rather than filtering SubmitEvent.submitter is the correct long-term call. The design doc's reasoning is thorough and the "insertion isolation" unit tests encode all three failure modes.
• handleDOMEvents over handleDrop/handlePaste in ImagePasteDrop.ts — running ahead of ProseMirror's editable gate is the only correct way to prevent browser navigation on a read-only editor; the file's leading comment explains this clearly.
• No document mutations in ListItemMarkerFormat — deriving marker format purely as a decoration avoids the dirty-attribute loop the PR also fixes in Editor.tsx. The dual-path reasoning (decoration + renderHTML) is sound and well-documented inline.
• computeMaxMarkerSize / computeMarkerLength correctly scope gutter math to direct list children (.forEach) so nested lists size their own gutter independently.
• Test coverage is exemplary: the insertion-isolation suite covers all three failure modes; ImagePasteDrop tests enabled/disabled/read-only states; markerFormat covers all five CSS properties, both style modes, and roman/alpha length edge cases.
• E2E clearEditor helper with toPass + Ctrl+A → Backspace reliably clears ProseMirror content where selectText() previously left table content behind.
• Both CHANGELOGs are correctly formatted (Keep a Changelog) and user-facing.