Notes: overlay floating notes on space reserved inside the canvas - #79864
Notes: overlay floating notes on space reserved inside the canvas#79864adamsilverstein wants to merge 26 commits into
Conversation
Encodes the target behavior from #73917 and the design discussion in #66377/#77484: notices and the canvas span the full editor width while floating notes are visible, notes never overlap full-width content, and the floating panel coexists with the Settings sidebar. Four of the six tests fail against trunk by design; the implementation follows.
Retire the transparent PluginSidebar that hosted floating notes. It made the editor treat the notes as a layout-occupying sidebar column, which truncated notices, pushed the canvas scrollbar to the middle of the viewport, and prevented the notes from coexisting with the Settings sidebar (see #73917). Floating notes now render into a FloatingNotesSlot inside the visual editor, overlaying space reserved inside the canvas document itself (padding on its html element, with overflow-x clipped on the body so full-bleed content cannot render under the notes). Because the reserved space lives inside the canvas it keeps the canvas background for any theme, leaves the scrollbar at the window edge, and lets notices span the full editor width. Visibility is now derived state (unresolved notes or a selected note, large viewport, All notes sidebar closed) instead of a complementary area, so FLOATING_NOTES_SIDEBAR, useEnableFloatingSidebar, and the related focusNote plumbing are removed. The geometry assertions in the layout spec are refined to measure the canvas surface rather than the skeleton body, since the Settings sidebar can now legitimately coexist.
The floating notes now live in a dedicated 'Notes' landmark region instead of the complementary area's 'Editor settings' region. The shared test utils accept either region so they work with both the floating panel and the All notes sidebar.
adamsilverstein
added
[Type] Bug
|
Size Change: +630 B (+0.01%) Total Size: 7.75 MB 📦 View Changed
|
|
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. |
|
With this in place, the need for the collapsed mode and the ability to hide notes are probably even more important: |
Mamaduka
left a comment
Mamaduka
left a comment
There was a problem hiding this comment.
Interesting approach. I knew we'll have to move away from the Sidebars at some point.
Will try to test this properly as soon as possible.
Use a Symbol key for the EditorFloatingNotes slot-fill so it cannot be targeted by string name from outside the editor package, addressing review feedback.
The floating notes overlay was anchored to the window edge, but the reserved space inside the canvas ends where the vertical scrollbar begins. This left the notes offset by the scrollbar width - tucked against the scrollbar with extra space on the opposite side, most visible beside full-width (alignfull) content. Inset the overlay by the measured canvas scrollbar width so the notes sit centered in the visible reserved space. The width is measured from the iframe (0 with overlay scrollbars) and re-synced via a ResizeObserver as the scrollbar appears and disappears with content. Add an e2e assertion that the floating note is centered in the space reserved beside a full-width cover block.
|
Fixed note alignment in 338dd6c Before After Comparison |
WordPress core's WP_REST_Attachments_Controller now declares create_item_from_url() as protected. The Gutenberg subclass overrode it as private, which PHP rejects because a child method cannot reduce the visibility of an inherited method, producing a fatal error that crashed the whole plugin on newer core (breaking PHPUnit and all Playwright jobs). Widen the override to protected to match the parent.
adamsilverstein
added
the
No Core Sync Required
… try/notes-in-canvas-margin
|
Added in 9d6df53: a thin vertical divider now marks the boundary of the reserved space, so full-wide blocks visibly stop at the notes column while it stays part of the canvas. It's drawn inside the canvas document at 10% The overlap-boundaries concept reads well as a follow-up once resize/collapse controls come into play. |
|
@jasmussen How should this look when the image is "Full width"? right now I have the border running at the right edge of the image, but I feel like it could use some padding between the line and the image: 
|
The divider hung off body::after, but the body's overflow clip constrains even fixed-position pseudo-elements, cutting the line short, and it sat exactly on the clip boundary where full-wide blocks touched it. Move it to :root::after (outside the clip) and inset it 8px into the reserved space so clipped full-bleed content keeps a visible gap and the line spans the visible canvas top to bottom.
|
I noticed in Google Docs the divider only shows on hover. Additionally, it acts as a way to switch to minified notes: 
And back to expanded mode: 
|
…ture Port the Minimize notes mode onto the floating-notes-in-canvas-margin implementation from #79864, matching the combined test branch (#79971) byte for byte: - Minimized threads collapse to an avatar pill that reveals the author name and time on hover or focus; selecting a pill expands the thread. - The reserved canvas width follows the presentation: 280px for full threads, 82px for pills, and the boundary divider scales with it. - The panel also yields progressively as the canvas narrows: full threads collapse to pills below 880px and the pills release their reserved space below 482px.
Rebuilds the Minimize notes mode on the canvas-margin floating notes architecture from #79864, replacing the FLOATING_NOTES_SIDEBAR-based implementation (previous head 63a217b). The four experimental notes keyboard shortcuts from the previous implementation are not carried over; they can return as a follow-up if still wanted.
What you have I feel is 99% correct insofar as I see the image going RIGHT up to the border, flush with it. But zooming in it looks like it SLIGHTLY overlaps the border precisely, i.e. if you add 1px right-padding, I picture it would look right. As far as the collapse button, yes indeed it is possible for us to build such a thing too. It's actually in one of the mockups, blink and you'll miss it: 
I don't think it's worth investing too heavily into, in the near future. I think there's some other infrastructure that's worth stabilising a bit first. But yes, that could be a thing if we decide it would be good! |
Per design feedback, full-wide content should run right up to the divider without overlapping it, so shrink the inset from 8px to a 1px hairline gap.
@jasmussen - Fixed in bba737f - the divider inset is now a 1px hairline gap, so full-wide content sits flush with the line without overlapping it. |
The trunk merge brought in the note-form rework that renamed the add-note form's class from __thread to __add-note. The floating overlay disables pointer events and only restored them for __thread, so every click on the new-note form fell through to the canvas iframe, failing the notes e2e tests.
The trunk merge added focus-behavior and mention tests that locate note threads in the 'Editor settings' sidebar region. On this branch those notes render in the floating overlay, a region named 'Notes', so the locators never resolved. Sidebar-scoped locators in tests that open the All notes sidebar are left untouched.
|
Nice, honestly works fantastic to me. How do you feel about it? 
Nit: I see a 1px gap between the fullwide image and the vertical border, not flush. I may have confused with my 1px padding comment. Ideally it's perfectly flush, but doesn't overlap the border. Is it a subpixel rendering thing? |
t-hamano
removed
the
No Core Sync Required
github-project-automation
Bot
moved this to 🔎 Needs Review
in WordPress 7.1 Editor Tasks
t-hamano
moved this from 🔎 Needs Review
to 🦵 Punted to 7.2
in WordPress 7.1 Editor Tasks
|
Based on this discussion, I'd like to punt this to 7.2. |
…margin # Conflicts: # packages/editor/src/components/collab-sidebar/format.js
4 participants
What?
Follow-up to #77484.
Fixes #79970.
Fixes #73917.
Retires the "transparent sidebar" approach for floating notes. The floating notes panel now overlays space reserved inside the editor canvas, instead of occupying a complementary-area sidebar column.
Why?
Floating notes currently render inside a transparent
PluginSidebar. Because the editor layout treats any active sidebar as a layout-occupying column, this:The design direction discussed in #77484 (and earlier in #66377 (comment)) treats notes as part of the canvas surface - "notes in the margin of an essay" - that can coexist with the inspector.
How?
padding-rightis applied to the canvas document'shtmlelement (the approach @ellatrix suggested). Because the reserved space lives inside the canvas document, it keeps the canvas background for any theme with no color detection needed, and the canvas - and its scrollbar - stay at the full editor width.overflow-x: clipis applied to the canvasbodyso full-bleed content (e.g.alignfull) can't render under the notes.FloatingNotesSlotplaced byVisualEditor, absolutely positioned over the reserved space, in a dedicated "Notes" landmark region. The existing floating board machinery (position computation,--canvas-scrollsync) is unchanged.FLOATING_NOTES_SIDEBAR,useEnableFloatingSidebar, and the relatedfocusNote/format plumbing are removed. This is what allows the notes and the Settings sidebar to be open at the same time.The "All notes" sidebar is untouched: it remains a regular
PluginSidebar, still hides the floating panel while open, and remains the only notes surface on small viewports.Screencast
Before
before.mp4
After
With a note present and the Settings sidebar open: the notice spans the full editor width, the canvas (and its scrollbar) reach the sidebar edge, and the note floats over the canvas surface with the theme's own background.
export-1783280959632.mp4
Adding Note alongside a full width image:
full.width.mp4
Test plan
Test in playground https://playground.wordpress.net/?gutenberg-pr=79864
Note: Test this feature combined with the notes display modes (expand / minimize / hide) in #76024, which stacks on this PR.
Written test-first:
test/e2e/specs/editor/various/notes-canvas-layout.spec.jsencodes the design contract and fails on trunk (first commit) before the fix (second commit):alignfull) content never renders under the notes area (hit-test probe).Existing
block-notes.spec.jsandcollaboration-notes.spec.tsare updated for the region rename ("Editor settings" → "Notes" for the floating surface) and pass locally, as do the collab-sidebar unit tests.Notes for reviewers
isRTL()) so the canvas padding always matches the physical side of the overlay..editor-styles-wrapperelement.