Browse Source

refactor: complete implementation phase and enhance off-screen indicator functionality with click handling

Yuki Takei 2 days ago
parent
commit
c9e63ab23c

+ 3 - 3
.kiro/specs/collaborative-editor-awareness/spec.json

@@ -3,7 +3,7 @@
   "created_at": "2026-04-07T00:00:00.000Z",
   "updated_at": "2026-04-16T09:00:00.000Z",
   "language": "en",
-  "phase": "tasks-generated",
+  "phase": "implementation-complete",
   "approvals": {
     "requirements": {
       "generated": true,
@@ -15,9 +15,9 @@
     },
     "tasks": {
       "generated": true,
-      "approved": false
+      "approved": true
     }
   },
-  "ready_for_implementation": false,
+  "ready_for_implementation": true,
   "cleanup_completed": false
 }

+ 10 - 10
.kiro/specs/collaborative-editor-awareness/tasks.md

@@ -93,20 +93,20 @@
 
 ## Phase 3: Off-Screen Indicator Click & Username Tooltip (Requirements 6.6–6.7, 7)
 
-- [ ] 17. Add click-to-scroll to off-screen cursor indicators
-- [ ] 17.1 Extend the off-screen indicator to accept and fire a click callback
+- [x] 17. Add click-to-scroll to off-screen cursor indicators
+- [x] 17.1 Extend the off-screen indicator to accept and fire a click callback
   - Add a user identifier and an optional click callback to the indicator creation options
   - Attach a click event listener on the indicator's root element that invokes the callback with the user identifier
   - Apply pointer cursor styling when a click handler is provided; omit it when not
   - _Requirements: 6.6, 6.7_
 
-- [ ] 17.2 Wire the scroll function to off-screen indicators via a mutable ref in the editor mode hook
+- [x] 17.2 Wire the scroll function to off-screen indicators via a mutable ref in the editor mode hook
   - Accept a mutable ref option in the rich cursor extension factory for the indicator click callback
   - When building off-screen indicators, pass the ref's current value as the click handler for each indicator
   - In the collaborative editor mode hook, create the mutable ref alongside the extension, write the existing scroll function to it when the provider is ready, and clear it on cleanup
   - _Requirements: 6.6_
 
-- [ ] 18. (P) Refactor the UserPicture component to eliminate the tooltip higher-order component
+- [x] 18. (P) Refactor the UserPicture component to eliminate the tooltip higher-order component
   - Remove the higher-order component that wraps the root element and tooltip in a React Fragment
   - Render the tooltip directly within the component's render body as a child of the root element, conditionally based on the noTooltip flag
   - Preserve the dynamic import for the tooltip component so that consumers who suppress tooltips never trigger the tooltip chunk load
@@ -114,31 +114,31 @@
   - The root element is always a single span, making it safe inside flex containers
   - _Requirements: 7.1, 7.3, 7.4, 7.5_
 
-- [ ] 19. Enable the native UserPicture tooltip in EditingUserList
+- [x] 19. Enable the native UserPicture tooltip in EditingUserList
   - Remove the tooltip-suppression flag from UserPicture in the avatar wrapper so the built-in tooltip renders automatically for all avatars
   - Both the first-four direct avatars and the overflow popover avatars use the same wrapper, so tooltips appear uniformly
   - _Requirements: 7.2_
   - _Depends on: Task 18_
 
-- [ ]\* 20. Test coverage for off-screen click and tooltip refactoring
-- [ ]\* 20.1 (P) Unit tests for off-screen indicator click behavior
+- [x]\* 20. Test coverage for off-screen click and tooltip refactoring
+- [x]\* 20.1 (P) Unit tests for off-screen indicator click behavior
   - Verify the indicator root has pointer cursor when a click handler is provided
   - Verify clicking the indicator calls the callback with the correct user identifier
   - Verify no click handler or pointer cursor when the callback is omitted
   - _Requirements: 6.6, 6.7_
 
-- [ ]\* 20.2 (P) Integration test for off-screen indicator scroll wiring
+- [x]\* 20.2 (P) Integration test for off-screen indicator scroll wiring
   - Verify that when the mutable ref holds a function and an off-screen indicator is clicked, the function is called with the correct user identifier
   - Verify that clicking when the ref is null does not throw
   - _Requirements: 6.6_
 
-- [ ]\* 20.3 (P) Unit tests for UserPicture tooltip refactoring
+- [x]\* 20.3 (P) Unit tests for UserPicture tooltip refactoring
   - Verify that without the tooltip-suppression flag, the component renders a single root element containing the image and a portal tooltip
   - Verify that with the tooltip-suppression flag, only the image is rendered inside the root element
   - Verify tooltip content includes the username prefix and display name
   - _Requirements: 7.1, 7.3, 7.4_
 
-- [ ]\* 20.4 Unit tests for EditingUserList tooltip integration
+- [x]\* 20.4 Unit tests for EditingUserList tooltip integration
   - Verify the avatar wrapper renders UserPicture without the tooltip-suppression flag
   - Verify tooltips are present for both direct avatars and overflow popover avatars
   - _Requirements: 7.2_

+ 56 - 1
apps/app/src/client/components/PageEditor/EditorNavbar/EditingUserList.spec.tsx

@@ -19,11 +19,17 @@ vi.mock('@growi/ui/dist/components', () => ({
   UserPicture: ({
     user,
     className,
+    noTooltip,
   }: {
     user: EditingClient;
     className?: string;
+    noTooltip?: boolean;
   }) => (
-    <span data-testid={`user-picture-${user.clientId}`} className={className}>
+    <span
+      data-testid={`user-picture-${user.clientId}`}
+      data-no-tooltip={noTooltip ? 'true' : undefined}
+      className={className}
+    >
       {user.name}
     </span>
   ),
@@ -202,3 +208,52 @@ describe('EditingUserList — Task 16.1', () => {
     });
   });
 });
+
+/**
+ * Task 20.4 — EditingUserList tooltip integration
+ * Requirements: 7.2
+ */
+describe('EditingUserList — Task 20.4 (tooltip integration)', () => {
+  describe('Req 7.2 — UserPicture rendered without noTooltip so tooltip is active', () => {
+    it('does not pass noTooltip to UserPicture for direct avatars', () => {
+      render(<EditingUserList clientList={[clientAlice]} />);
+
+      const pic = screen.getByTestId('user-picture-1');
+      // data-no-tooltip attribute is only set when noTooltip=true; should be absent
+      expect(pic.getAttribute('data-no-tooltip')).toBeNull();
+    });
+
+    it('does not pass noTooltip to UserPicture for all first-4 direct avatars', () => {
+      render(
+        <EditingUserList
+          clientList={[clientAlice, clientBob, clientCarol, clientDave]}
+        />,
+      );
+
+      for (const client of [clientAlice, clientBob, clientCarol, clientDave]) {
+        const pic = screen.getByTestId(`user-picture-${client.clientId}`);
+        expect(pic.getAttribute('data-no-tooltip')).toBeNull();
+      }
+    });
+
+    it('does not pass noTooltip to UserPicture for overflow popover avatars', async () => {
+      render(
+        <EditingUserList
+          clientList={[
+            clientAlice,
+            clientBob,
+            clientCarol,
+            clientDave,
+            clientEve,
+          ]}
+        />,
+      );
+
+      // Open the overflow popover
+      await userEvent.click(screen.getByRole('button', { name: /^\+1$/ }));
+
+      const evePic = screen.getByTestId('user-picture-5');
+      expect(evePic.getAttribute('data-no-tooltip')).toBeNull();
+    });
+  });
+});

+ 1 - 1
apps/app/src/client/components/PageEditor/EditorNavbar/EditingUserList.tsx

@@ -25,7 +25,7 @@ const AvatarWrapper: FC<{
       style={{ border: `2px solid ${client.color}` }}
       onClick={() => onUserClick?.(client.clientId)}
     >
-      <UserPicture user={client} noLink noTooltip />
+      <UserPicture user={client} noLink />
     </button>
   );
 };

+ 24 - 4
packages/editor/src/client/services-internal/extensions/y-rich-cursors/index.ts

@@ -2,26 +2,46 @@ import type { Extension } from '@codemirror/state';
 import { ViewPlugin } from '@codemirror/view';
 import type { WebsocketProvider } from 'y-websocket';
 
+import type { ScrollCallbackRef } from './plugin';
 import { YRichCursorsPluginValue } from './plugin';
 import { richCursorsTheme } from './theme';
 
 export type { OffScreenIndicatorOptions } from './off-screen-indicator';
 export { createOffScreenIndicator } from './off-screen-indicator';
+export type { ScrollCallbackRef } from './plugin';
 export { RichCaretWidget } from './widget';
 
 type Awareness = WebsocketProvider['awareness'];
 
+/** Options for the yRichCursors extension. */
+export type YRichCursorsOptions = {
+  /**
+   * Mutable ref holding the scroll-to-remote-cursor callback.
+   * When set, off-screen indicator clicks invoke ref.current(clientId).
+   * Null or unset means clicks are no-ops.
+   */
+  onClickIndicator?: ScrollCallbackRef;
+};
+
 /**
  * Creates a CodeMirror Extension that renders remote user cursors with
  * name labels and avatar images, reading user data from state.editors.
  *
  * Also broadcasts the local user's cursor position via state.cursor.
+ * Renders clickable off-screen indicators for cursors outside the viewport.
  */
-export function yRichCursors(awareness: Awareness): Extension {
+export function yRichCursors(
+  awareness: Awareness,
+  options?: YRichCursorsOptions,
+): Extension {
   return [
-    ViewPlugin.define((view) => new YRichCursorsPluginValue(view, awareness), {
-      decorations: (v) => (v as YRichCursorsPluginValue).decorations,
-    }),
+    ViewPlugin.define(
+      (view) =>
+        new YRichCursorsPluginValue(view, awareness, options?.onClickIndicator),
+      {
+        decorations: (v) => (v as YRichCursorsPluginValue).decorations,
+      },
+    ),
     richCursorsTheme,
   ];
 }

+ 92 - 0
packages/editor/src/client/services-internal/extensions/y-rich-cursors/off-screen-indicator.spec.ts

@@ -14,6 +14,7 @@ describe('createOffScreenIndicator', () => {
   it('renders an indicator with an upward Material Symbol for direction "above"', () => {
     const el = createOffScreenIndicator({
       direction: 'above',
+      clientId: 1,
       color: '#ff0000',
       name: 'Alice',
       imageUrlCached: '/avatar.png',
@@ -29,6 +30,7 @@ describe('createOffScreenIndicator', () => {
   it('renders an indicator with a downward Material Symbol for direction "below"', () => {
     const el = createOffScreenIndicator({
       direction: 'below',
+      clientId: 1,
       color: '#ff0000',
       name: 'Alice',
       imageUrlCached: '/avatar.png',
@@ -43,6 +45,7 @@ describe('createOffScreenIndicator', () => {
   it('places the arrow before the avatar for direction "above"', () => {
     const el = createOffScreenIndicator({
       direction: 'above',
+      clientId: 1,
       color: '#ff0000',
       name: 'Alice',
       imageUrlCached: undefined,
@@ -56,6 +59,7 @@ describe('createOffScreenIndicator', () => {
   it('places the avatar before the arrow for direction "below"', () => {
     const el = createOffScreenIndicator({
       direction: 'below',
+      clientId: 1,
       color: '#ff0000',
       name: 'Alice',
       imageUrlCached: undefined,
@@ -69,6 +73,7 @@ describe('createOffScreenIndicator', () => {
   it('renders an avatar image when imageUrlCached is provided', () => {
     const el = createOffScreenIndicator({
       direction: 'above',
+      clientId: 1,
       color: '#ff0000',
       name: 'Alice',
       imageUrlCached: '/avatar.png',
@@ -85,6 +90,7 @@ describe('createOffScreenIndicator', () => {
   it('renders initials fallback when imageUrlCached is undefined', () => {
     const el = createOffScreenIndicator({
       direction: 'above',
+      clientId: 1,
       color: '#ff0000',
       name: 'Alice',
       imageUrlCached: undefined,
@@ -102,6 +108,7 @@ describe('createOffScreenIndicator', () => {
   it('applies cm-yRichCursorActive class when isActive is true', () => {
     const el = createOffScreenIndicator({
       direction: 'above',
+      clientId: 1,
       color: '#ff0000',
       name: 'Alice',
       imageUrlCached: '/avatar.png',
@@ -114,6 +121,7 @@ describe('createOffScreenIndicator', () => {
   it('does NOT apply cm-yRichCursorActive class when isActive is false', () => {
     const el = createOffScreenIndicator({
       direction: 'above',
+      clientId: 1,
       color: '#ff0000',
       name: 'Alice',
       imageUrlCached: '/avatar.png',
@@ -126,6 +134,7 @@ describe('createOffScreenIndicator', () => {
   it('applies border-color on the indicator wrapper from the color parameter', () => {
     const el = createOffScreenIndicator({
       direction: 'above',
+      clientId: 1,
       color: '#ff0000',
       name: 'Alice',
       imageUrlCached: undefined,
@@ -138,6 +147,7 @@ describe('createOffScreenIndicator', () => {
   it('sets borderColor on the avatar img to the cursor color', () => {
     const el = createOffScreenIndicator({
       direction: 'above',
+      clientId: 1,
       color: '#ff0000',
       name: 'Alice',
       imageUrlCached: '/avatar.png',
@@ -153,6 +163,7 @@ describe('createOffScreenIndicator', () => {
   it('sets borderColor on the initials element to the cursor color', () => {
     const el = createOffScreenIndicator({
       direction: 'above',
+      clientId: 1,
       color: '#0000ff',
       name: 'Alice',
       imageUrlCached: undefined,
@@ -168,6 +179,7 @@ describe('createOffScreenIndicator', () => {
   it('sets borderColor on the onerror-fallback initials to the cursor color', () => {
     const el = createOffScreenIndicator({
       direction: 'below',
+      clientId: 1,
       color: '#00ff00',
       name: 'Alice',
       imageUrlCached: '/broken.png',
@@ -183,3 +195,83 @@ describe('createOffScreenIndicator', () => {
     expect(initials?.style.borderColor).toBe('#00ff00');
   });
 });
+
+/**
+ * Task 20.1 — Click behavior tests for off-screen indicators
+ * Requirements: 6.6, 6.7
+ */
+describe('createOffScreenIndicator — click behavior (Task 20.1)', () => {
+  it('applies cursor: pointer on the indicator root when onClick is provided', () => {
+    const el = createOffScreenIndicator({
+      direction: 'above',
+      clientId: 42,
+      color: '#ff0000',
+      name: 'Alice',
+      imageUrlCached: undefined,
+      isActive: false,
+      onClick: vi.fn(),
+    });
+
+    expect(el.style.cursor).toBe('pointer');
+  });
+
+  it('does NOT apply cursor: pointer when onClick is not provided', () => {
+    const el = createOffScreenIndicator({
+      direction: 'above',
+      clientId: 42,
+      color: '#ff0000',
+      name: 'Alice',
+      imageUrlCached: undefined,
+      isActive: false,
+    });
+
+    expect(el.style.cursor).toBe('');
+  });
+
+  it('calls onClick with the correct clientId when indicator is clicked', () => {
+    const onClick = vi.fn();
+    const el = createOffScreenIndicator({
+      direction: 'above',
+      clientId: 42,
+      color: '#ff0000',
+      name: 'Alice',
+      imageUrlCached: undefined,
+      isActive: false,
+      onClick,
+    });
+
+    el.dispatchEvent(new Event('click'));
+
+    expect(onClick).toHaveBeenCalledOnce();
+    expect(onClick).toHaveBeenCalledWith(42);
+  });
+
+  it('does not throw when clicked and onClick is not provided', () => {
+    const el = createOffScreenIndicator({
+      direction: 'below',
+      clientId: 99,
+      color: '#00ff00',
+      name: 'Bob',
+      imageUrlCached: undefined,
+      isActive: false,
+    });
+
+    expect(() => el.dispatchEvent(new Event('click'))).not.toThrow();
+  });
+
+  it('works with clientId 0 (boundary value)', () => {
+    const onClick = vi.fn();
+    const el = createOffScreenIndicator({
+      direction: 'below',
+      clientId: 0,
+      color: '#0000ff',
+      name: 'Zero',
+      imageUrlCached: undefined,
+      isActive: false,
+      onClick,
+    });
+
+    el.dispatchEvent(new Event('click'));
+    expect(onClick).toHaveBeenCalledWith(0);
+  });
+});

+ 18 - 1
packages/editor/src/client/services-internal/extensions/y-rich-cursors/off-screen-indicator.ts

@@ -2,10 +2,14 @@ import { toInitials } from './widget';
 
 export type OffScreenIndicatorOptions = {
   direction: 'above' | 'below';
+  /** Client ID of the remote user; passed to onClick when the indicator is clicked. */
+  clientId: number;
   color: string;
   name: string;
   imageUrlCached: string | undefined;
   isActive: boolean;
+  /** Invoked with clientId when the indicator is clicked. Omit to suppress click handling. */
+  onClick?: (clientId: number) => void;
 };
 
 /**
@@ -29,7 +33,15 @@ export type OffScreenIndicatorOptions = {
 export function createOffScreenIndicator(
   opts: OffScreenIndicatorOptions,
 ): HTMLElement {
-  const { direction, color, name, imageUrlCached, isActive } = opts;
+  const {
+    direction,
+    clientId,
+    color,
+    name,
+    imageUrlCached,
+    isActive,
+    onClick,
+  } = opts;
 
   const indicator = document.createElement('span');
   indicator.className = 'cm-offScreenIndicator';
@@ -38,6 +50,11 @@ export function createOffScreenIndicator(
     indicator.classList.add('cm-yRichCursorActive');
   }
 
+  if (onClick != null) {
+    indicator.style.cursor = 'pointer';
+    indicator.addEventListener('click', () => onClick(clientId));
+  }
+
   const arrow = document.createElement('span');
   arrow.className = 'material-symbols-outlined cm-offScreenArrow';
   arrow.style.color = color;

+ 122 - 0
packages/editor/src/client/services-internal/extensions/y-rich-cursors/plugin.integ.ts

@@ -395,6 +395,128 @@ describe('Task 12.2 — Off-screen indicator renders for cursor in render buffer
   });
 });
 
+// ---------------------------------------------------------------------------
+// Task 20.2 — Off-screen indicator scroll wiring via scrollCallbackRef
+// ---------------------------------------------------------------------------
+
+describe('Task 20.2 — Off-screen indicator click wires through scrollCallbackRef', () => {
+  it('invokes ref.current with the correct clientId when an off-screen indicator is clicked', () => {
+    const ydoc = new Y.Doc({ guid: 'click-wire-test' });
+    const ytext = ydoc.getText('codemirror');
+    // Insert content long enough to push a remote cursor off-screen
+    const content = 'Line\n'.repeat(200);
+    ytext.insert(0, content);
+
+    const awareness = new FakeAwareness(ydoc);
+
+    const scrollFn = vi.fn();
+    const scrollCallbackRef: { current: ((clientId: number) => void) | null } =
+      { current: scrollFn };
+
+    const state = EditorState.create({
+      doc: content,
+      extensions: [
+        yCollab(ytext, null),
+        yRichCursors(awareness as never, {
+          onClickIndicator: scrollCallbackRef,
+        }),
+      ],
+    });
+
+    const container = document.createElement('div');
+    container.style.height = '100px';
+    container.style.overflow = 'auto';
+    document.body.appendChild(container);
+
+    const view = new EditorView({ state, parent: container });
+
+    // Place remote cursor far from viewport (end of document)
+    const farIndex = content.length - 10;
+    const anchor = Y.createRelativePositionFromTypeIndex(ytext, farIndex);
+    const head = Y.createRelativePositionFromTypeIndex(ytext, farIndex);
+    const remoteClient = makeClient(77, 'FarUser');
+
+    awareness.setRemoteClientState(77, {
+      editors: remoteClient,
+      cursor: { anchor, head },
+    });
+
+    // Force update so the indicator is built
+    view.dispatch({});
+
+    // Find the off-screen indicator in the bottom container
+    const bottomContainer = view.dom.querySelector('.cm-offScreenBottom');
+    const indicator = bottomContainer?.querySelector(
+      '.cm-offScreenIndicator',
+    ) as HTMLElement | null;
+    expect(indicator).not.toBeNull();
+    if (indicator == null) throw new Error('indicator not found');
+
+    // Click the indicator
+    indicator.dispatchEvent(new Event('click'));
+
+    expect(scrollFn).toHaveBeenCalledOnce();
+    expect(scrollFn).toHaveBeenCalledWith(77);
+
+    view.destroy();
+    container.remove();
+  });
+
+  it('does not throw when ref.current is null and indicator is clicked', () => {
+    const ydoc = new Y.Doc({ guid: 'null-ref-test' });
+    const ytext = ydoc.getText('codemirror');
+    const content = 'Line\n'.repeat(200);
+    ytext.insert(0, content);
+
+    const awareness = new FakeAwareness(ydoc);
+
+    const scrollCallbackRef: { current: ((clientId: number) => void) | null } =
+      { current: null };
+
+    const state = EditorState.create({
+      doc: content,
+      extensions: [
+        yCollab(ytext, null),
+        yRichCursors(awareness as never, {
+          onClickIndicator: scrollCallbackRef,
+        }),
+      ],
+    });
+
+    const container = document.createElement('div');
+    container.style.height = '100px';
+    container.style.overflow = 'auto';
+    document.body.appendChild(container);
+
+    const view = new EditorView({ state, parent: container });
+
+    const farIndex = content.length - 10;
+    const anchor = Y.createRelativePositionFromTypeIndex(ytext, farIndex);
+    const head = Y.createRelativePositionFromTypeIndex(ytext, farIndex);
+    const remoteClient = makeClient(88, 'NullRefUser');
+
+    awareness.setRemoteClientState(88, {
+      editors: remoteClient,
+      cursor: { anchor, head },
+    });
+
+    view.dispatch({});
+
+    const bottomContainer = view.dom.querySelector('.cm-offScreenBottom');
+    const indicator = bottomContainer?.querySelector(
+      '.cm-offScreenIndicator',
+    ) as HTMLElement | null;
+
+    // Indicator must exist so the click actually fires
+    if (indicator == null) throw new Error('indicator not found');
+    // Clicking when ref.current is null must be a silent no-op (no throw)
+    indicator.dispatchEvent(new Event('click'));
+
+    view.destroy();
+    container.remove();
+  });
+});
+
 // ---------------------------------------------------------------------------
 // Task 10.2 — Activity tracking timer lifecycle
 // ---------------------------------------------------------------------------

+ 28 - 1
packages/editor/src/client/services-internal/extensions/y-rich-cursors/plugin.ts

@@ -19,11 +19,17 @@ type AwarenessState = {
   };
 };
 
+/** Mutable ref container for the scroll-to-remote-cursor function. */
+export type ScrollCallbackRef = {
+  current: ((clientId: number) => void) | null;
+};
+
 export const yRichCursorsAnnotation = Annotation.define<number[]>();
 
 export class YRichCursorsPluginValue {
   decorations: DecorationSet;
   private readonly awareness: Awareness;
+  private readonly scrollCallbackRef: ScrollCallbackRef | undefined;
   private readonly changeListener: (update: {
     added: number[];
     updated: number[];
@@ -36,8 +42,13 @@ export class YRichCursorsPluginValue {
   private readonly topContainer: HTMLElement;
   private readonly bottomContainer: HTMLElement;
 
-  constructor(view: EditorView, awareness: Awareness) {
+  constructor(
+    view: EditorView,
+    awareness: Awareness,
+    scrollCallbackRef?: ScrollCallbackRef,
+  ) {
     this.awareness = awareness;
+    this.scrollCallbackRef = scrollCallbackRef;
     this.decorations = RangeSet.of([]);
 
     // Create off-screen containers
@@ -241,15 +252,25 @@ export class YRichCursorsPluginValue {
       const headIndex = head.index;
 
       // Classify: off-screen (above/below) or in-viewport
+      // Build a click handler from the mutable ref (captured once per indicator build).
+      // Using a closure that reads ref.current at call-time so no extension recreation
+      // is needed when the scroll function changes.
+      const onClickIndicator =
+        this.scrollCallbackRef != null
+          ? (id: number) => this.scrollCallbackRef?.current?.(id)
+          : undefined;
+
       if (rangedMode) {
         if (headIndex < vpFrom) {
           aboveIndicators.push({
             el: createOffScreenIndicator({
               direction: 'above',
+              clientId,
               color: editors.color,
               name: editors.name,
               imageUrlCached: editors.imageUrlCached,
               isActive,
+              onClick: onClickIndicator,
             }),
             headIndex,
           });
@@ -259,10 +280,12 @@ export class YRichCursorsPluginValue {
           belowIndicators.push({
             el: createOffScreenIndicator({
               direction: 'below',
+              clientId,
               color: editors.color,
               name: editors.name,
               imageUrlCached: editors.imageUrlCached,
               isActive,
+              onClick: onClickIndicator,
             }),
             headIndex,
           });
@@ -283,10 +306,12 @@ export class YRichCursorsPluginValue {
           aboveIndicators.push({
             el: createOffScreenIndicator({
               direction: 'above',
+              clientId,
               color: editors.color,
               name: editors.name,
               imageUrlCached: editors.imageUrlCached,
               isActive,
+              onClick: onClickIndicator,
             }),
             headIndex,
           });
@@ -296,10 +321,12 @@ export class YRichCursorsPluginValue {
           belowIndicators.push({
             el: createOffScreenIndicator({
               direction: 'below',
+              clientId,
               color: editors.color,
               name: editors.name,
               imageUrlCached: editors.imageUrlCached,
               isActive,
+              onClick: onClickIndicator,
             }),
             headIndex,
           });

+ 8 - 2
packages/editor/src/client/stores/use-collaborative-editor-mode.ts

@@ -1,4 +1,4 @@
-import { useEffect, useState } from 'react';
+import { useEffect, useRef, useState } from 'react';
 import { EditorView, keymap } from '@codemirror/view';
 import { YJS_WEBSOCKET_BASE_PATH } from '@growi/core/dist/consts';
 import type { IUserHasId } from '@growi/core/dist/interfaces';
@@ -91,6 +91,10 @@ export const useCollaborativeEditorMode = (
     onScrollToRemoteCursorReady,
   } = configuration ?? {};
 
+  // Stable mutable ref passed to yRichCursors so off-screen indicator clicks
+  // can invoke the scroll function without recreating the extension.
+  const scrollCallbackRef = useRef<((clientId: number) => void) | null>(null);
+
   const { primaryDoc, activeDoc } =
     useSecondaryYdocs(isEnabled, {
       pageId,
@@ -190,7 +194,7 @@ export const useCollaborativeEditorMode = (
     const extensions = [
       keymap.of(yUndoManagerKeymap),
       yCollab(activeText, null, { undoManager }),
-      yRichCursors(provider.awareness),
+      yRichCursors(provider.awareness, { onClickIndicator: scrollCallbackRef }),
     ];
 
     const cleanupFunctions = extensions.map((ext) =>
@@ -223,9 +227,11 @@ export const useCollaborativeEditorMode = (
       () => codeMirrorEditor.view,
     );
 
+    scrollCallbackRef.current = scrollFn;
     onScrollToRemoteCursorReady?.(scrollFn);
 
     return () => {
+      scrollCallbackRef.current = null;
       onScrollToRemoteCursorReady?.(null);
     };
   }, [

+ 136 - 0
packages/ui/src/components/UserPicture.spec.tsx

@@ -0,0 +1,136 @@
+import { render, screen } from '@testing-library/react';
+
+/**
+ * Unit tests for the refactored UserPicture component.
+ *
+ * Task 20.3 — UserPicture tooltip refactoring
+ * Requirements: 7.1, 7.3, 7.4, 7.5
+ *
+ * Key structural invariant:
+ *   Before refactor: withTooltip HOC returns a Fragment
+ *     → two sibling elements in the container (<span> + <Tooltip>)
+ *   After refactor: tooltip is a child of the root <span>
+ *     → exactly one child element in the container
+ */
+
+// ---------------------------------------------------------------------------
+// Module mocks
+// ---------------------------------------------------------------------------
+
+// Mock next/dynamic to return a synchronous stub component.
+// This makes the dynamically-loaded UncontrolledTooltip testable without
+// async chunk loading or portals in the test environment.
+vi.mock('next/dynamic', () => ({
+  default: (_importFn: () => Promise<unknown>, _opts?: unknown) => {
+    // The stub renders its children inline so we can inspect tooltip content.
+    const Stub = ({ children }: { children?: React.ReactNode }) => (
+      <span data-testid="mock-tooltip">{children}</span>
+    );
+    return Stub;
+  },
+}));
+
+vi.mock('next/router', () => ({
+  useRouter: () => ({ push: vi.fn() }),
+}));
+
+// ---------------------------------------------------------------------------
+// Component under test (imported AFTER mocks are in place)
+// ---------------------------------------------------------------------------
+
+import { UserPicture } from './UserPicture';
+
+// ---------------------------------------------------------------------------
+// Helpers
+// ---------------------------------------------------------------------------
+
+const makeUser = (overrides?: object) => ({
+  name: 'Alice',
+  username: 'alice',
+  imageUrlCached: '/avatar.png',
+  ...overrides,
+});
+
+// ---------------------------------------------------------------------------
+// Tests
+// ---------------------------------------------------------------------------
+
+describe('UserPicture — Task 20.3 (tooltip refactoring, req 7.1, 7.3, 7.4)', () => {
+  describe('Req 7.3 / 7.4 — single root element (no Fragment)', () => {
+    it('renders exactly one child element in the container when noTooltip is not set', () => {
+      const { container } = render(<UserPicture user={makeUser()} noLink />);
+
+      // After HOC removal, exactly ONE root child (the <span>).
+      // Before the fix two siblings lived at this level (span + tooltip HOC fragment).
+      expect(container.children).toHaveLength(1);
+      expect(container.firstElementChild?.tagName).toBe('SPAN');
+    });
+
+    it('renders exactly one child element in the container when noTooltip is true', () => {
+      const { container } = render(
+        <UserPicture user={makeUser()} noLink noTooltip />,
+      );
+
+      expect(container.children).toHaveLength(1);
+      expect(container.firstElementChild?.tagName).toBe('SPAN');
+    });
+  });
+
+  describe('Req 7.4 — image rendered inside the root span', () => {
+    it('renders the avatar image', () => {
+      render(<UserPicture user={makeUser()} noLink />);
+
+      // screen.getByRole throws if not found — implicit assertion of presence
+      const img = screen.getByRole('img') as HTMLImageElement;
+      expect(img.src).toContain('/avatar.png');
+    });
+
+    it('the image is nested inside the root span', () => {
+      const { container } = render(<UserPicture user={makeUser()} noLink />);
+
+      const rootSpan = container.firstElementChild;
+      expect(rootSpan?.querySelector('img')).not.toBeNull();
+    });
+  });
+
+  describe('Req 7.1 — tooltip renders when noTooltip is absent', () => {
+    it('renders the tooltip stub when noTooltip is not set and user has a name', () => {
+      render(<UserPicture user={makeUser()} noLink />);
+
+      expect(screen.queryByTestId('mock-tooltip')).not.toBeNull();
+    });
+
+    it('does not render the tooltip stub when noTooltip is true', () => {
+      render(<UserPicture user={makeUser()} noLink noTooltip />);
+
+      expect(screen.queryByTestId('mock-tooltip')).toBeNull();
+    });
+
+    it('includes @username in tooltip content when username is available', () => {
+      render(<UserPicture user={makeUser({ username: 'alice' })} noLink />);
+
+      const tooltip = screen.queryByTestId('mock-tooltip');
+      expect(tooltip?.textContent).toContain('@alice');
+    });
+
+    it('includes the display name in tooltip content', () => {
+      render(<UserPicture user={makeUser({ name: 'Alice' })} noLink />);
+
+      const tooltip = screen.queryByTestId('mock-tooltip');
+      expect(tooltip?.textContent).toContain('Alice');
+    });
+  });
+
+  describe('Req 7.3 — tooltip is nested inside root span (portal child, not sibling)', () => {
+    it('tooltip stub is a descendant of the root span (not a sibling)', () => {
+      const { container } = render(<UserPicture user={makeUser()} noLink />);
+
+      // Single root child; tooltip stub is inside it
+      expect(container.children).toHaveLength(1);
+      const rootSpan = container.firstElementChild;
+      expect(
+        rootSpan?.querySelector('[data-testid="mock-tooltip"]'),
+      ).not.toBeNull();
+    });
+  });
+});

+ 48 - 45
packages/ui/src/components/UserPicture.tsx

@@ -82,42 +82,6 @@ const UserPictureRootWithLink = forwardRef<
   );
 });
 
-// wrapper with Tooltip
-const withTooltip =
-  <P extends BaseUserPictureRootProps>(
-    UserPictureSpanElm: React.ForwardRefExoticComponent<
-      P & React.RefAttributes<HTMLSpanElement>
-    >,
-  ) =>
-  (props: P): JSX.Element => {
-    const { displayName, size } = props;
-    const username = 'username' in props ? props.username : undefined;
-
-    const tooltipClassName = `${moduleTooltipClass} user-picture-tooltip-${size ?? 'md'}`;
-    const userPictureRef = useRef<HTMLSpanElement>(null);
-
-    return (
-      <>
-        <UserPictureSpanElm ref={userPictureRef} {...props} />
-        <UncontrolledTooltip
-          placement="bottom"
-          target={userPictureRef}
-          popperClassName={tooltipClassName}
-          delay={0}
-          fade={false}
-        >
-          {username ? (
-            <>
-              {`@${username}`}
-              <br />
-            </>
-          ) : null}
-          {displayName}
-        </UncontrolledTooltip>
-      </>
-    );
-  };
-
 /**
  * type guard to determine whether the specified object is IUser
  */
@@ -181,20 +145,59 @@ export const UserPicture = memo((userProps: Props): JSX.Element => {
     .filter(Boolean)
     .join(' ');
 
+  // ref is always called unconditionally to satisfy React hooks rules.
+  // Passed to the root element so UncontrolledTooltip can target it.
+  const rootRef = useRef<HTMLSpanElement>(null);
+
+  const tooltipClassName = `${moduleTooltipClass} user-picture-tooltip-${size ?? 'md'}`;
+
   // biome-ignore lint/performance/noImgElement: ignore
   const imgElement = <img src={src} alt={displayName} className={className} />;
-  const baseProps = { displayName, size, children: imgElement };
+
+  const children = (
+    <>
+      {imgElement}
+      {showTooltip && (
+        <UncontrolledTooltip
+          placement="bottom"
+          target={rootRef}
+          popperClassName={tooltipClassName}
+          delay={0}
+          fade={false}
+        >
+          {username ? (
+            <>
+              {`@${username}`}
+              <br />
+            </>
+          ) : null}
+          {displayName}
+        </UncontrolledTooltip>
+      )}
+    </>
+  );
 
   if (username == null || noLink) {
-    const Component = showTooltip
-      ? withTooltip(UserPictureRootWithoutLink)
-      : UserPictureRootWithoutLink;
-    return <Component {...baseProps} />;
+    return (
+      <UserPictureRootWithoutLink
+        ref={rootRef}
+        displayName={displayName}
+        size={size}
+      >
+        {children}
+      </UserPictureRootWithoutLink>
+    );
   }
 
-  const Component = showTooltip
-    ? withTooltip(UserPictureRootWithLink)
-    : UserPictureRootWithLink;
-  return <Component {...baseProps} username={username} />;
+  return (
+    <UserPictureRootWithLink
+      ref={rootRef}
+      displayName={displayName}
+      size={size}
+      username={username}
+    >
+      {children}
+    </UserPictureRootWithLink>
+  );
 });
 UserPicture.displayName = 'UserPicture';

+ 3 - 0
packages/ui/test/setup.ts

@@ -0,0 +1,3 @@
+// Test setup for packages/ui
+// @testing-library/jest-dom is not a devDependency here;
+// tests use standard vitest matchers only.

+ 14 - 0
packages/ui/vitest.config.ts

@@ -0,0 +1,14 @@
+import react from '@vitejs/plugin-react';
+import tsconfigPaths from 'vite-tsconfig-paths';
+import { defineConfig } from 'vitest/config';
+
+export default defineConfig({
+  plugins: [tsconfigPaths(), react()],
+  test: {
+    clearMocks: true,
+    globals: true,
+    environment: 'happy-dom',
+    include: ['src/**/*.spec.{ts,tsx}'],
+    setupFiles: ['./test/setup.ts'],
+  },
+});