fix: refactor SearchResultContent to integrate watchRenderingAndReScroll for keyword scrolling

.kiro/specs/auto-scroll/research.md

@@ -99,6 +99,85 @@
 - **Risk**: Renaming the attribute requires coordinated changes across `@growi/core`, `remark-drawio`, and consuming apps — **Mitigation**: Constants are centralized; single constant rename propagates via imports
 - **Risk**: MutationObserver on `subtree: true` may be expensive on large pages — **Mitigation**: Retained 10s maximum watch timeout from current implementation
 
+## Post-Implementation Finding: SearchResultContent Integration Misalignment
+
+**Discovered after task 6 implementation** during code review conversation.
+
+### Problem
+
+The task 6 implementation added `useContentAutoScroll` to `SearchResultContent`, but this was architecturally incorrect. `useContentAutoScroll` is URL-hash–driven (`if (hash.length === 0) return`) and will never activate in the search results context — the search page URL (`/search?q=foo`) carries no fragment identifier.
+
+### Actual Requirement
+
+The real requirement for SearchResultContent is:
+1. **Keyword scroll** (already working): scroll to the first `.highlighted-keyword` element when content loads, via MutationObserver + 500ms debounce
+2. **Re-scroll after rendering** (missing): when drawio / mermaid diagrams render asynchronously after the initial keyword scroll, the layout shifts and the keyword moves out of view — `watchRenderingAndReScroll` should re-scroll to the keyword once rendering settles
+
+### Current Code State (as of this writing)
+
+`apps/app/src/features/search/client/components/SearchPage/SearchResultContent.tsx` contains:
+- `useContentAutoScroll(...)` call — **should be removed**
+- keyword scroll `useEffect` with hash guard (`if (window.location.hash.length > 0) return`) — the guard may also be removable depending on how the hook is refactored
+- `scrollToTargetWithinContainer` local helper (shared distance calculation) — **keep**
+
+### Proposed Refactoring Direction
+
+Two-phase refactor, designed for the next session:
+
+**Phase 1 — Immediate fix (SearchResultContent)**
+
+Wire `watchRenderingAndReScroll` directly into the keyword scroll `useEffect`:
+
+```typescript
+useEffect(() => {
+  const scrollElement = scrollElementRef.current;
+  if (scrollElement == null) return;
+
+  const scrollToKeyword = (): boolean => {
+    const toElem = scrollElement.querySelector('.highlighted-keyword') as HTMLElement | null;
+    if (toElem == null) return false;
+    scrollToTargetWithinContainer(toElem, scrollElement);
+    return true;
+  };
+
+  // MutationObserver for incremental content loading (debounced)
+  const observer = new MutationObserver(() => {
+    scrollToFirstHighlightedKeywordDebounced(scrollElement);
+  });
+  observer.observe(scrollElement, MUTATION_OBSERVER_CONFIG);
+
+  // Rendering watch: re-scroll after drawio/mermaid layout shifts
+  const cleanupWatch = watchRenderingAndReScroll(scrollElement, scrollToKeyword);
+  return cleanupWatch;
+}, [page._id]);
+```
+
+Remove the `useContentAutoScroll` import and call entirely.
+
+**Phase 2 — Architecture improvement (shared hook)**
+
+Reorganize the relationship between `useContentAutoScroll` and `watchRenderingAndReScroll`:
+
+- `watchRenderingAndReScroll` (pure function) is the core shared primitive — **promote it to a named export** so callers other than `useContentAutoScroll` can use it directly
+- Consider introducing a thin React wrapper hook `useRenderingRescroll(scrollToTarget, deps)` that manages the `useEffect` lifecycle for `watchRenderingAndReScroll`, making it composable
+- `useContentAutoScroll` becomes the **hash-navigation–specific** hook: hash guard → target resolution → initial scroll → delegates to `useRenderingRescroll`
+- `SearchResultContent` keyword scroll becomes: MO-debounce → initial scroll → delegates to `useRenderingRescroll`
+- PageView-specific logic (default `scrollIntoView`, `getElementById` resolver) stays in PageView or in `useContentAutoScroll`
+
+Resulting dependency graph:
+
+```
+useContentAutoScroll  ─┐
+                        ├── useRenderingRescroll ── watchRenderingAndReScroll
+SearchResultContent   ─┘
+```
+
+### Key Questions for Next Session Design
+
+1. Should `useRenderingRescroll` be a hook (managing `useEffect` internally) or should callers be responsible for calling it inside their own effect? A hook is more ergonomic; a plain function is more flexible.
+2. The current keyword-scroll `useEffect` has no dependency array (fires every render) and no cleanup — intentional per inline comment. Adding `[page._id]` deps and a cleanup changes this behavior. Is that safe?
+3. Should the hash guard on the keyword-scroll `useEffect` be removed once `useContentAutoScroll` is also removed from `SearchResultContent`?
+
 ## References
 - [MutationObserver API](https://developer.mozilla.org/en-US/docs/Web/API/MutationObserver) — core browser API used for DOM observation
 - [Element.scrollIntoView()](https://developer.mozilla.org/en-US/docs/Web/API/Element/scrollIntoView) — default scroll behavior

.kiro/specs/auto-scroll/spec.json

@@ -15,7 +15,8 @@
     },
     "tasks": {
       "generated": true,
-      "approved": false
+      "approved": false,
+      "notes": "Tasks 1–7 implemented. Task 8 is Phase 2 architecture refactoring — requires design review before implementation. See research.md § Post-Implementation Finding."
     }
   },
   "ready_for_implementation": false

.kiro/specs/auto-scroll/tasks.md

@@ -101,3 +101,39 @@
   - Test that the keyword-scroll `useEffect` skips observation when `window.location.hash` is non-empty
   - Test that the keyword-scroll `useEffect` sets up the MutationObserver normally when no hash is present
   - _Requirements: 5.1, 5.2, 5.3, 5.5_
+
+---
+
+## Phase 2: Architecture Refactoring (Next Session)
+
+> **Context**: Tasks 6.1–6.3 were implemented but are architecturally incorrect. `useContentAutoScroll` is URL-hash–driven and never activates on the search page (`/search?q=foo`). The actual need is to re-scroll to `.highlighted-keyword` after async renderers (drawio, mermaid) cause layout shifts. See `research.md` § "Post-Implementation Finding" for full analysis.
+>
+> **Recommended model**: Opus — this phase requires design decisions on hook decomposition.
+
+- [x] 7. Fix SearchResultContent: replace `useContentAutoScroll` with `watchRenderingAndReScroll`
+- [x] 7.1 Wire `watchRenderingAndReScroll` into keyword-scroll effect
+  - Remove `useContentAutoScroll` import and call from `SearchResultContent.tsx`
+  - Import `watchRenderingAndReScroll` (already exported from `watch-rendering-and-rescroll.ts`)
+  - Inside the keyword-scroll `useEffect`, after setting up the MutationObserver, call `watchRenderingAndReScroll(scrollElement, scrollToKeyword)` where `scrollToKeyword` calls `scrollToTargetWithinContainer` on the first `.highlighted-keyword` element
+  - Add `[page._id]` to the dependency array (currently has no deps) and return the watch cleanup function
+  - Remove the hash guard (`if (window.location.hash.length > 0) return`) — no longer needed once `useContentAutoScroll` is removed
+  - _See research.md for proposed code sketch_
+
+- [x] 7.2 Update SearchResultContent tests
+  - Remove tests that assert `useContentAutoScroll` is called
+  - Add tests that `watchRenderingAndReScroll` re-scrolls to `.highlighted-keyword` after a rendering element settles
+  - Update MutationObserver suppression test: remove the hash-guard test (guard will be gone)
+
+- [ ] 8. (Design required) Extract shared `useRenderingRescroll` hook
+  - **Design this task in the next session before implementing**
+  - Goal: provide a reusable React hook that wraps `watchRenderingAndReScroll` with `useEffect` lifecycle management, composable by both `useContentAutoScroll` (hash path) and `SearchResultContent` (keyword path)
+  - Key design questions to resolve (see research.md):
+    1. Hook vs. plain function — who owns the `useEffect`?
+    2. Is adding `[page._id]` deps + cleanup to the keyword-scroll effect safe given the original intentional no-cleanup design?
+    3. Should the hash guard on keyword-scroll be removed entirely?
+  - Proposed dependency graph after refactor:
+    ```
+    useContentAutoScroll  ─┐
+                            ├── useRenderingRescroll ── watchRenderingAndReScroll
+    SearchResultContent   ─┘
+    ```

apps/app/src/features/search/client/components/SearchPage/SearchResultContent.spec.tsx

@@ -0,0 +1,201 @@
+import type { IPageHasId } from '@growi/core';
+import { render } from '@testing-library/react';
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
+
+import type { IPageWithSearchMeta } from '~/interfaces/search';
+
+// Mock watchRenderingAndReScroll
+vi.mock(
+  '~/client/hooks/use-content-auto-scroll/watch-rendering-and-rescroll',
+  () => ({
+    watchRenderingAndReScroll: vi.fn(() => vi.fn()), // returns a cleanup fn
+  }),
+);
+
+// Mock scrollWithinContainer
+vi.mock('~/client/util/smooth-scroll', () => ({
+  scrollWithinContainer: vi.fn(),
+}));
+
+// Mock next/dynamic
+vi.mock('next/dynamic', () => ({
+  default: () => {
+    return function MockDynamic() {
+      return null;
+    };
+  },
+}));
+
+// Mock various hooks and stores used by SearchResultContent
+vi.mock('~/services/layout/use-should-expand-content', () => ({
+  useShouldExpandContent: vi.fn(() => false),
+}));
+vi.mock('~/states/global', () => ({
+  useCurrentUser: vi.fn(() => null),
+}));
+vi.mock('~/states/ui/modal/page-delete', () => ({
+  usePageDeleteModalActions: vi.fn(() => ({ open: vi.fn() })),
+}));
+vi.mock('~/states/ui/modal/page-duplicate', () => ({
+  usePageDuplicateModalActions: vi.fn(() => ({ open: vi.fn() })),
+}));
+vi.mock('~/states/ui/modal/page-rename', () => ({
+  usePageRenameModalActions: vi.fn(() => ({ open: vi.fn() })),
+}));
+vi.mock('~/stores/page-listing', () => ({
+  mutatePageList: vi.fn(),
+  mutatePageTree: vi.fn(),
+  mutateRecentlyUpdated: vi.fn(),
+}));
+vi.mock('~/stores/renderer', () => ({
+  useSearchResultOptions: vi.fn(() => ({ data: null })),
+}));
+vi.mock('~/stores/search', () => ({
+  mutateSearching: vi.fn(),
+}));
+vi.mock('next-i18next', () => ({
+  useTranslation: vi.fn(() => ({ t: (key: string) => key })),
+}));
+vi.mock('~/components/Common/PagePathNav', () => ({
+  PagePathNav: () => null,
+}));
+
+import { watchRenderingAndReScroll } from '~/client/hooks/use-content-auto-scroll/watch-rendering-and-rescroll';
+import { scrollWithinContainer } from '~/client/util/smooth-scroll';
+
+import { SearchResultContent } from './SearchResultContent';
+
+const mockWatchRenderingAndReScroll = vi.mocked(watchRenderingAndReScroll);
+const mockScrollWithinContainer = vi.mocked(scrollWithinContainer);
+
+const createMockPage = (overrides: Partial<IPageHasId> = {}): IPageHasId =>
+  ({
+    _id: 'page-123',
+    path: '/test/page',
+    revision: 'rev-456',
+    wip: false,
+    ...overrides,
+  }) as unknown as IPageHasId;
+
+const createMockPageWithMeta = (page: IPageHasId = createMockPage()) =>
+  ({ data: page }) as unknown as IPageWithSearchMeta;
+
+describe('SearchResultContent', () => {
+  beforeEach(() => {
+    mockWatchRenderingAndReScroll.mockReset();
+    mockWatchRenderingAndReScroll.mockReturnValue(vi.fn());
+    mockScrollWithinContainer.mockReset();
+    window.location.hash = '';
+  });
+
+  afterEach(() => {
+    window.location.hash = '';
+  });
+
+  describe('watchRenderingAndReScroll integration', () => {
+    it('should call watchRenderingAndReScroll with the scroll container element', () => {
+      const page = createMockPage({ _id: 'page-123' });
+      const pageWithMeta = createMockPageWithMeta(page);
+
+      render(<SearchResultContent pageWithMeta={pageWithMeta} />);
+
+      expect(mockWatchRenderingAndReScroll).toHaveBeenCalledTimes(1);
+      const firstCall = mockWatchRenderingAndReScroll.mock.calls[0];
+      expect(firstCall).toBeDefined();
+      const containerArg = firstCall?.[0];
+      expect(containerArg).toBeInstanceOf(HTMLElement);
+      expect((containerArg as HTMLElement).id).toBe(
+        'search-result-content-body-container',
+      );
+    });
+
+    it('should pass a scrollToKeyword function as the second argument', () => {
+      const page = createMockPage();
+      const pageWithMeta = createMockPageWithMeta(page);
+
+      render(<SearchResultContent pageWithMeta={pageWithMeta} />);
+
+      const scrollToKeyword = mockWatchRenderingAndReScroll.mock.calls[0]?.[1];
+      expect(typeof scrollToKeyword).toBe('function');
+    });
+
+    it('scrollToKeyword should scroll to .highlighted-keyword within container', () => {
+      const page = createMockPage();
+      const pageWithMeta = createMockPageWithMeta(page);
+
+      render(<SearchResultContent pageWithMeta={pageWithMeta} />);
+
+      const firstCall = mockWatchRenderingAndReScroll.mock.calls[0];
+      expect(firstCall).toBeDefined();
+      const container = firstCall?.[0] as HTMLElement;
+      const scrollToKeyword = firstCall?.[1];
+
+      // Inject a highlighted-keyword element into the container
+      const keyword = document.createElement('span');
+      keyword.className = 'highlighted-keyword';
+      container.appendChild(keyword);
+
+      vi.spyOn(keyword, 'getBoundingClientRect').mockReturnValue({
+        top: 250,
+        bottom: 270,
+        left: 0,
+        right: 100,
+        width: 100,
+        height: 20,
+        x: 0,
+        y: 250,
+        toJSON: () => ({}),
+      });
+      vi.spyOn(container, 'getBoundingClientRect').mockReturnValue({
+        top: 100,
+        bottom: 600,
+        left: 0,
+        right: 100,
+        width: 100,
+        height: 500,
+        x: 0,
+        y: 100,
+        toJSON: () => ({}),
+      });
+
+      const result = scrollToKeyword();
+
+      // distance = 250 - 100 - 30 = 120
+      expect(mockScrollWithinContainer).toHaveBeenCalledWith(container, 120);
+      expect(result).toBe(true);
+
+      container.removeChild(keyword);
+    });
+
+    it('scrollToKeyword should return false when no .highlighted-keyword element exists', () => {
+      const page = createMockPage();
+      const pageWithMeta = createMockPageWithMeta(page);
+
+      render(<SearchResultContent pageWithMeta={pageWithMeta} />);
+
+      const scrollToKeyword = mockWatchRenderingAndReScroll.mock.calls[0]?.[1];
+      expect(scrollToKeyword).toBeDefined();
+
+      const result = scrollToKeyword?.();
+
+      expect(result).toBe(false);
+      expect(mockScrollWithinContainer).not.toHaveBeenCalled();
+    });
+
+    it('should call watchRenderingAndReScroll cleanup when component unmounts', () => {
+      const mockCleanup = vi.fn();
+      mockWatchRenderingAndReScroll.mockReturnValue(mockCleanup);
+
+      const page = createMockPage();
+      const pageWithMeta = createMockPageWithMeta(page);
+
+      const { unmount } = render(
+        <SearchResultContent pageWithMeta={pageWithMeta} />,
+      );
+
+      unmount();
+
+      expect(mockCleanup).toHaveBeenCalledTimes(1);
+    });
+  });
+});

apps/app/src/features/search/client/components/SearchPage/SearchResultContent.tsx

@@ -12,6 +12,7 @@ import type {
   ForceHideMenuItems,
 } from '~/client/components/Common/Dropdown/PageItemControl';
 import type { RevisionLoaderProps } from '~/client/components/Page/RevisionLoader';
+import { watchRenderingAndReScroll } from '~/client/hooks/use-content-auto-scroll/watch-rendering-and-rescroll';
 import { exportAsMarkdown } from '~/client/services/page-operation';
 import { scrollWithinContainer } from '~/client/util/smooth-scroll';
 import { toastSuccess } from '~/client/util/toastr';
@@ -99,20 +100,24 @@ type Props = {
   forceHideMenuItems?: ForceHideMenuItems;
 };
 
+const scrollToTargetWithinContainer = (
+  target: HTMLElement,
+  container: HTMLElement,
+): void => {
+  const distance =
+    target.getBoundingClientRect().top -
+    container.getBoundingClientRect().top -
+    SCROLL_OFFSET_TOP;
+  scrollWithinContainer(container, distance);
+};
+
 const scrollToFirstHighlightedKeyword = (scrollElement: HTMLElement): void => {
   // use querySelector to intentionally get the first element found
   const toElem = scrollElement.querySelector(
     '.highlighted-keyword',
   ) as HTMLElement | null;
-  if (toElem == null) {
-    return;
-  }
-
-  const distance =
-    toElem.getBoundingClientRect().top -
-    scrollElement.getBoundingClientRect().top -
-    SCROLL_OFFSET_TOP;
-  scrollWithinContainer(scrollElement, distance);
+  if (toElem == null) return;
+  scrollToTargetWithinContainer(toElem, scrollElement);
 };
 const scrollToFirstHighlightedKeywordDebounced = debounce(
   500,
@@ -122,34 +127,43 @@ const scrollToFirstHighlightedKeywordDebounced = debounce(
 export const SearchResultContent: FC<Props> = (props: Props) => {
   const scrollElementRef = useRef<HTMLDivElement | null>(null);
 
-  // ***************************  Auto Scroll  ***************************
+  const { pageWithMeta } = props;
+  const page = pageWithMeta.data;
+
+  // ***************************  Keyword Scroll  ***************************
+  // biome-ignore lint/correctness/useExhaustiveDependencies: page._id is a trigger dep: re-run this effect when the selected page changes
   useEffect(() => {
     const scrollElement = scrollElementRef.current;
 
     if (scrollElement == null) return;
 
+    const scrollToKeyword = (): boolean => {
+      const toElem = scrollElement.querySelector(
+        '.highlighted-keyword',
+      ) as HTMLElement | null;
+      if (toElem == null) return false;
+      scrollToTargetWithinContainer(toElem, scrollElement);
+      return true;
+    };
+
     const observer = new MutationObserver(() => {
       scrollToFirstHighlightedKeywordDebounced(scrollElement);
     });
     observer.observe(scrollElement, MUTATION_OBSERVER_CONFIG);
 
-    // no cleanup function -- 2023.07.31 Yuki Takei
-    // see: https://developer.mozilla.org/en-US/docs/Web/API/MutationObserver/observe
-    // > You can call observe() multiple times on the same MutationObserver
-    // > to watch for changes to different parts of the DOM tree and/or different types of changes.
-  });
+    // Re-scroll to keyword after async renderers (drawio/mermaid) cause layout shifts
+    const cleanupWatch = watchRenderingAndReScroll(
+      scrollElement,
+      scrollToKeyword,
+    );
+    return cleanupWatch;
+  }, [page._id]);
   // *******************************  end  *******************************
 
-  const {
-    pageWithMeta,
-    highlightKeywords,
-    showPageControlDropdown,
-    forceHideMenuItems,
-  } = props;
+  const { highlightKeywords, showPageControlDropdown, forceHideMenuItems } =
+    props;
 
   const { t } = useTranslation();
-
-  const page = pageWithMeta.data;
   const { open: openDuplicateModal } = usePageDuplicateModalActions();
   const { open: openRenameModal } = usePageRenameModalActions();
   const { open: openDeleteModal } = usePageDeleteModalActions();