2
0
Эх сурвалжийг харах

fix: implement auto-scroll functionality with resize observer and rendering completion handling

Shun Miyazawa 4 долоо хоног өмнө
parent
commit
5181e69f99

+ 4 - 94
apps/app/src/components/PageView/PageView.tsx

@@ -1,17 +1,5 @@
-import {
-  type JSX,
-  memo,
-  useCallback,
-  useEffect,
-  useId,
-  useMemo,
-  useRef,
-} from 'react';
+import { type JSX, memo, useCallback, useId, useMemo, useRef } from 'react';
 import dynamic from 'next/dynamic';
 import dynamic from 'next/dynamic';
-import {
-  GROWI_RENDERING_ATTR,
-  GROWI_RENDERING_ATTR_SELECTOR,
-} from '@growi/core/dist/consts';
 import { isDeepEquals } from '@growi/core/dist/utils/is-deep-equals';
 import { isDeepEquals } from '@growi/core/dist/utils/is-deep-equals';
 import { isUsersHomepage } from '@growi/core/dist/utils/page-path-utils';
 import { isUsersHomepage } from '@growi/core/dist/utils/page-path-utils';
 import { useSlidesByFrontmatter } from '@growi/presentation/dist/services';
 import { useSlidesByFrontmatter } from '@growi/presentation/dist/services';
@@ -35,6 +23,7 @@ import { PageAlerts } from './PageAlerts/PageAlerts';
 import { PageContentFooter } from './PageContentFooter';
 import { PageContentFooter } from './PageContentFooter';
 import { PageViewLayout } from './PageViewLayout';
 import { PageViewLayout } from './PageViewLayout';
 import RevisionRenderer from './RevisionRenderer';
 import RevisionRenderer from './RevisionRenderer';
+import { useHashAutoScroll } from './use-hash-auto-scroll';
 
 
 // biome-ignore-start lint/style/noRestrictedImports: no-problem dynamic import
 // biome-ignore-start lint/style/noRestrictedImports: no-problem dynamic import
 const NotCreatablePage = dynamic(
 const NotCreatablePage = dynamic(
@@ -131,87 +120,8 @@ const PageViewComponent = (props: Props): JSX.Element => {
     rendererConfig.isEnabledMarp,
     rendererConfig.isEnabledMarp,
   );
   );
 
 
-  // ***************************  Auto Scroll  ***************************
-  useEffect(() => {
-    if (currentPageId == null) {
-      return;
-    }
-
-    // do nothing if hash is empty
-    const { hash } = window.location;
-    if (hash.length === 0) {
-      return;
-    }
-
-    const contentContainer = document.getElementById(contentContainerId);
-    if (contentContainer == null) return;
-
-    const targetId = decodeURIComponent(hash.slice(1));
-
-    const scrollToTarget = (): boolean => {
-      const target = document.getElementById(targetId);
-      if (target == null) return false;
-      target.scrollIntoView();
-      return true;
-    };
-
-    // After the initial scroll, watch for data-growi-rendering elements to
-    // appear and then fully disappear. Components such as DrawioViewer may be
-    // mounted AFTER this effect runs (lazy / remark-plugin rendering), so we
-    // cannot just check synchronously — we must observe the full lifecycle.
-    const watchRenderingCompletion = () => {
-      let everSawRendering = false;
-
-      const renderingObserver = new MutationObserver(() => {
-        const hasRendering =
-          contentContainer.querySelector(GROWI_RENDERING_ATTR_SELECTOR) != null;
-
-        if (hasRendering) {
-          // At least one component is still rendering
-          everSawRendering = true;
-          return;
-        }
-
-        if (everSawRendering) {
-          // All rendering-in-progress elements have finished — scroll to the
-          // correct (post-render) position and stop observing
-          scrollToTarget();
-          renderingObserver.disconnect();
-        }
-      });
-
-      renderingObserver.observe(contentContainer, {
-        childList: true,
-        subtree: true,
-        attributes: true,
-        attributeFilter: [GROWI_RENDERING_ATTR],
-      });
-
-      return renderingObserver;
-    };
-
-    // Wait for the target heading element to appear in DOM first
-    if (!scrollToTarget()) {
-      const targetObserver = new MutationObserver(() => {
-        if (scrollToTarget()) {
-          targetObserver.disconnect();
-          watchRenderingCompletion();
-        }
-      });
-
-      targetObserver.observe(contentContainer, {
-        childList: true,
-        subtree: true,
-      });
-
-      return () => targetObserver.disconnect();
-    }
-
-    const renderingObserver = watchRenderingCompletion();
-    return () => renderingObserver.disconnect();
-  }, [currentPageId, contentContainerId]);
-
-  // *******************************  end  *******************************
+  // Auto-scroll to URL hash target, handling lazy-rendered content
+  useHashAutoScroll(currentPageId, contentContainerId);
 
 
   const specialContents = useMemo(() => {
   const specialContents = useMemo(() => {
     if (isIdenticalPathPage) {
     if (isIdenticalPathPage) {

+ 319 - 0
apps/app/src/components/PageView/use-hash-auto-scroll.spec.tsx

@@ -0,0 +1,319 @@
+import { GROWI_RENDERING_ATTR } from '@growi/core/dist/consts';
+import { renderHook } from '@testing-library/react';
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
+
+import {
+  useHashAutoScroll,
+  watchRenderingAndReScroll,
+} from './use-hash-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_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_RENDERING_ATTR, 'true');
+    container.appendChild(renderingEl);
+
+    const cleanup = watchRenderingAndReScroll(container, scrollToTarget);
+
+    // Advance 3 seconds
+    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_RENDERING_ATTR, 'true');
+    container.appendChild(renderingEl);
+
+    // Flush microtasks so MutationObserver callback fires
+    await vi.advanceTimersByTimeAsync(0);
+
+    // Timer should be scheduled — fires after 5s
+    vi.advanceTimersByTime(5000);
+    expect(scrollToTarget).toHaveBeenCalledTimes(1);
+
+    cleanup();
+  });
+
+  it('should clean up timer and observer on cleanup call', () => {
+    const renderingEl = document.createElement('div');
+    renderingEl.setAttribute(GROWI_RENDERING_ATTR, 'true');
+    container.appendChild(renderingEl);
+
+    const cleanup = watchRenderingAndReScroll(container, scrollToTarget);
+
+    cleanup();
+
+    vi.advanceTimersByTime(10000);
+    expect(scrollToTarget).not.toHaveBeenCalled();
+  });
+
+  it('should scroll once when multiple rendering elements exist simultaneously', () => {
+    // Two rendering elements present from the start (e.g. two DrawIO diagrams)
+    const renderingEl1 = document.createElement('div');
+    renderingEl1.setAttribute(GROWI_RENDERING_ATTR, 'true');
+    container.appendChild(renderingEl1);
+
+    const renderingEl2 = document.createElement('div');
+    renderingEl2.setAttribute(GROWI_RENDERING_ATTR, 'true');
+    container.appendChild(renderingEl2);
+
+    const cleanup = watchRenderingAndReScroll(container, scrollToTarget);
+
+    // Scroll fires once at 5s — not multiplied by element count
+    vi.advanceTimersByTime(5000);
+    expect(scrollToTarget).toHaveBeenCalledTimes(1);
+
+    cleanup();
+  });
+
+  it('should stop watching after 10s timeout', () => {
+    const renderingEl = document.createElement('div');
+    renderingEl.setAttribute(GROWI_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.
+    // The scroll may or may not execute depending on timer ordering.
+    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();
+  });
+});
+
+describe('useHashAutoScroll', () => {
+  const containerId = 'test-content-container';
+  let container: HTMLDivElement;
+
+  beforeEach(() => {
+    vi.useFakeTimers();
+    container = document.createElement('div');
+    container.id = containerId;
+    document.body.appendChild(container);
+  });
+
+  afterEach(() => {
+    vi.useRealTimers();
+    document.body.innerHTML = '';
+    window.location.hash = '';
+  });
+
+  it('should not scroll when pageId is null', () => {
+    window.location.hash = '#heading';
+    const target = document.createElement('div');
+    target.id = 'heading';
+    target.scrollIntoView = vi.fn();
+    container.appendChild(target);
+
+    renderHook(() => useHashAutoScroll(null, containerId));
+
+    expect(target.scrollIntoView).not.toHaveBeenCalled();
+  });
+
+  it('should not scroll when hash is empty', () => {
+    window.location.hash = '';
+    const target = document.createElement('div');
+    target.id = 'heading';
+    target.scrollIntoView = vi.fn();
+    container.appendChild(target);
+
+    renderHook(() => useHashAutoScroll('page-id', containerId));
+
+    expect(target.scrollIntoView).not.toHaveBeenCalled();
+  });
+
+  it('should not scroll when container is not found', () => {
+    window.location.hash = '#heading';
+    const target = document.createElement('div');
+    target.id = 'heading';
+    target.scrollIntoView = vi.fn();
+    container.appendChild(target);
+
+    renderHook(() => useHashAutoScroll('page-id', 'nonexistent-id'));
+
+    expect(target.scrollIntoView).not.toHaveBeenCalled();
+  });
+
+  it('should scroll to target when it already exists in DOM', () => {
+    window.location.hash = '#heading';
+    const target = document.createElement('div');
+    target.id = 'heading';
+    target.scrollIntoView = vi.fn();
+    container.appendChild(target);
+
+    const { unmount } = renderHook(() =>
+      useHashAutoScroll('page-id', containerId),
+    );
+
+    expect(target.scrollIntoView).toHaveBeenCalledTimes(1);
+
+    unmount();
+  });
+
+  it('should start rendering watch after scrolling to target', () => {
+    window.location.hash = '#heading';
+
+    // Target with a rendering element
+    const target = document.createElement('div');
+    target.id = 'heading';
+    target.scrollIntoView = vi.fn();
+    container.appendChild(target);
+
+    const renderingEl = document.createElement('div');
+    renderingEl.setAttribute(GROWI_RENDERING_ATTR, 'true');
+    container.appendChild(renderingEl);
+
+    const { unmount } = renderHook(() =>
+      useHashAutoScroll('page-id', containerId),
+    );
+
+    // Initial scroll
+    expect(target.scrollIntoView).toHaveBeenCalledTimes(1);
+
+    // Re-scroll after 5s due to rendering watch
+    vi.advanceTimersByTime(5000);
+    expect(target.scrollIntoView).toHaveBeenCalledTimes(2);
+
+    unmount();
+  });
+
+  it('should stop target observer after 10s timeout', async () => {
+    window.location.hash = '#never-appears';
+
+    const { unmount } = renderHook(() =>
+      useHashAutoScroll('page-id', containerId),
+    );
+
+    // Advance past the timeout
+    vi.advanceTimersByTime(11000);
+
+    // Target appears after timeout — should NOT trigger scroll
+    const target = document.createElement('div');
+    target.id = 'never-appears';
+    target.scrollIntoView = vi.fn();
+    container.appendChild(target);
+
+    await vi.advanceTimersByTimeAsync(0);
+
+    expect(target.scrollIntoView).not.toHaveBeenCalled();
+
+    unmount();
+  });
+
+  it('should clean up all observers and timers on unmount', () => {
+    window.location.hash = '#heading';
+    const target = document.createElement('div');
+    target.id = 'heading';
+    target.scrollIntoView = vi.fn();
+    container.appendChild(target);
+
+    const renderingEl = document.createElement('div');
+    renderingEl.setAttribute(GROWI_RENDERING_ATTR, 'true');
+    container.appendChild(renderingEl);
+
+    const { unmount } = renderHook(() =>
+      useHashAutoScroll('page-id', containerId),
+    );
+
+    // Initial scroll
+    expect(target.scrollIntoView).toHaveBeenCalledTimes(1);
+
+    // Unmount before poll timer fires
+    unmount();
+
+    // No further scrolls after unmount
+    vi.advanceTimersByTime(20000);
+    expect(target.scrollIntoView).toHaveBeenCalledTimes(1);
+  });
+
+  it('should re-run effect when pageId changes', () => {
+    window.location.hash = '#heading';
+    const target = document.createElement('div');
+    target.id = 'heading';
+    target.scrollIntoView = vi.fn();
+    container.appendChild(target);
+
+    const { rerender, unmount } = renderHook(
+      ({ pageId }) => useHashAutoScroll(pageId, containerId),
+      { initialProps: { pageId: 'page-1' as string | null } },
+    );
+
+    expect(target.scrollIntoView).toHaveBeenCalledTimes(1);
+
+    // Change pageId — effect re-runs
+    rerender({ pageId: 'page-2' });
+    expect(target.scrollIntoView).toHaveBeenCalledTimes(2);
+
+    // Set to null — no additional scroll
+    rerender({ pageId: null });
+    expect(target.scrollIntoView).toHaveBeenCalledTimes(2);
+
+    unmount();
+  });
+});

+ 125 - 0
apps/app/src/components/PageView/use-hash-auto-scroll.tsx

@@ -0,0 +1,125 @@
+import { useEffect } from 'react';
+import {
+  GROWI_RENDERING_ATTR,
+  GROWI_RENDERING_ATTR_SELECTOR,
+} from '@growi/core/dist/consts';
+
+const RENDERING_POLL_INTERVAL_MS = 5000;
+const WATCH_TIMEOUT_MS = 10000;
+
+/**
+ * Watch for `data-growi-rendering` elements in the container.
+ * While any exist, wait 5 seconds then re-scroll to the target.
+ * Repeats until no rendering elements remain.
+ * A MutationObserver re-triggers the check when new elements appear.
+ * Returns a cleanup function.
+ */
+export const watchRenderingAndReScroll = (
+  contentContainer: HTMLElement,
+  scrollToTarget: () => boolean,
+): (() => void) => {
+  let timerId: number | undefined;
+
+  const cleanup = () => {
+    observer.disconnect();
+    if (timerId != null) {
+      window.clearTimeout(timerId);
+    }
+    window.clearTimeout(watchTimeoutId);
+  };
+
+  const checkAndSchedule = () => {
+    // If a timer is already ticking, let it fire — don't reset.
+    // Resetting on every DOM mutation would prevent the scroll
+    // from ever executing when rendering completes quickly.
+    if (timerId != null) return;
+
+    const hasRendering =
+      contentContainer.querySelector(GROWI_RENDERING_ATTR_SELECTOR) != null;
+
+    if (hasRendering) {
+      timerId = window.setTimeout(() => {
+        timerId = undefined;
+        scrollToTarget();
+        checkAndSchedule();
+      }, RENDERING_POLL_INTERVAL_MS);
+    }
+  };
+
+  const observer = new MutationObserver(checkAndSchedule);
+
+  observer.observe(contentContainer, {
+    childList: true,
+    subtree: true,
+    attributes: true,
+    attributeFilter: [GROWI_RENDERING_ATTR],
+  });
+
+  // Initial check
+  checkAndSchedule();
+
+  // Stop watching after timeout regardless of rendering state
+  const watchTimeoutId = window.setTimeout(cleanup, WATCH_TIMEOUT_MS);
+
+  return cleanup;
+};
+
+/**
+ * Auto-scroll to the URL hash target when the page loads.
+ * Handles lazy-rendered content by polling for `data-growi-rendering`
+ * elements and re-scrolling after they finish.
+ */
+export const useHashAutoScroll = (
+  pageId: string | undefined | null,
+  contentContainerId: string,
+): void => {
+  useEffect(() => {
+    if (pageId == null) return;
+
+    const { hash } = window.location;
+    if (hash.length === 0) return;
+
+    const contentContainer = document.getElementById(contentContainerId);
+    if (contentContainer == null) return;
+
+    const targetId = decodeURIComponent(hash.slice(1));
+
+    const scrollToTarget = (): boolean => {
+      const target = document.getElementById(targetId);
+      if (target == null) return false;
+      target.scrollIntoView();
+      return true;
+    };
+
+    // Target already in DOM — scroll and watch rendering
+    if (scrollToTarget()) {
+      return watchRenderingAndReScroll(contentContainer, scrollToTarget);
+    }
+
+    // Target not in DOM yet — wait for it, then watch rendering
+    let renderingCleanup: (() => void) | undefined;
+
+    const observer = new MutationObserver(() => {
+      if (scrollToTarget()) {
+        observer.disconnect();
+        window.clearTimeout(timeoutId);
+        renderingCleanup = watchRenderingAndReScroll(
+          contentContainer,
+          scrollToTarget,
+        );
+      }
+    });
+
+    observer.observe(contentContainer, { childList: true, subtree: true });
+    const timeoutId = window.setTimeout(
+      () => observer.disconnect(),
+      WATCH_TIMEOUT_MS,
+    );
+
+    return () => {
+      observer.disconnect();
+      window.clearTimeout(timeoutId);
+      renderingCleanup?.();
+    };
+  }, [pageId, contentContainerId]);
+};