Skip to content

Notes: Keep notes on the original block after a split - #84008

Closed
adamsilverstein wants to merge 2 commits into
trunkfrom
fix/notes-split-block-duplicate-note-id
Closed

Notes: Keep notes on the original block after a split#84008
adamsilverstein wants to merge 2 commits into
trunkfrom
fix/notes-split-block-duplicate-note-id

Conversation

@adamsilverstein

@adamsilverstein adamsilverstein commented Oct 1, 2026

Copy link
Copy Markdown
Member

Found this while testing #83786, stacked on that branch.

Claude tracked this one down and wrote up the fix:

What?

Keeps notes anchored to the original block after the block is split (or duplicated).

Why?

Splitting a paragraph mid-text copies its attributes, metadata.noteId included, into the new block. useNoteThreads mapped each note id to the last block carrying it, so the notes jumped to the new block, and each thread was listed once per block. The copied ids are saved with the post, so the wrong position survives a reload. Duplicating a block copies the ids the same way.

The same mapping is on trunk; the canvas margin in #83786 just makes it easy to spot.

How?

  • When a note id appears on more than one block, it belongs to the block holding its inline marker, otherwise to the first block in document order.
  • A thread is only listed under the block that owns it.
  • The block toolbar's note indicator uses the same mapping instead of the selected block's raw metadata.noteId.

Content already saved with copied ids displays correctly without any migration. The stale ids remain in the new block's markup but have no effect.

Testing Instructions

  1. Add a paragraph with a few sentences.
  2. Select a word and add an inline note, then add a block-level note to the same paragraph.
  3. Place the caret in the middle of the text and press Enter.
  4. Both notes should stay next to the first paragraph, each listed once. Selecting a note should select the first paragraph.
  5. Save and reload, the notes should stay put.

New e2e test: keeps notes on the original block after splitting it in block-notes.spec.js. It fails without the fix (each thread is listed twice).

AI Use

The fix, the test, and this description came out of a session with 🤖 Claude Code. I will review and test.

🤖 Generated with Claude Code

https://claude.ai/code/session_01JcpV7ZJ1j3fzQdAWHWJFyR

@adamsilverstein adamsilverstein added [Type] Bug An existing feature does not function as intended [Feature] Notes Phase 3 of the Gutenberg roadmap around block commenting labels Oct 1, 2026
@github-actions github-actions Bot added the [Package] Editor /packages/editor label Oct 1, 2026
@adamsilverstein adamsilverstein mentioned this pull request Oct 1, 2026
Merged
@github-actions

github-actions Bot commented Oct 1, 2026 •
edited
Loading

Copy link
Copy Markdown

🤖 PR meta 🤖

🎉 Props

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: adamsilverstein <adamsilverstein@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Updated as activity occurs, without notifying anyone named here. Add the props-bot label to refresh.

📦 Bundle size

Size Change: -6 B (0%)

Total Size: 8.25 MB

📦 View Changed
Filename Size Change
build/scripts/editor/index.min.js 617 kB -6 B (0%)

From 917afb3, not the current head. Run

⚡ Performance

Show the results

Client side metrics exclude the server response time.

front-end-block-theme

Metric 80c9697 add/notes-canvas-margin % Change
timeToFirstByte 58.4 ms +12.84% -2.48% 58.15 ms +11.69% -2.49% 0.43%
largestContentfulPaint 100 ms +6% -8% 100 ms +4% -8% 0%
lcpMinusTtfb 36.45 ms +15.09% -5.49% 36.65 ms +16.78% -4.64% -0.55%
wpBeforeTemplate 28.77 ms +18.25% -3.06% 29.05 ms +12.94% -2.31% -0.96%
wpTemplate 25.8 ms +3.33% -5.08% 25.44 ms +7.27% -4.17% 1.42%
wpTotal 54.62 ms +12.54% -2.36% 54.4 ms +11.53% -2.65% 0.4%
wpMemoryUsage 7.62 MB +0% -0% 7.61 MB +0% -0% 0.19%
wpDbQueries 17 +0% -0% 17 +0% -0% 0%

front-end-classic-theme

Metric 80c9697 add/notes-canvas-margin % Change
timeToFirstByte 48.45 ms +7.33% -1.24% 44.65 ms +8.96% -3.14% 8.51%
largestContentfulPaint 104 ms +3.85% -0% 96 ms +4.17% -4.17% 8.33%
lcpMinusTtfb 56 ms +2.59% -0.63% 51.2 ms +1.37% -3.91% 9.37%
wpBeforeTemplate 26.83 ms +9.5% -2.94% 27.42 ms +9.92% -2.63% -2.15%
wpTemplate 18.27 ms +3.17% -0.82% 13.96 ms +7.66% -2.29% 30.87%
wpTotal 45.28 ms +8.11% -1.5% 41.57 ms +9.29% -2.55% 8.92%
wpMemoryUsage 6.25 MB +0% -0% 6.09 MB +0% -0% 2.56%
wpDbQueries 14 +0% -0% 10 +0% -0% 40%

media-processing

Metric 80c9697 add/notes-canvas-margin % Change
mediaProcessingJpeg 398.87 ms +0.43% -0.91% 396.93 ms +0.69% -0.28% 0.49%
mediaProcessingAvif 6081.75 ms +0.28% -0.21% 6057.74 ms +0.11% -0.07% 0.4%
mediaProcessingJpegToAvif 4199.39 ms +0.09% -0.52% 4180.48 ms +0.11% -0.12% 0.45%

media-upload

Metric 80c9697 add/notes-canvas-margin % Change
jpegUploadProcessing 1445.88 ms +33.23% -1.25% 1418.94 ms +1.26% -0.74% 1.9%
pngUploadProcessing 175.02 ms +50.13% -2.94% 196.21 ms +16.66% -5.46% -10.8%
largeJpegUploadProcessing 1405.23 ms +0.34% -0.41% 1421.41 ms +1.32% -1.35% -1.14%
multipleImageUploadProcessing 1555.8 ms +1.09% -0.4% 1550.62 ms +1.2% -0.43% 0.33%

post-editor

Metric 80c9697 add/notes-canvas-margin % Change
serverResponse 514.41 ms +2.69% -2.6% 515.57 ms +4.3% -5.23% -0.22%
firstPaint 248.55 ms +7.88% -19.79% 249.77 ms +3.58% -10.4% -0.49%
domContentLoaded 1097.85 ms +1.28% -3.43% 1085.48 ms +0.96% -0.5% 1.14%
loaded 1099.15 ms +1.26% -3.45% 1086.8 ms +0.95% -0.52% 1.14%
firstContentfulPaint 452.68 ms +1.94% -2.61% 457.19 ms +0.55% -3.04% -0.99%
firstBlock 3275.78 ms +0.42% -0.66% 3220.51 ms +1.54% -0.58% 1.72%
type 16.99 ms +3.41% -5.77% 17.87 ms +1.23% -5.93% -4.92%
typeWithoutInspector 16.78 ms +2.03% -3.4% 16.23 ms +7.58% -3.7% 3.39%
typeWithTopToolbar 22 ms +2.55% -3.32% 19.92 ms +9.84% -3.26% 10.44%
typeContainer 8.35 ms +4.43% -6.71% 7.75 ms +10.71% -6.58% 7.74%
focus 70.4 ms +9.91% -3.47% 66.68 ms +10.03% -5.61% 5.58%
firstFocus 194.61 ms +0% -0% 197.88 ms +0% -0% -1.65%
selectAll 545.21 ms +0.08% -7.98% 500.05 ms +3.97% -1.18% 9.03%
listViewOpen 62.35 ms +9.24% -11.71% 60.02 ms +6.96% -7.68% 3.88%
inserterOpen 23.53 ms +7.05% -6.54% 21.81 ms +8.39% -5.04% 7.89%
inserterHover 2.12 ms +21.23% -4.72% 1.99 ms +11.06% -12.06% 6.53%
inserterSearch 7.7 ms +3.77% -7.4% 7.71 ms +10.64% -8.3% -0.13%
loadPatterns 635.93 ms +3.43% -7.4% 633.05 ms +2.29% -3.14% 0.45%
wpTotal 503.4 ms +2.9% -2.42% 505.23 ms +4.48% -5.25% -0.36%
wpMemoryUsage 13.17 MB +0% -0% 13.15 MB +0% -0% 0.16%
wpDbQueries 54 +0% -1.85% 54 +0% -1.85% 0%

site-editor

Metric 80c9697 add/notes-canvas-margin % Change
serverResponse 337.7 ms +4.45% -3.16% 326.75 ms +5.78% -5.98% 3.35%
firstPaint 201.15 ms +26.69% -16.65% 184.81 ms +41.65% -11.62% 8.84%
domContentLoaded 881.02 ms +1.09% -2.2% 878.29 ms +2.6% -1.16% 0.31%
loaded 881.88 ms +1.08% -2.2% 879.17 ms +2.6% -1.16% 0.31%
firstContentfulPaint 365.86 ms +1.4% -2.16% 357.35 ms +2.42% -0.95% 2.38%
firstBlock 3134.84 ms +0.83% -0.94% 3114.02 ms +0.38% -1.36% 0.67%
type 15.26 ms +1.9% -2.03% 15.32 ms +5.29% -4.11% -0.39%
navigate 108.11 ms +5.71% -15.58% 106.65 ms +4.99% -6.54% 1.37%
loadPatterns 1076.78 ms +9.89% -2.99% 1050.95 ms +4.27% -2.08% 2.46%
loadPages 1102.64 ms +1.6% -1.41% 1094.36 ms +2.21% -0.18% 0.76%
wpTotal 329.28 ms +4.57% -3.3% 318.59 ms +5.85% -6.19% 3.36%
wpMemoryUsage 12.14 MB +0% -0% 12.13 MB +0% -0% 0.11%
wpDbQueries 44 +0% -2.27% 44 +0% -2.27% 0%

From 917afb3, not the current head. Run

adamsilverstein and others added 2 commits October 1, 2026 16:08
@adamsilverstein @claude
Notes: Add failing e2e test for notes after splitting a block
2ad9dd2 
Splitting a paragraph mid-text copies its metadata.noteId into the new
block, so the notes list each thread twice and anchor them to the new
block instead of the original.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JcpV7ZJ1j3fzQdAWHWJFyR
@adamsilverstein @claude
Notes: Anchor a duplicated note id to a single block
cbe2c21 
Splitting or duplicating a block copies metadata.noteId, so one note id
can sit on several blocks. The last block won, which moved notes onto
the new block and listed each thread once per block. Resolve each id to
the block holding its inline marker, otherwise the first block in
document order, and list the thread only there.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JcpV7ZJ1j3fzQdAWHWJFyR
@adamsilverstein
adamsilverstein force-pushed the fix/notes-split-block-duplicate-note-id branch from 917afb3 to cbe2c21 Compare October 1, 2026 23:16
@adamsilverstein
adamsilverstein changed the base branch from add/notes-canvas-margin to trunk October 1, 2026 23:16
@adamsilverstein adamsilverstein mentioned this pull request Oct 1, 2026
Open
@adamsilverstein

adamsilverstein commented Oct 1, 2026

Copy link
Copy Markdown
Member Author

Folded into #80009 so there's one PR to review.

@Mamaduka
Mamaduka deleted the fix/notes-split-block-duplicate-note-id branch October 2, 2026 05:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@desrosj desrosj Awaiting requested review from desrosj
@retrofox retrofox Awaiting requested review from retrofox
@oandregal oandregal Awaiting requested review from oandregal
@gigitux gigitux Awaiting requested review from gigitux
@ntsekouras ntsekouras Awaiting requested review from ntsekouras
@manzoorwanijk manzoorwanijk Awaiting requested review from manzoorwanijk
@ellatrix ellatrix Awaiting requested review from ellatrix
@fabiankaegy fabiankaegy Awaiting requested review from fabiankaegy
@Mamaduka Mamaduka Awaiting requested review from Mamaduka
@juanmaguitar juanmaguitar Awaiting requested review from juanmaguitar

Assignees

No one assigned

Labels

[Feature] Notes Phase 3 of the Gutenberg roadmap around block commenting [Package] Editor /packages/editor [Type] Bug An existing feature does not function as intended

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@adamsilverstein