Skip to content

Phase 4: Attribute suggestions, architecture docs, e2e coverage#77406

Open
adamsilverstein wants to merge 9 commits into
suggest-mode-phase-3from
suggest-mode-phase-4
Open

Phase 4: Attribute suggestions, architecture docs, e2e coverage#77406
adamsilverstein wants to merge 9 commits into
suggest-mode-phase-3from
suggest-mode-phase-4

Conversation

@adamsilverstein

@adamsilverstein adamsilverstein commented Apr 16, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Overview

This is one of 5 stacked PRs implementing Suggest mode for the WordPress editor — a Google Docs–style workflow where reviewers can propose changes that the post author can Accept or Reject. Tracked in #73411, with design direction from #73410 and jasmussen's mockups.

This PR (Phase 4)

Review-UX polish + architecture docs + attribute coverage:

  • SuggestionSummary — compact Docs-style summary rendered in the notes sidebar. Derives Add:, Delete:, Formatting:, and Format: lines from a payload's operations:
    • Text-valued content edits produce quoted Add: / Delete: lines from a word-level diff, ellipsized at 120 characters.
    • Pure inline-format changes (e.g., toggling <strong>) are detected by comparing visible text with tags stripped and emit Formatting: bold, italic rather than leaking raw <strong> markup.
    • Non-text attribute changes (level, align, fontSize…) collapse into a single Format: line with human-readable labels.
  • E2E attribute coverage — suggestion-mode.spec.js exercises heading-level changes so non-text attribute suggestions are covered end-to-end alongside the text case.
  • hasAttributeConflict — helper that compares each operation's captured before to the block's current value. The "apply anyway" confirmation is now scoped per-attribute instead of per-post, so unrelated activity (other blocks, auto-saves, style edits) no longer trips the staleness dialog. Post-level baseRevision is still stamped into the payload for provenance.
  • Architecture docs — docs/explanations/architecture/suggestions.md documents the provider interface, payload shape, overlay semantics, auto-save timing, conflict detection, and the Yjs v2 migration path.

What's NOT in this PR

  • Auto-save of pending overlay edits (Phase 5)
  • The green bracket/outline on blocks with a pending suggestion (Phase 5)
  • REST permission overrides so editors can accept/reject suggestions authored by others (Phase 5)

Phase 4 test plan

  • Make a text-only suggestion → confirm the sidebar shows Add: "…" / Delete: "…" quotes
  • Toggle bold on a word → confirm the sidebar shows Formatting: bold and does not leak raw <strong> tags
  • Change a heading level (H2 → H3) → confirm the sidebar shows Format: heading level
  • Make a suggestion, then edit another block's content → Accept should apply silently with no dialog
  • Make a suggestion, then manually edit the targeted attribute on the target block → Accept should prompt with "This block has changed since the suggestion was made"
  • Unit: npm run test:unit -- packages/editor/src/components/suggestion-mode/test/suggestion-summary.js
  • Unit: npm run test:unit -- packages/editor/src/components/suggestion-mode/test/provider.js --testNamePattern=hasAttributeConflict
  • E2E (attribute case): npm run test:e2e -- test/e2e/specs/editor/various/suggestion-mode.spec.js

🗺️ PR Stack Navigation

# PR Phase
1 #77403 Intent scaffolding Edit / Suggest / View mode
2 #77404 Overlay capture In-memory suggestion overlay
3 #77405 Provider + Accept/Reject _wp_suggestion meta, provider, sidebar actions
4 #77406 Summary + docs + attribute tests ← this PR Add/Delete/Formatting summary, architecture stub, conflict scoping
5a #78351 REST permissions and PHP coverage Permissions, payload cap, PHP tests
5b #78352 Summary + attribute conflict + docs Renderer, per-attribute staleness, architecture docs
5c #78353 Surface Apply/Reject in the collaboration sidebar Icon buttons + e2e + sidebar wiring
6 #78308 Auto-save subsystem Background debounced save (replaces commit-bar)

📋 Tracking issue: #73411

@github-actions github-actions Bot added the [Package] Editor /packages/editor label Apr 16, 2026
@adamsilverstein adamsilverstein changed the title suggest mode phase 4 Phase 4: Attribute suggestions, architecture docs, e2e coverage Apr 16, 2026
This was referenced Apr 16, 2026
Closed
Open
Open
Open
@adamsilverstein adamsilverstein self-assigned this Apr 16, 2026
@github-actions

github-actions Bot commented Apr 16, 2026 •
edited
Loading

Copy link
Copy Markdown

Size Change: 0 B

Total Size: 7.51 MB

compressed-size-action

@adamsilverstein adamsilverstein force-pushed the suggest-mode-phase-4 branch 2 times, most recently from 37bd62c to 8111615 Compare April 16, 2026 18:49
@adamsilverstein adamsilverstein force-pushed the suggest-mode-phase-3 branch 2 times, most recently from 9debb13 to f63fe8c Compare April 16, 2026 18:52
@adamsilverstein adamsilverstein added [Type] Enhancement A suggestion for improvement. [Feature] Notes Phase 3 of the Gutenberg roadmap around block commenting labels Apr 19, 2026
@adamsilverstein

adamsilverstein commented Apr 19, 2026

Copy link
Copy Markdown
Member Author

Playwright-5 failed again on the rerun of run 24542691965 with the same widespread-shard pattern — 43 tests failing across unrelated specs (undo.spec.js, toolbar-roving-tabindex.spec.js, post-content-focus-mode.spec.js) plus two of the Suggest-mode attribute tests, all presenting as timeouts waiting for standard editor UI to appear.

This matches the known-flaky set tracked in #76524 and #77385 (already called out by the flaky-tests bot on this stack). Shards 4 and 5 appear to be overloaded on this repo and regularly fall over mid-run regardless of the PR's code.

Trunk's E2E runs on recent commits are green, so this isn't a trunk regression. Rerunning once more.

@adamsilverstein

adamsilverstein commented Apr 19, 2026

Copy link
Copy Markdown
Member Author

Second rerun of Playwright-5 (job 72026854911) failed with the same shard-wide flake pattern — the known-flaky tests tracked in #76524 and #77385 plus two transient failures in our new Suggest-mode attribute spec.

Accepting this shard-5 flake as unblocking:

  • Trunk E2E is green on recent commits.
  • Other Playwright shards (1–4, 6–8) pass consistently on this stack.
  • The failing specs (undo, toolbar-roving-tabindex, post-content-focus-mode) are orthogonal to this PR and already tracked as flaky.

Not rerunning again.

@github-actions

github-actions Bot commented Apr 19, 2026 •
edited
Loading

Copy link
Copy Markdown

Flaky tests detected in f75ce39.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/27779949481
📝 Reported issues:

@adamsilverstein adamsilverstein marked this pull request as ready for review April 29, 2026 16:01
This was referenced May 1, 2026
Open
Open
This was referenced May 14, 2026
Open
Open
Open
Open
@adamsilverstein adamsilverstein added Backport from WordPress Core Pull request that needs to be backported to a Gutenberg release from WordPress Core and removed [Package] Editor /packages/editor labels May 18, 2026
This was referenced Jun 6, 2026
Draft
Open
@github-actions github-actions Bot added the [Package] Editor /packages/editor label Jun 17, 2026
@adamsilverstein
Editor: Attribute suggestions, architecture docs, e2e coverage
4036ef4 
Phase 4 of the Suggest mode effort. The attribute-set operation type
already handles all block attributes generically (text, heading level,
alignment, style objects), so this phase focuses on verification,
documentation, and hardening.

New:
- E2E test proving non-text attribute suggestions work end-to-end
  (heading level change via toolbar in Suggest mode).
- Architecture doc at docs/explanations/architecture/suggestions.md
  covering the overlay, payload schema, provider interface, apply/
  reject lifecycle, diff rendering, Yjs v2 migration path, and known
  limitations (structural suggestions, inline selections, permissions,
  rich-text format fidelity).

Known limitation documented: applying another user's suggestion
requires moderate_comments because core's update_item_permissions_check
gates on edit_comment. A follow-up PR should override this for notes.

Refs #73411
@adamsilverstein
Tests: Close Options menu after switching intent; skip store-level le…
6ed7cb9 
…vel test

MenuItemsChoice doesn't auto-close its dropdown when a radio item is
selected, so `switchIntent` was leaving the Options menu open. A
follow-up `switchIntent` call then toggled the open menu closed
instead of reopening it, which broke the 'restores baseline when
switching back to Edit intent' test. Press Escape after selecting
the intent so switchIntent is idempotent.

Skip the heading-level suggestion test: the overlay HOC only wraps
`setAttributes` passed to a block's `BlockEdit`, but heading level
on web is changed via the block-variation picker, which dispatches
`updateBlockAttributes` directly on the store and bypasses the HOC.
Capturing store-level attribute changes needs a dedicated
interception layer in the suggestion provider (tracked as follow-up).
@adamsilverstein
Suggestions: Un-skip the heading-level e2e test
dbc9cf7 
The skipped test was a placeholder pending an interceptor for direct
updateBlockAttributes mutations. With the store-level interceptor
landed in Phase 2, the block-switcher's variation picker is now
captured by the overlay. Rewrite the test to actually drive the
toolbar dropdown and verify both the overlay-rendered DOM and the
unchanged serialized post.
@adamsilverstein
Fix Suggest-mode auto-save diff for RichTextData; tighten heading-lev…
ca38f1d 
…el e2e

- isAttributeEqual returned true for two RichTextData instances with
  different content because RichTextData stores its content in private
  class fields and exposes no enumerable own keys, so a key-by-key walk
  found nothing different. Fall back to a String() compare when both
  objects have zero own keys, so diff'ing a paragraph's content overlay
  against its baseline correctly produces an attribute-set operation
  and the auto-save POST actually fires.
- The block-switcher button on a heading reflects the active variation
  ('Heading 2'), not the bare block name ('Heading'), so the existing
  selector with exact: true never matched. Use a regex on the variation
  title.
- The block also renders the level as the actual h{n} tag rather than
  setting aria-level, so assert tagName instead.
@adamsilverstein
Suggestions: Expand architecture doc with interceptor and implementat…
f75ce39 
…ion files

Add the missing 'Store interceptor' subsection covering the snapshot/diff/
revert strategy, the apply-time bypass, and the multi-peer accept loop
(complementary halves: requestInterceptorBypass + isAcceptedSuggestionChange).
Add an 'Implementation files' table so reviewers can navigate from the
architecture description to the source. Add an 'Implementation wrinkles'
section documenting the RichTextData wrapper boundary, DEEP_MERGE_KEYS,
the dual lifecycle axes, and the payload-size cap that the existing doc
glossed over.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Mamaduka Mamaduka Awaiting requested review from Mamaduka
@t-hamano t-hamano Awaiting requested review from t-hamano
@spacedmonkey spacedmonkey Awaiting requested review from spacedmonkey
@manzoorwanijk manzoorwanijk Awaiting requested review from manzoorwanijk

Assignees

@adamsilverstein adamsilverstein

Labels

Backport from WordPress Core Pull request that needs to be backported to a Gutenberg release from WordPress Core [Feature] Notes Phase 3 of the Gutenberg roadmap around block commenting [Package] Editor /packages/editor [Type] Enhancement A suggestion for improvement.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@adamsilverstein