Repository navigation
Notes: Mark the text an unsent note is about with a draft marker - #84125
Conversation
807a9df to
40bb7aa
Compare
🤖 PR meta 🤖🎉 PropsIf 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. Updated as activity occurs, without notifying anyone named here. Add the 📦 Bundle sizeSize Change: +101 B (0%) Total Size: 8.3 MB 📦 View Changed
⚡ PerformanceShow the resultsClient side metrics exclude the server response time. front-end-block-theme
front-end-classic-theme
media-processing
media-upload
post-editor
site-editor
🏁 Flaky testsSome 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. Navigates the items list via UP/DOWN arrow keys in
|
|
A small, side effect of this change is that now selection is highlighted when you're adding the inline note. I think this creates a better connection between the text and the note. |
487d5a7 to
c9ad5c3
Compare
|
It sounds like this mainly fixes a bug, nice one, thanks for iterating. Ship it after a code glance.
I like note highlights, in some of the mockups it's mostly opacity changes; semi-opaque when unselected, more opaque when selected. But I think since those mockups, to ensure contrast in every possible scenario, a colour-mixed bottom border may have been added as well in an unmerged PR, just something to remember (and an important one to land!) But what's also important to remember: the colour of the marking is meant to be the user avatar colour of the person who left it:
I.e. even in its resting/unselected state, it should ideally retain some colour so you can glance who left it. |
The placeholder marker uses the same highlighting system, and its color matches the current author. |
|
Ah perfect, all good then. It just looked grey in one of the images, I guess that's just the unfocused text selection inside the iframe, rather than the final |
c9ad5c3 to
4505a4a
Compare
|
This might be a bit of a stretch, but could we use That said, if this isn't the best approach, I'm completely fine with moving forward with this PR, as the benefits outweigh the downsides. |
|
Thanks, @t-hamano! I looked at an editor-only format for the draft marker, including The main problem is that rich text only feeds annotation formats through It would also add a new Orphans are limited. They only happen if the post is saved while a draft is open. The frontend unwraps every |
|
Overall this worked as expected testing in Playground. One thing I noticed that this PR makes more obvious I think is that while selections are highlighted when adding a note, but the block is not highlighted when adding a block level note. Which makes me wonder:
|
|
@Mamaduka thanks for picking this up, the draft-follows-edits behavior is a nice improvement. I ran a review pass and most of what turned up traces back to one choice: the Claude reviewed the diff and drafted the findings and options below:
Happy to help with whichever direction you pick. |
4505a4a to
2db707f
Compare
|
Thanks, @adamsilverstein! I'm going with A and keeping the marker in content. B is the editor-only format I looked at in #84125 (comment), and I tried a range snapshot like C first; without a marker, a stale anchor is invisible to the user.
|
2db707f to
aa627e7
Compare
Wrap the selection a new note starts from with a core/note marker whose id is 'new', so the anchor follows edits and survives a block switch until the note is sent. Submit swaps in the saved id; cancel, Escape and the empty-form dismissal strip it. The floating board anchors the pending note to the marker instead of the live selection. Also restore an unsent draft when its block is selected again while no note surface is mounted: the block sync now runs in NotesSidebar, and only the thread focus stays with the threads' DOM in Notes.
aa627e7 to
3f5847d
Compare
Realized this was a general RichText issue. A format shouldn't split on paste or edit. Fixed separately in #84303. |
| // Here rather than in `Notes`, which unmounts with its surface: a draft | ||
| // must be restored on block selection even while no note is shown. | ||
| useNoteSelection( { notes, drafts, onDiscard } ); |
There was a problem hiding this comment.
Last feedback: It seems that an empty floating notes area can appear when a block has resolved notes. For example:
- Create a new post with a paragraph.
- Add a note to the paragraph, then click "Resolve" on its floating thread.
- Click the post title.
- Click the paragraph.
- An empty "Unresolved notes" area appears. On trunk, nothing appears.
empty-floating-note-area.mp4
| // Here rather than in `Notes`, which unmounts with its surface: a draft | |
| // must be restored on block selection even while no note is shown. | |
| useNoteSelection( { notes, drafts, onDiscard } ); | |
| // Here rather than in `Notes`, which unmounts with its surface: a draft | |
| // must be restored on block selection even while no note is shown. Sync | |
| // against the threads the open surface lists: floating notes omit resolved. | |
| useNoteSelection( { | |
| notes: isAllNotesSidebarOpen ? notes : unresolvedNotes, | |
| drafts, | |
| onDiscard, | |
| } ); |
This change seems to fix it.
There was a problem hiding this comment.
I see. In that case, I think this PR is good to merge as is 👍
|
Nice work! |
Trunk's draft-marker work (#84125) and caret note selection (#84147) route note picking through usePickNote and anchor inline notes via the draft marker. Switch usePickNote to toggleBlockHighlight, feed the draft marker into the highlight CSS alongside the block-level highlights, and keep both sets of e2e tests. The post-save selection collapse keyed off inlineSelection, which the draft refactor removed; it is dropped here rather than ported, following review feedback that the lingering selection is acceptable. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AFjnTCdrevt1tgAviXfHs2

What?
Follow-up to #84110. See #73473.
When a note is started from a text selection, the selected text is wrapped with a
core/notemarker (idnew) until the note is sent; submit swaps in the saved ID, cancel strips it.Before, if the draft stayed open across a canvas click or block switch, the note submitted silently as a block-level note. Also, with no notes on the post, selecting a block with an unsent draft again did not reopen the draft until "Add note" was pressed.
Why?
onCreateread the inline anchor from the live block-editor selection at submit, which matched only the selection at form open, while any focus-out closed the form. Drafts broke that assumption.The draft restore lived in
useNoteSelectioninsideNotes, which only mounts while a note surface shows, so with zero notes nobody watched block transitions once the board unmounted.How?
The marker is a real format, so rich text tracks its range through edits and the existing
wrapInlineNote/removeInlineNote/updateNoteAnchorhelpers do all the work; no store state and no editor-only format.The front end already unwraps every
mark.wp-noteby class, so a marker saved mid-draft never leaks publicly; after a reload it stays as an inert tinted marker until the caret enters it (opens the form, Cancel strips it) or "Add note" runs on that block.Known cost: the post is dirty while an inline draft exists and stays dirty after Cancel, since
hasEditsForEntityRecordchecks that ablocksedit exists, not that it differs.Testing Instructions
Screenshots or screencast
CleanShot.2026-10-06.at.19.49.21.mp4
Use of AI Tools
Assisted by Claude.