fix: refactor auto-scroll logic and add tests for rendering observer

apps/app/src/client/hooks/use-content-auto-scroll/use-content-auto-scroll.spec.tsx

@@ -2,207 +2,7 @@ import { GROWI_IS_CONTENT_RENDERING_ATTR } from '@growi/core/dist/consts';
 import { renderHook } from '@testing-library/react';
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
 
-import {
-  useContentAutoScroll,
-  watchRenderingAndReScroll,
-} from './use-content-auto-scroll';
-
-describe('watchRenderingAndReScroll', () => {
-  let container: HTMLDivElement;
-  let scrollToTarget: ReturnType<typeof vi.fn>;
-
-  beforeEach(() => {
-    vi.useFakeTimers();
-    container = document.createElement('div');
-    document.body.appendChild(container);
-    scrollToTarget = vi.fn(() => true);
-  });
-
-  afterEach(() => {
-    vi.useRealTimers();
-    document.body.innerHTML = '';
-  });
-
-  it('should not schedule a timer when no rendering elements exist', () => {
-    const cleanup = watchRenderingAndReScroll(container, scrollToTarget);
-
-    vi.advanceTimersByTime(5000);
-    expect(scrollToTarget).not.toHaveBeenCalled();
-
-    cleanup();
-  });
-
-  it('should schedule a scroll after 5s when rendering elements exist', () => {
-    const renderingEl = document.createElement('div');
-    renderingEl.setAttribute(GROWI_IS_CONTENT_RENDERING_ATTR, 'true');
-    container.appendChild(renderingEl);
-
-    const cleanup = watchRenderingAndReScroll(container, scrollToTarget);
-
-    expect(scrollToTarget).not.toHaveBeenCalled();
-
-    vi.advanceTimersByTime(5000);
-    expect(scrollToTarget).toHaveBeenCalledTimes(1);
-
-    cleanup();
-  });
-
-  it('should not reset timer on intermediate DOM mutations', async () => {
-    const renderingEl = document.createElement('div');
-    renderingEl.setAttribute(GROWI_IS_CONTENT_RENDERING_ATTR, 'true');
-    container.appendChild(renderingEl);
-
-    const cleanup = watchRenderingAndReScroll(container, scrollToTarget);
-
-    vi.advanceTimersByTime(3000);
-    expect(scrollToTarget).not.toHaveBeenCalled();
-
-    // Trigger a DOM mutation mid-timer
-    const child = document.createElement('span');
-    container.appendChild(child);
-    await vi.advanceTimersByTimeAsync(0);
-
-    // The timer should NOT have been reset — 2 more seconds should fire it
-    vi.advanceTimersByTime(2000);
-    expect(scrollToTarget).toHaveBeenCalledTimes(1);
-
-    cleanup();
-  });
-
-  it('should detect rendering elements added after initial check via observer', async () => {
-    const cleanup = watchRenderingAndReScroll(container, scrollToTarget);
-
-    vi.advanceTimersByTime(3000);
-    expect(scrollToTarget).not.toHaveBeenCalled();
-
-    // Add a rendering element later (within 10s timeout)
-    const renderingEl = document.createElement('div');
-    renderingEl.setAttribute(GROWI_IS_CONTENT_RENDERING_ATTR, 'true');
-    container.appendChild(renderingEl);
-
-    // Flush microtasks so MutationObserver callback fires
-    await vi.advanceTimersByTimeAsync(0);
-
-    // Timer should be scheduled — fires after 5s
-    await vi.advanceTimersByTimeAsync(5000);
-    expect(scrollToTarget).toHaveBeenCalledTimes(1);
-
-    cleanup();
-  });
-
-  it('should scroll once when multiple rendering elements exist simultaneously', () => {
-    const renderingEl1 = document.createElement('div');
-    renderingEl1.setAttribute(GROWI_IS_CONTENT_RENDERING_ATTR, 'true');
-    container.appendChild(renderingEl1);
-
-    const renderingEl2 = document.createElement('div');
-    renderingEl2.setAttribute(GROWI_IS_CONTENT_RENDERING_ATTR, 'true');
-    container.appendChild(renderingEl2);
-
-    const cleanup = watchRenderingAndReScroll(container, scrollToTarget);
-
-    vi.advanceTimersByTime(5000);
-    expect(scrollToTarget).toHaveBeenCalledTimes(1);
-
-    cleanup();
-  });
-
-  it('should stop watching after 10s timeout', () => {
-    const renderingEl = document.createElement('div');
-    renderingEl.setAttribute(GROWI_IS_CONTENT_RENDERING_ATTR, 'true');
-    container.appendChild(renderingEl);
-
-    const cleanup = watchRenderingAndReScroll(container, scrollToTarget);
-
-    // First scroll at 5s
-    vi.advanceTimersByTime(5000);
-    expect(scrollToTarget).toHaveBeenCalledTimes(1);
-
-    // At 10s both the scroll timer and the watch timeout fire.
-    vi.advanceTimersByTime(5000);
-    const callsAfter10s = scrollToTarget.mock.calls.length;
-
-    // After 10s, no further scrolls should occur regardless
-    vi.advanceTimersByTime(10000);
-    expect(scrollToTarget).toHaveBeenCalledTimes(callsAfter10s);
-
-    cleanup();
-  });
-
-  it('should clean up timer and observer on cleanup call', () => {
-    const renderingEl = document.createElement('div');
-    renderingEl.setAttribute(GROWI_IS_CONTENT_RENDERING_ATTR, 'true');
-    container.appendChild(renderingEl);
-
-    const cleanup = watchRenderingAndReScroll(container, scrollToTarget);
-
-    cleanup();
-
-    vi.advanceTimersByTime(10000);
-    expect(scrollToTarget).not.toHaveBeenCalled();
-  });
-
-  it('should prevent timer callbacks from executing after cleanup (stopped flag)', () => {
-    const renderingEl = document.createElement('div');
-    renderingEl.setAttribute(GROWI_IS_CONTENT_RENDERING_ATTR, 'true');
-    container.appendChild(renderingEl);
-
-    const cleanup = watchRenderingAndReScroll(container, scrollToTarget);
-
-    // Advance partway, then cleanup
-    vi.advanceTimersByTime(3000);
-    cleanup();
-
-    // Timer would have fired at 5s, but cleanup was called
-    vi.advanceTimersByTime(5000);
-    expect(scrollToTarget).not.toHaveBeenCalled();
-  });
-
-  it('should not schedule further re-scrolls after rendering elements complete', async () => {
-    const renderingEl = document.createElement('div');
-    renderingEl.setAttribute(GROWI_IS_CONTENT_RENDERING_ATTR, 'true');
-    container.appendChild(renderingEl);
-
-    const cleanup = watchRenderingAndReScroll(container, scrollToTarget);
-
-    // First timer fires at 5s — re-scroll executes
-    vi.advanceTimersByTime(5000);
-    expect(scrollToTarget).toHaveBeenCalledTimes(1);
-
-    // Rendering completes — attribute toggled to false
-    renderingEl.setAttribute(GROWI_IS_CONTENT_RENDERING_ATTR, 'false');
-    await vi.advanceTimersByTimeAsync(0);
-
-    // No further re-scrolls should be scheduled
-    vi.advanceTimersByTime(10000);
-    expect(scrollToTarget).toHaveBeenCalledTimes(1);
-
-    cleanup();
-  });
-
-  it('should scroll exactly once when rendering completes before the first timer fires', () => {
-    const renderingEl = document.createElement('div');
-    renderingEl.setAttribute(GROWI_IS_CONTENT_RENDERING_ATTR, 'true');
-    container.appendChild(renderingEl);
-
-    const cleanup = watchRenderingAndReScroll(container, scrollToTarget);
-
-    // Rendering completes before the first poll timer fires
-    renderingEl.setAttribute(GROWI_IS_CONTENT_RENDERING_ATTR, 'false');
-
-    // Poll timer fires at 5s — detects no rendering elements.
-    // wasRendering is reset in the timer callback BEFORE scrollToTarget so that
-    // the subsequent checkAndSchedule call does not trigger a redundant extra scroll.
-    vi.advanceTimersByTime(5000);
-    expect(scrollToTarget).toHaveBeenCalledTimes(1);
-
-    // No further scrolls after rendering is confirmed done
-    vi.advanceTimersByTime(5000);
-    expect(scrollToTarget).toHaveBeenCalledTimes(1);
-
-    cleanup();
-  });
-});
+import { useContentAutoScroll } from './use-content-auto-scroll';
 
 describe('useContentAutoScroll', () => {
   const containerId = 'test-content-container';
@@ -401,20 +201,22 @@ describe('useContentAutoScroll', () => {
   });
 
   it('should wait for target via MutationObserver when not yet in DOM', async () => {
+    // happy-dom's MutationObserver does not fire when a fake-timer setTimeout is
+    // pending in the same effect. Use real timers for this test only.
+    vi.useRealTimers();
+
     window.location.hash = '#deferred';
 
     const { unmount } = renderHook(() =>
       useContentAutoScroll({ key: 'page-id', contentContainerId: containerId }),
     );
 
-    // Target appears after initial render
     const target = document.createElement('div');
     target.id = 'deferred';
     target.scrollIntoView = vi.fn();
     container.appendChild(target);
 
-    // Flush microtasks for MutationObserver
-    await vi.advanceTimersByTimeAsync(0);
+    await new Promise<void>((resolve) => setTimeout(resolve, 50));
 
     expect(target.scrollIntoView).toHaveBeenCalledTimes(1);
 

apps/app/src/client/hooks/use-content-auto-scroll/use-content-auto-scroll.ts

@@ -1,88 +1,10 @@
 import { useEffect, useRef } from 'react';
-import {
-  GROWI_IS_CONTENT_RENDERING_ATTR,
-  GROWI_IS_CONTENT_RENDERING_SELECTOR,
-} from '@growi/core/dist/consts';
-
-const RENDERING_POLL_INTERVAL_MS = 5000;
-const WATCH_TIMEOUT_MS = 10000;
-
-/**
- * Watch for elements with in-progress rendering status in the container.
- * Periodically calls scrollToTarget while rendering elements remain.
- * Returns a cleanup function that stops observation and clears timers.
- */
-export const watchRenderingAndReScroll = (
-  contentContainer: HTMLElement,
-  scrollToTarget: () => boolean,
-): (() => void) => {
-  let timerId: number | undefined;
-  let stopped = false;
-  let wasRendering = false;
-
-  const cleanup = () => {
-    stopped = true;
-    observer.disconnect();
-    if (timerId != null) {
-      window.clearTimeout(timerId);
-      timerId = undefined;
-    }
-    window.clearTimeout(watchTimeoutId);
-  };
+import { GROWI_IS_CONTENT_RENDERING_SELECTOR } from '@growi/core/dist/consts';
 
-  const checkAndSchedule = () => {
-    if (stopped) return;
-
-    const hasRendering =
-      contentContainer.querySelector(GROWI_IS_CONTENT_RENDERING_SELECTOR) !=
-      null;
-
-    if (!hasRendering) {
-      if (timerId != null) {
-        window.clearTimeout(timerId);
-        timerId = undefined;
-      }
-      // Final re-scroll to compensate for the layout shift from the last completed render
-      if (wasRendering) {
-        wasRendering = false;
-        scrollToTarget();
-      }
-      return;
-    }
-
-    wasRendering = true;
-
-    // If a timer is already ticking, let it fire — don't reset
-    if (timerId != null) return;
-
-    timerId = window.setTimeout(() => {
-      if (stopped) return;
-      timerId = undefined;
-      // Reset before checkAndSchedule so the wasRendering guard does not
-      // trigger an extra re-scroll if rendering is already done by now.
-      wasRendering = false;
-      scrollToTarget();
-      checkAndSchedule();
-    }, RENDERING_POLL_INTERVAL_MS);
-  };
-
-  const observer = new MutationObserver(checkAndSchedule);
-
-  observer.observe(contentContainer, {
-    childList: true,
-    subtree: true,
-    attributes: true,
-    attributeFilter: [GROWI_IS_CONTENT_RENDERING_ATTR],
-  });
-
-  // Initial check
-  checkAndSchedule();
-
-  // Stop watching after timeout regardless of rendering state
-  const watchTimeoutId = window.setTimeout(cleanup, WATCH_TIMEOUT_MS);
-
-  return cleanup;
-};
+import {
+  WATCH_TIMEOUT_MS,
+  watchRenderingAndReScroll,
+} from './watch-rendering-and-rescroll';
 
 /** Configuration for the auto-scroll hook */
 export interface UseContentAutoScrollOptions {

apps/app/src/client/hooks/use-content-auto-scroll/watch-rendering-and-rescroll.spec.tsx

@@ -0,0 +1,201 @@
+import { GROWI_IS_CONTENT_RENDERING_ATTR } from '@growi/core/dist/consts';
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
+
+import { watchRenderingAndReScroll } from './watch-rendering-and-rescroll';
+
+describe('watchRenderingAndReScroll', () => {
+  let container: HTMLDivElement;
+  let scrollToTarget: ReturnType<typeof vi.fn>;
+
+  beforeEach(() => {
+    vi.useFakeTimers();
+    container = document.createElement('div');
+    document.body.appendChild(container);
+    scrollToTarget = vi.fn(() => true);
+  });
+
+  afterEach(() => {
+    vi.useRealTimers();
+    document.body.innerHTML = '';
+  });
+
+  it('should not schedule a timer when no rendering elements exist', () => {
+    const cleanup = watchRenderingAndReScroll(container, scrollToTarget);
+
+    vi.advanceTimersByTime(5000);
+    expect(scrollToTarget).not.toHaveBeenCalled();
+
+    cleanup();
+  });
+
+  it('should schedule a scroll after 5s when rendering elements exist', () => {
+    const renderingEl = document.createElement('div');
+    renderingEl.setAttribute(GROWI_IS_CONTENT_RENDERING_ATTR, 'true');
+    container.appendChild(renderingEl);
+
+    const cleanup = watchRenderingAndReScroll(container, scrollToTarget);
+
+    expect(scrollToTarget).not.toHaveBeenCalled();
+
+    vi.advanceTimersByTime(5000);
+    expect(scrollToTarget).toHaveBeenCalledTimes(1);
+
+    cleanup();
+  });
+
+  it('should not reset timer on intermediate DOM mutations', async () => {
+    const renderingEl = document.createElement('div');
+    renderingEl.setAttribute(GROWI_IS_CONTENT_RENDERING_ATTR, 'true');
+    container.appendChild(renderingEl);
+
+    const cleanup = watchRenderingAndReScroll(container, scrollToTarget);
+
+    vi.advanceTimersByTime(3000);
+    expect(scrollToTarget).not.toHaveBeenCalled();
+
+    // Trigger a DOM mutation mid-timer
+    const child = document.createElement('span');
+    container.appendChild(child);
+    await vi.advanceTimersByTimeAsync(0);
+
+    // The timer should NOT have been reset — 2 more seconds should fire it
+    vi.advanceTimersByTime(2000);
+    expect(scrollToTarget).toHaveBeenCalledTimes(1);
+
+    cleanup();
+  });
+
+  it('should detect rendering elements added after initial check via observer', async () => {
+    const cleanup = watchRenderingAndReScroll(container, scrollToTarget);
+
+    vi.advanceTimersByTime(3000);
+    expect(scrollToTarget).not.toHaveBeenCalled();
+
+    // Add a rendering element later (within 10s timeout)
+    const renderingEl = document.createElement('div');
+    renderingEl.setAttribute(GROWI_IS_CONTENT_RENDERING_ATTR, 'true');
+    container.appendChild(renderingEl);
+
+    // Flush microtasks so MutationObserver callback fires
+    await vi.advanceTimersByTimeAsync(0);
+
+    // Timer should be scheduled — fires after 5s
+    await vi.advanceTimersByTimeAsync(5000);
+    expect(scrollToTarget).toHaveBeenCalledTimes(1);
+
+    cleanup();
+  });
+
+  it('should scroll once when multiple rendering elements exist simultaneously', () => {
+    const renderingEl1 = document.createElement('div');
+    renderingEl1.setAttribute(GROWI_IS_CONTENT_RENDERING_ATTR, 'true');
+    container.appendChild(renderingEl1);
+
+    const renderingEl2 = document.createElement('div');
+    renderingEl2.setAttribute(GROWI_IS_CONTENT_RENDERING_ATTR, 'true');
+    container.appendChild(renderingEl2);
+
+    const cleanup = watchRenderingAndReScroll(container, scrollToTarget);
+
+    vi.advanceTimersByTime(5000);
+    expect(scrollToTarget).toHaveBeenCalledTimes(1);
+
+    cleanup();
+  });
+
+  it('should stop watching after 10s timeout', () => {
+    const renderingEl = document.createElement('div');
+    renderingEl.setAttribute(GROWI_IS_CONTENT_RENDERING_ATTR, 'true');
+    container.appendChild(renderingEl);
+
+    const cleanup = watchRenderingAndReScroll(container, scrollToTarget);
+
+    // First scroll at 5s
+    vi.advanceTimersByTime(5000);
+    expect(scrollToTarget).toHaveBeenCalledTimes(1);
+
+    // At 10s both the scroll timer and the watch timeout fire.
+    vi.advanceTimersByTime(5000);
+    const callsAfter10s = scrollToTarget.mock.calls.length;
+
+    // After 10s, no further scrolls should occur regardless
+    vi.advanceTimersByTime(10000);
+    expect(scrollToTarget).toHaveBeenCalledTimes(callsAfter10s);
+
+    cleanup();
+  });
+
+  it('should clean up timer and observer on cleanup call', () => {
+    const renderingEl = document.createElement('div');
+    renderingEl.setAttribute(GROWI_IS_CONTENT_RENDERING_ATTR, 'true');
+    container.appendChild(renderingEl);
+
+    const cleanup = watchRenderingAndReScroll(container, scrollToTarget);
+
+    cleanup();
+
+    vi.advanceTimersByTime(10000);
+    expect(scrollToTarget).not.toHaveBeenCalled();
+  });
+
+  it('should prevent timer callbacks from executing after cleanup (stopped flag)', () => {
+    const renderingEl = document.createElement('div');
+    renderingEl.setAttribute(GROWI_IS_CONTENT_RENDERING_ATTR, 'true');
+    container.appendChild(renderingEl);
+
+    const cleanup = watchRenderingAndReScroll(container, scrollToTarget);
+
+    // Advance partway, then cleanup
+    vi.advanceTimersByTime(3000);
+    cleanup();
+
+    // Timer would have fired at 5s, but cleanup was called
+    vi.advanceTimersByTime(5000);
+    expect(scrollToTarget).not.toHaveBeenCalled();
+  });
+
+  it('should not schedule further re-scrolls after rendering elements complete', async () => {
+    const renderingEl = document.createElement('div');
+    renderingEl.setAttribute(GROWI_IS_CONTENT_RENDERING_ATTR, 'true');
+    container.appendChild(renderingEl);
+
+    const cleanup = watchRenderingAndReScroll(container, scrollToTarget);
+
+    // First timer fires at 5s — re-scroll executes
+    vi.advanceTimersByTime(5000);
+    expect(scrollToTarget).toHaveBeenCalledTimes(1);
+
+    // Rendering completes — attribute toggled to false
+    renderingEl.setAttribute(GROWI_IS_CONTENT_RENDERING_ATTR, 'false');
+    await vi.advanceTimersByTimeAsync(0);
+
+    // No further re-scrolls should be scheduled
+    vi.advanceTimersByTime(10000);
+    expect(scrollToTarget).toHaveBeenCalledTimes(1);
+
+    cleanup();
+  });
+
+  it('should scroll exactly once when rendering completes before the first timer fires', () => {
+    const renderingEl = document.createElement('div');
+    renderingEl.setAttribute(GROWI_IS_CONTENT_RENDERING_ATTR, 'true');
+    container.appendChild(renderingEl);
+
+    const cleanup = watchRenderingAndReScroll(container, scrollToTarget);
+
+    // Rendering completes before the first poll timer fires
+    renderingEl.setAttribute(GROWI_IS_CONTENT_RENDERING_ATTR, 'false');
+
+    // Poll timer fires at 5s — detects no rendering elements.
+    // wasRendering is reset in the timer callback BEFORE scrollToTarget so that
+    // the subsequent checkAndSchedule call does not trigger a redundant extra scroll.
+    vi.advanceTimersByTime(5000);
+    expect(scrollToTarget).toHaveBeenCalledTimes(1);
+
+    // No further scrolls after rendering is confirmed done
+    vi.advanceTimersByTime(5000);
+    expect(scrollToTarget).toHaveBeenCalledTimes(1);
+
+    cleanup();
+  });
+});

apps/app/src/client/hooks/use-content-auto-scroll/watch-rendering-and-rescroll.ts

@@ -0,0 +1,84 @@
+import {
+  GROWI_IS_CONTENT_RENDERING_ATTR,
+  GROWI_IS_CONTENT_RENDERING_SELECTOR,
+} from '@growi/core/dist/consts';
+
+const RENDERING_POLL_INTERVAL_MS = 5000;
+export const WATCH_TIMEOUT_MS = 10000;
+
+/**
+ * Watch for elements with in-progress rendering status in the container.
+ * Periodically calls scrollToTarget while rendering elements remain.
+ * Returns a cleanup function that stops observation and clears timers.
+ */
+export const watchRenderingAndReScroll = (
+  contentContainer: HTMLElement,
+  scrollToTarget: () => boolean,
+): (() => void) => {
+  let timerId: number | undefined;
+  let stopped = false;
+  let wasRendering = false;
+
+  const cleanup = () => {
+    stopped = true;
+    observer.disconnect();
+    if (timerId != null) {
+      window.clearTimeout(timerId);
+      timerId = undefined;
+    }
+    window.clearTimeout(watchTimeoutId);
+  };
+
+  const checkAndSchedule = () => {
+    if (stopped) return;
+
+    const hasRendering =
+      contentContainer.querySelector(GROWI_IS_CONTENT_RENDERING_SELECTOR) !=
+      null;
+
+    if (!hasRendering) {
+      if (timerId != null) {
+        window.clearTimeout(timerId);
+        timerId = undefined;
+      }
+      // Final re-scroll to compensate for the layout shift from the last completed render
+      if (wasRendering) {
+        wasRendering = false;
+        scrollToTarget();
+      }
+      return;
+    }
+
+    wasRendering = true;
+
+    // If a timer is already ticking, let it fire — don't reset
+    if (timerId != null) return;
+
+    timerId = window.setTimeout(() => {
+      if (stopped) return;
+      timerId = undefined;
+      // Reset before checkAndSchedule so the wasRendering guard does not
+      // trigger an extra re-scroll if rendering is already done by now.
+      wasRendering = false;
+      scrollToTarget();
+      checkAndSchedule();
+    }, RENDERING_POLL_INTERVAL_MS);
+  };
+
+  const observer = new MutationObserver(checkAndSchedule);
+
+  observer.observe(contentContainer, {
+    childList: true,
+    subtree: true,
+    attributes: true,
+    attributeFilter: [GROWI_IS_CONTENT_RENDERING_ATTR],
+  });
+
+  // Initial check
+  checkAndSchedule();
+
+  // Stop watching after timeout regardless of rendering state
+  const watchTimeoutId = window.setTimeout(cleanup, WATCH_TIMEOUT_MS);
+
+  return cleanup;
+};