Notes: Support inline rich text (bold, italic, link, code)#78242
Notes: Support inline rich text (bold, italic, link, code)#78242adamsilverstein wants to merge 16 commits into
Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Size Change: +5 B (0%) Total Size: 7.96 MB 📦 View Changed
ℹ️ View Unchanged
|
|
Sorry, don't have time to do a proper review now. A technical note: we shouldn't use |
Hmmm, that sounds like a bug. Checking.
Ok, thanks for the tip @Mamaduka and reference. I guess that means this ticket is blocked by #75275, unless you think there is a different approach we should use in the meantime? |
9eabe70 to
36b8be3
Compare
Per review feedback on #78242, replace the block-editor RichText component with RichTextControl introduced in #75275. RichText is intended for use inside blocks and pulls in editor state (selection store, format toolbar wiring, etc.) that the note form does not need. RichTextControl wraps useRichText directly and exposes the smaller surface the note form actually uses, which mirrors the approach DataForms takes for its rich-text field. The form keeps the same allowed-formats list (bold, italic, link, code), the same keyboard shortcuts (Cmd+B / Cmd+I / Cmd+K), and the same empty-state submit guard. The Cmd+Enter / Escape handling moves up to the form element because RichTextControl does not expose an onKeyDown prop; the events still reach it via bubbling from the contenteditable. Note styles are rescoped to the form-level container (the input now carries the .block-editor-rich-text-control class) and the explicit VisuallyHidden label is replaced by RichTextControl's built-in hideLabelFromVision option. Refs #75275 Refs #73413
Per review feedback on #78242, replace the block-editor RichText component with RichTextControl introduced in #75275. RichText is intended for use inside blocks and pulls in editor state (selection store, format toolbar wiring, etc.) that the note form does not need. RichTextControl wraps useRichText directly and exposes the smaller surface the note form actually uses, which mirrors the approach DataForms takes for its rich-text field. The form keeps the same allowed-formats list (bold, italic, link, code), the same keyboard shortcuts (Cmd+B / Cmd+I / Cmd+K), and the same empty-state submit guard. The Cmd+Enter / Escape handling moves up to the form element because RichTextControl does not expose an onKeyDown prop; the events still reach it via bubbling from the contenteditable. Note styles are rescoped to the form-level container (the input now carries the .block-editor-rich-text-control class) and the explicit VisuallyHidden label is replaced by RichTextControl's built-in hideLabelFromVision option. Refs #75275 Refs #73413
Thanks for testing.
Works well for me and seems like a natural improvement! I have some code questions so want to get additional feedback on the approach, but realized they are mostly for the upstream component in #75275 which this PR depends on. |
@Mamaduka I updated the PR to be based on and depend on the RichText component from #75275 |
|
Opened a core backport for the PHP server-side piece: WordPress/wordpress-develop#11843 |
|
Flaky tests detected in 53c00b6. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/26045902178
|
Replace the plain textarea in the note form with the block editor's RichText component, restricted to bold, italic, link, and code formats via allowedFormats. Users get ⌘B / ⌘I / ⌘K keyboard shortcuts and no toolbar, matching the lightweight inline input direction agreed on in the issue thread. The empty-state check now strips HTML before measuring length so an empty contenteditable (which may contain stray markup) still disables the submit button. Refs #73413
Standard comment sanitization strips all HTML through wp_filter_kses for users without unfiltered_html. With the note form now sending bold, italic, link, and code markup, notes need a narrower allowlist that preserves those tags without opening up the broader comment surface. Install a scoped filter on pre_comment_content for REST POST/PUT/PATCH requests to /wp/v2/comments where the target is a note (either type=note in the body or an existing note being updated). The filter runs wp_kses with a strong/em/a/code allowlist and forces rel="noopener nofollow" on links. Regular comments still go through the default kses pipeline. Refs #73413
Cover the contract that the issue calls out: the format allowlist is limited to bold/italic/link/code, the submit button is gated by visible text (not raw HTML), ⌘+Enter / Ctrl+Enter submits, Escape cancels, and both the submit and cancel handlers run when expected. While here, lift the submit call out of form.requestSubmit() into a plain function. The keyboard shortcut now calls it directly instead of asking the form element to re-dispatch a submit event, which is more predictable across wrapper components and easier to test. Refs #73413
Cover the server contract introduced for note rich text: bold, italic, link, and code tags survive a REST POST round-trip; client-supplied `rel` on anchors is replaced with rel="noopener nofollow"; script, img, and event-handler attributes are stripped from notes; and regular (non-note) comments continue to use the standard comment kses pipeline so the relaxation does not leak across comment types. Refs #73413
Core's wp_rel_ugc filter runs on pre_comment_content after the note kses filter and appends 'ugc' to <a rel>, producing rel="noopener nofollow ugc". The forced safety tokens are still present, so assert the tokens via regex instead of an exact string match. Also fix the non-note-leak test: the strict comment kses allowlist permits <strong>, so the prior 'Bold body' substring check was wrong (the closing </strong> sits between them). Assert what the test actually cares about — content survives and the note filter's forced 'noopener' is absent.
Playwright's locator.fill() on a contenteditable does not reliably fire the input events that the block-editor RichText component listens to, so the form's React state stays empty, the submit button stays aria-disabled, and locator.click() on it times out. Switch the note-form interactions to pressSequentially() so the keystrokes travel through the same beforeinput path the component uses in production. For edit flows that need to replace existing content, click the textbox, select all with ControlOrMeta+a, and type the new value.
…ount the form The note form uses RichText from @wordpress/block-editor, which dispatches selectionChange to the block-editor store when it gains focus. Outside a block edit context this clears the canvas block selection. AddNote was gating its render on getSelectedBlockClientId(), so focusing the form's input made selectedBlockClientId go null mid-edit and unmounted the form before the user could submit. Split AddNote into an outer component that gates only on selectedNote and an inner component that snapshots the initial clientId via useState. Thread that snapshot through to onCreate via a new blockClientId argument so the new note attaches to the originating block even if Redux selection shifted while the form was open.
When RichText is the form input, React re-renders during typing can drop focus to document.body for one tick, producing a blur event with no relatedTarget. The handler treated that as 'focus left the form' and called selectNote(undefined), unmounting the form mid-edit. Skip blurs without a concrete relatedTarget; document.hasFocus() still guards the window-loses-focus case.
Per review feedback on #78242, replace the block-editor RichText component with RichTextControl introduced in #75275. RichText is intended for use inside blocks and pulls in editor state (selection store, format toolbar wiring, etc.) that the note form does not need. RichTextControl wraps useRichText directly and exposes the smaller surface the note form actually uses, which mirrors the approach DataForms takes for its rich-text field. The form keeps the same allowed-formats list (bold, italic, link, code), the same keyboard shortcuts (Cmd+B / Cmd+I / Cmd+K), and the same empty-state submit guard. The Cmd+Enter / Escape handling moves up to the form element because RichTextControl does not expose an onKeyDown prop; the events still reach it via bubbling from the contenteditable. Note styles are rescoped to the form-level container (the input now carries the .block-editor-rich-text-control class) and the explicit VisuallyHidden label is replaced by RichTextControl's built-in hideLabelFromVision option. Refs #75275 Refs #73413
Mock @wordpress/block-editor's privateApis and the editor lock-unlock module so the test substitutes a lightweight RichTextControl for the real one. The mock keeps the same data-testid the suite already uses so the existing assertions about format allowlist and keyboard handling continue to work unchanged. Refs #75275
Pressing Cmd+K inside a note (with text selected) dismissed the note form before the inline link UI could render: the format popover's portal target lives outside the form container, so AddNote's onBlur treated the focus move as leaving the form and tore it down. Skip AddNote's blur dismissal when the new focus target is inside a `.components-popover`, so the form stays mounted while the user interacts with format-spawned popovers. The companion RichTextControl-side deselection deferral now lives upstream on the base branch, so this commit only carries the Notes-specific guard and its e2e coverage. Verified live: typing "visit example" in a note, selecting it, and pressing Cmd+K opens the link search input while the note form stays mounted and the command palette stays closed.
Replace the preg_replace_callback rewrite of <a rel="..."> with WP_HTML_Tag_Processor. The HTML API handles attribute quoting and edge cases (multiple/duplicated rels, attribute-name boundaries) without the regex risk, and matches the core backport in WordPress/wordpress-develop#11843.
- Revert the format-library/link change: restoring the `isVisible &&` guard on `InlineLinkUI` here. Making the link popover appear inside RichTextControl is the upstream component's responsibility and belongs in #75275. - AddNote: hoist the `!clientId` early return up to the parent so AddNoteInner only mounts once we have a clientId snapshot. This removes the two `@wordpress/no-unused-vars-before-return` suppressions around `useDispatch` calls and a now-unreachable inner guard. - hooks.js: clarify in-place that the `blockClientId` snapshot parameter is a workaround for selection drift caused by RichTextControl focus, with a pointer to #75275 so the param can be removed once that lands.
The earlier eslint-disable cleanup hoisted the `!clientId` guard from AddNoteInner up to the AddNote parent. That made the parent re-check *live* block selection every render, so when focusing the note form cleared the canvas selection the form unmounted mid-edit — regressing the trunk-passing 'should move focus to add a new note form' e2e test. Restore the snapshot + inner-guard structure; the two eslint-disables are ugly but load-bearing. Mark 'Cmd+K opens the inline link popover' as test.fixme: it asserts the link popover renders inside a toolbar-less RichTextControl, which depends on the format-library link-gate relaxation that is intentionally being moved to the upstream RichTextControl PR (#75275) rather than shipped here.
The note form now uses RichTextControl, which does not reliably take/hold focus in a standalone, toolbar-less context. This regresses several trunk-passing e2e tests (focus-to-form, reply, reopen-and-reply, keyboard nav, multiple notes, and collaboration reply-sync) — all failing on toBeFocused()/reply-button timeouts with the same root cause. The fix belongs in the upstream RichTextControl PR (#75275), not here. Mark the affected tests test.fixme with explicit comments so the regression is visible to reviewers and the tests are re-enabled when #75275 lands. This is a known, documented scope gap, not a silent skip.


What?
Adds a minimal, safe rich text input to Notes, replacing the plain textarea with the new
RichTextControlfrom #75275 — restricted to bold, italic, link, andcodeformatting. Keyboard shortcuts (⌘B / ⌘I / ⌘K) work; no visible toolbar. Surrounding text with backticks (`) adds code formatting.Closes #73413.
Stacked on #75275
This PR is stacked on #75275 (WIP — adds
RichTextControlto the block-editor private API and uses it for a DataForms rich-text field).To keep CI runnable and the diff testable today, the first four commits of this branch are cherry-picks of #75275 rebased onto current trunk:
Initial field and usage of field for titleAdd stylesAdd labelWIP - type wranglingWhen #75275 (or an equivalent landing of
RichTextControl) merges, this PR will be rebased and those four commits will be absorbed, leaving only the Notes changes.Why
RichTextControland not block-editorRichText?Per #78242 (comment) from @Mamaduka,
RichTextfrom@wordpress/block-editoris intended for use inside blocks: it ties into the block-editor selection store, format-toolbar slot fill, block-attribute persistence, and other editor-only machinery the note form doesn't need. Using it here pulled that infrastructure into a tiny inline input, and exposed friction around focus/selection lifecycle (see the workaround commits later in the branch).RichTextControl(introduced in #75275) wrapsuseRichTextdirectly and exposes the smaller surface a standalone rich-text field actually needs. This is the same approach DataForms takes for its rich-text field, and what the related discussion in #73180 calls for. Switching to it removes the editor-store coupling, keeps the implementation aligned with the rest of the fields/DataForms direction, and gives notes a lightweight inline input without dragging the full RichText surface in.Why?
Per discussion in #73413 with @jasmussen and @Mamaduka, Notes needed a lightweight inline rich-text input — useful for emphasizing labels, linking to references, and quoting identifiers — but without taking up sidebar space with a toolbar. This PR ships the smallest viable formatting set (bold/italic/link/code) so reviewers can express feedback more clearly without bloating the UI.
How?
Client —
packages/editor/src/components/collab-sidebar/note-form.jsRichTextControl(unlocked from@wordpress/block-editorprivate APIs) withallowedFormats = ['core/bold','core/italic','core/link','core/code']andhideLabelFromVisionfor the accessible label.RichTextControldoesn't take anonKeyDownprop, but format-edit keyboard shortcuts (⌘B/⌘I/⌘K) are wired up inside the control.Server — new
lib/compat/wordpress-7.1/notes-rich-text.phppre_comment_contentfor REST POST/PUT/PATCH to/wp/v2/commentsonly when the target is a note (eithertype=notein the body or the existing comment is a note).wp_kseswith a minimal allowlist:strong,em,a,code. Forcesrel="noopener nofollow"on links regardless of client input.rest_post_dispatchso non-note comments and non-REST paths are unaffected.Output — already renders HTML via
RawHTML(note.js:162-164) and the excerpt path already callsstripHTML, so no display-side changes were needed.Testing instructions
npm run wp-env start && npm start.<script>alert(1)</script>or<img src=x onerror=alert(2)>— confirm it is stripped server-side, not rendered.Automated tests
npm run test:unit -- packages/editor/src/components/collab-sidebar/test/note-form.jscomposer test -- --filter WP_Test_REST_Comments_Controller_GutenbergOut of scope (deferred per issue discussion)
**bold**, backtick toggle, etc.) — inline input rules don't exist in the rich-text package today; keyboard shortcuts cover the primary UX.Screenshots
Types of changes
New feature.
Checklist