Repository navigation
Notes: show every thread participant on the block toolbar avatar indicator - #82357
adamsilverstein wants to merge 4 commits into
Conversation
A block can carry several note threads, but the toolbar indicator was wired to a single "primary" thread picked by pickPrimaryNote(). Since block-level notes sort ahead of inline ones, an unresolved block-level note always won and other users who left inline notes on the same block never got an avatar. Pass every thread on the block to NoteAvatarIndicator instead, and build the participant list across all of them. pickIndicatorNotes() keeps the previous resolved/unresolved preference (unresolved threads, falling back to all when none are unresolved), and getThreadParticipants() moves the author + replier dedupe out of the component so it can be tested directly. The existing "+n" overflow handling covers the longer participant lists this produces; the indicator's click target still opens the primary thread. Fixes #82273
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughThe block toolbar avatar indicator now aggregates relevant note threads and participants. Shared utilities select indicator threads and deduplicate chronological participants. Tests cover unresolved-thread fallback, nested replies, ordering, deduplication, and missing data. ChangesCollab sidebar indicator aggregation
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This PR updates the block toolbar indicator to show participants from all relevant note threads while preserving existing thread-opening and overflow behavior; no actionable merge-blocking risk remains beyond normal checks and review. Sequence Diagram(s)sequenceDiagram
participant NotesSidebar
participant CollabSidebarUtils
participant NoteAvatarIndicator
NotesSidebar->>NotesSidebar: Read note IDs from block metadata
NotesSidebar->>CollabSidebarUtils: Select indicator threads
CollabSidebarUtils-->>NotesSidebar: Return unresolved or all threads
NotesSidebar->>NoteAvatarIndicator: Pass selected notes
NoteAvatarIndicator->>CollabSidebarUtils: Extract thread participants
CollabSidebarUtils-->>NoteAvatarIndicator: Return ordered unique participants
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Linked Issues checkExplanation The implementation satisfies issue
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
🤖 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: -40 B (0%) Total Size: 8.25 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
|
…icator-all-threads # Conflicts: # packages/editor/src/components/collab-sidebar/index.jsx
|
Thanks, @adamsilverstein! I pushed a few simplifications:
|
…icator-all-threads
|
Closing, since this is no longer applicable after #84635. |
What?
Closes #82273
Makes the block toolbar avatar indicator represent all of the block's note threads instead of a single one, so every user with a note on the block shows up.
Why?
A block can carry several threads - a block-level note plus one or more inline notes - but the indicator was wired to a single "primary" thread picked by
pickPrimaryNote(). Block-level notes sort ahead of inline ones, so an unresolved block-level note always won and the other users who left inline notes on that block never got an avatar. Two inline threads by two different users had the same problem with no block-level note involved.Per the discussion on the issue, the button was designed to hold multiple avatars, and with multiple inline notes per block landing in 7.1 the intent is to show everyone participating in unresolved threads (block level, inline, and replies), with the existing 3+ overflow behavior.
How?
NotesSidebarnow passes the whole set of the block's threads toNoteAvatarIndicatorrather than one picked thread, and two helpers moved intoutils.js:pickIndicatorNotes()keeps the previous resolved/unresolved preference - unresolved threads, falling back to all of them when none are unresolved.getThreadParticipants()collects the distinct authors and repliers across those threads in chronological order. This is the same dedupe the component used to do inline, just testable on its own.The click target is unchanged: the indicator still opens the primary thread via
openNoteForBlock().Unit tests cover both helpers, including the multi-thread case from the issue.
Testing Instructions
Playground signs you in as a single user, so it won't show the two-user case below, but it is handy for checking a block-level note alongside an inline note and confirming replies still add avatars.
This needs two users to see the bug properly - eg. two browsers logged in as different users on the same post, with Notes enabled.
Before: only user A's avatar shows. After: both avatars show.
Also worth checking:
Unit tests:
npm run test:unit packages/editor/src/components/collab-sidebar/test/utils.jsdom.test.jsTesting Instructions for Keyboard
The toolbar button itself is unchanged - tab to the block toolbar and confirm the notes button is still reachable and activates with Enter or Space, and that its "View notes" label still reads correctly.
Screenshots or screencast
Use of AI Tools
The code, tests, and this description were written with 🤖 Claude Code. I will review and test.