Notes: Keep tall floating threads scrollable on short content

packages/editor/src/components/collab-sidebar/note-thread.js

@@ -26,7 +26,11 @@ import { Note } from './note';
 import { NoteCard } from './note-card';
 import { NoteForm } from './note-form';
 import { FloatingContainer } from './floating-container';
-import { focusNoteThread, getNoteExcerpt } from './utils';
+import {
+	focusNoteThread,
+	getNoteExcerpt,
+	scrollNoteThreadIntoView,
+} from './utils';
 import { store as editorStore } from '../../store';
 import { unlock } from '../../lock-unlock';
 
@@ -68,6 +72,15 @@ export function NoteThread( {
 		return () => unregisterThread?.( note.id );
 	}, [ relatedBlockElement, note.id, registerThread, unregisterThread ] );
 
+	// Scroll the thread into view when it becomes selected, and re-scroll
+	// when its floating position settles after `useFloatingBoard` recomputes.
+	useEffect( () => {
+		if ( ! isSelected || note.id === 'new' ) {
+			return;
+		}
+		scrollNoteThreadIntoView( note.id, sidebarRef.current );
+	}, [ isSelected, floating?.y, note.id, sidebarRef ] );
+
 	const onMouseEnter = () => {
 		debouncedToggleBlockHighlight( note.blockClientId, true );
 	};
@@ -113,26 +126,31 @@ export function NoteThread( {
 		// - An element other than a note is clicked.
 		// - Focus was lost by tabbing.
 		toggleBlockHighlight( note.blockClientId, false );
-		unselectNote();
+		onDeselectNote();
 	};
 
-	const handleNoteSelect = () => {
+	const onSelectNote = () => {
+		if ( isSelected ) {
+			return;
+		}
+
 		selectNote( note.id );
+		focusNoteThread( note.id, sidebarRef.current );
 		toggleBlockSpotlight( note.blockClientId, true );
 		if ( !! note.blockClientId ) {
 			// Pass `null` as the second parameter to prevent focusing the block.
 			selectBlock( note.blockClientId, null );
 		}
 	};
 
-	const unselectNote = () => {
+	const onDeselectNote = () => {
 		selectNote( undefined );
 		toggleBlockSpotlight( note.blockClientId, false );
 	};
 
 	const handleResolve = () => {
 		onEditNote( { id: note.id, status: 'approved' } );
-		unselectNote();
+		onDeselectNote();
 		if ( isFloating ) {
 			relatedBlockElement?.focus();
 		} else {
@@ -181,7 +199,7 @@ export function NoteThread( {
 			} ) }
 			id={ `note-thread-${ note.id }` }
 			gap="md"
-			onClick={ handleNoteSelect }
+			onClick={ onSelectNote }
 			onMouseEnter={ onMouseEnter }
 			onMouseLeave={ onMouseLeave }
 			onFocus={ onFocus }
@@ -247,9 +265,9 @@ export function NoteThread( {
 						size="compact"
 						variant="tertiary"
 						className="editor-collab-sidebar-panel__more-reply-button"
-						onClick={ () => {
-							selectNote( note.id );
-							focusNoteThread( note.id, sidebarRef.current );
+						onClick={ ( event ) => {
+							event.stopPropagation();
+							onSelectNote();
 						} }
 					>
 						{ sprintf(
@@ -295,7 +313,7 @@ export function NoteThread( {
 						onCancel={ ( event ) => {
 							// Prevent the parent onClick from being triggered.
 							event.stopPropagation();
-							unselectNote();
+							onDeselectNote();
 							focusNoteThread( note.id, sidebarRef.current );
 						} }
 						labels={ {

packages/editor/src/components/collab-sidebar/notes.js

@@ -111,12 +111,15 @@ export function Notes( { notes, sidebarRef, isFloating = false, styles } ) {
 			return;
 		}
 
-		if ( nextThread ) {
-			selectNote( nextThread.id );
-			focusNoteThread( nextThread.id, sidebarRef.current );
-		} else if ( prevThread ) {
-			selectNote( prevThread.id );
-			focusNoteThread( prevThread.id, sidebarRef.current );
+		const adjacentThread = nextThread ?? prevThread;
+		if ( adjacentThread ) {
+			selectNote( adjacentThread.id );
+			focusNoteThread( adjacentThread.id, sidebarRef.current );
+			if ( adjacentThread.blockClientId ) {
+				toggleBlockSpotlight( adjacentThread.blockClientId, true );
+				// Pass `null` as the second parameter to prevent focusing the block.
+				selectBlock( adjacentThread.blockClientId, null );
+			}
 		} else {
 			selectNote( undefined );
 			toggleBlockSpotlight( note.blockClientId, false );

packages/editor/src/components/collab-sidebar/style.scss

@@ -51,6 +51,10 @@
 		// so each thread tracks the canvas scroll.
 		transform: translateY(var(--canvas-scroll, 0));
 		will-change: transform;
+		// Keep the reply input reachable when a thread is taller than the viewport
+		// (e.g. many replies on short content where the canvas can't scroll).
+		max-height: calc(100dvh - var(--wp-admin--admin-bar--height, 0px) - #{$header-height + $grid-unit-80});
+		overflow-y: auto;
 	}
 }
 

packages/editor/src/components/collab-sidebar/utils.js

@@ -198,18 +198,17 @@ export function calculateNotePositions( {
 }
 
 /**
- * Shift focus to the note thread associated with a particular note ID.
- * If an additional selector is provided, the focus will be shifted to the element matching the selector.
+ * Resolve the DOM element for a note thread once it's mounted,
+ * or `null` if not found within 3 seconds.
  *
- * @typedef {import('@wordpress/element').RefObject} RefObject
- *
- * @param {string}       noteId             The ID of the note thread to focus.
- * @param {?HTMLElement} container          The container element to search within.
- * @param {string}       additionalSelector The additional selector to focus on.
+ * @param {string}       noteId             Note thread ID.
+ * @param {?HTMLElement} container          Container to search within.
+ * @param {string}       additionalSelector Optional descendant selector.
+ * @return {Promise<HTMLElement|null>} Resolved element, or `null` on timeout.
  */
-export function focusNoteThread( noteId, container, additionalSelector ) {
+function findNoteThread( noteId, container, additionalSelector ) {
 	if ( ! container ) {
-		return;
+		return Promise.resolve( null );
 	}
 
 	// A thread without a noteId is a new note thread.
@@ -236,15 +235,43 @@ export function focusNoteThread( noteId, container, additionalSelector ) {
 			}
 		} );
 
-		observer.observe( container, {
-			childList: true,
-			subtree: true,
-		} );
+		observer.observe( container, { childList: true, subtree: true } );
 
 		// Stop trying after 3 seconds.
 		timer = setTimeout( () => {
 			observer.disconnect();
 			resolve( null );
 		}, 3000 );
-	} ).then( ( element ) => element?.focus() );
+	} );
+}
+
+/**
+ * Focus a note thread (or a descendant) and scroll it into view.
+ *
+ * @param {string}       noteId             Note thread ID.
+ * @param {?HTMLElement} container          Container to search within.
+ * @param {string}       additionalSelector Optional descendant selector.
+ */
+export function focusNoteThread( noteId, container, additionalSelector ) {
+	return findNoteThread( noteId, container, additionalSelector ).then(
+		( element ) => {
+			if ( ! element ) {
+				return;
+			}
+			element.focus();
+			element.scrollIntoView( { block: 'nearest' } );
+		}
+	);
+}
+
+/**
+ * Scroll a note thread into view without changing focus.
+ *
+ * @param {string}       noteId    Note thread ID.
+ * @param {?HTMLElement} container Container to search within.
+ */
+export function scrollNoteThreadIntoView( noteId, container ) {
+	return findNoteThread( noteId, container ).then( ( element ) => {
+		element?.scrollIntoView( { block: 'nearest' } );
+	} );
 }