|
@@ -81,6 +81,8 @@ graph TB
|
|
|
- Existing patterns preserved: MutationObserver + polling hybrid, timeout-based safety bounds
|
|
- Existing patterns preserved: MutationObserver + polling hybrid, timeout-based safety bounds
|
|
|
- Steering compliance: Named exports, immutable patterns, co-located tests
|
|
- Steering compliance: Named exports, immutable patterns, co-located tests
|
|
|
|
|
|
|
|
|
|
+**Co-location rationale**: `watchRenderingAndReScroll` lives in `src/client/util/` (not `hooks/`) because it is a plain function, not a React hook — co-located with `smooth-scroll.ts` as both are DOM scroll utilities. `useHashAutoScroll` lives next to `PageView.tsx` because it is hash-navigation–specific (`window.location.hash`) and PageView is its only consumer. `useKeywordRescroll` lives next to `SearchResultContent.tsx` for the same reason. The old `src/client/hooks/use-content-auto-scroll/` shared directory was removed because the hook was never truly shared — only the underlying utility function was.
|
|
|
|
|
+
|
|
|
### Technology Stack
|
|
### Technology Stack
|
|
|
|
|
|
|
|
| Layer | Choice / Version | Role in Feature | Notes |
|
|
| Layer | Choice / Version | Role in Feature | Notes |
|
|
@@ -448,119 +450,6 @@ useKeywordRescroll({ scrollElementRef, key: page._id });
|
|
|
|
|
|
|
|
---
|
|
---
|
|
|
|
|
|
|
|
-## Task 8 Design: Module Reorganization
|
|
|
|
|
-
|
|
|
|
|
-### Background
|
|
|
|
|
-
|
|
|
|
|
-After tasks 1–7, the auto-scroll modules are functional but organizationally misaligned:
|
|
|
|
|
-
|
|
|
|
|
-- `useContentAutoScroll` lives in a shared hooks directory (`src/client/hooks/use-content-auto-scroll/`) but is effectively PageView-specific — it depends on `window.location.hash` and only PageView uses it
|
|
|
|
|
-- `watchRenderingAndReScroll` lives alongside that hook as if it were hook-internal, but it is a plain function and the actual shared primitive used by both PageView and SearchResultContent
|
|
|
|
|
-- SearchResultContent's keyword-scroll + rendering-watch logic is inlined in the component, not extracted into a testable hook
|
|
|
|
|
-
|
|
|
|
|
-### Goals
|
|
|
|
|
-
|
|
|
|
|
-- Place each module where its consumer lives (co-location)
|
|
|
|
|
-- Move the only truly shared primitive (`watchRenderingAndReScroll`) to the shared utility directory
|
|
|
|
|
-- Remove the `src/client/hooks/use-content-auto-scroll/` directory — it no longer represents a shared hook
|
|
|
|
|
-
|
|
|
|
|
-### Non-Goals
|
|
|
|
|
-
|
|
|
|
|
-- Changing any scroll behavior or logic — this is a pure reorganization
|
|
|
|
|
-- Simplifying `useContentAutoScroll`'s generic options (`resolveTarget`, `scrollTo`) — retained for future extensibility
|
|
|
|
|
-
|
|
|
|
|
-### Architecture After Reorganization
|
|
|
|
|
-
|
|
|
|
|
-```mermaid
|
|
|
|
|
-graph TB
|
|
|
|
|
- subgraph shared_util[src/client/util]
|
|
|
|
|
- WRRS[watch-rendering-and-rescroll.ts]
|
|
|
|
|
- end
|
|
|
|
|
-
|
|
|
|
|
- subgraph page_view[src/components/PageView]
|
|
|
|
|
- UHAS[use-hash-auto-scroll.ts]
|
|
|
|
|
- PV[PageView.tsx]
|
|
|
|
|
- end
|
|
|
|
|
-
|
|
|
|
|
- subgraph search[features/search/.../SearchPage]
|
|
|
|
|
- UKR[use-keyword-rescroll.ts]
|
|
|
|
|
- SRC[SearchResultContent.tsx]
|
|
|
|
|
- end
|
|
|
|
|
-
|
|
|
|
|
- PV -->|calls| UHAS
|
|
|
|
|
- UHAS -->|imports| WRRS
|
|
|
|
|
- SRC -->|calls| UKR
|
|
|
|
|
- UKR -->|imports| WRRS
|
|
|
|
|
-```
|
|
|
|
|
-
|
|
|
|
|
-### File Moves
|
|
|
|
|
-
|
|
|
|
|
-| Before | After | Change |
|
|
|
|
|
-|--------|-------|--------|
|
|
|
|
|
-| `src/client/hooks/use-content-auto-scroll/watch-rendering-and-rescroll.ts` | `src/client/util/watch-rendering-and-rescroll.ts` | Move to shared util (not a hook) |
|
|
|
|
|
-| `src/client/hooks/use-content-auto-scroll/watch-rendering-and-rescroll.spec.tsx` | `src/client/util/watch-rendering-and-rescroll.spec.tsx` | Co-locate test with source |
|
|
|
|
|
-| `src/client/hooks/use-content-auto-scroll/use-content-auto-scroll.ts` | `src/components/PageView/use-hash-auto-scroll.ts` | Rename + move next to consumer |
|
|
|
|
|
-| `src/client/hooks/use-content-auto-scroll/use-content-auto-scroll.spec.tsx` | `src/components/PageView/use-hash-auto-scroll.spec.tsx` | Co-locate test with source |
|
|
|
|
|
-| SearchResultContent.tsx (inline keyword-scroll effect) | `features/search/.../SearchPage/use-keyword-rescroll.ts` | Extract into co-located hook |
|
|
|
|
|
-| (new) | `features/search/.../SearchPage/use-keyword-rescroll.spec.tsx` | New test file for extracted hook |
|
|
|
|
|
-| `src/client/hooks/use-content-auto-scroll/` | (deleted) | Directory removed after all files moved |
|
|
|
|
|
-
|
|
|
|
|
-### Component Details
|
|
|
|
|
-
|
|
|
|
|
-#### watch-rendering-and-rescroll (move only)
|
|
|
|
|
-
|
|
|
|
|
-No code changes. Moved from `src/client/hooks/use-content-auto-scroll/` to `src/client/util/` because it is a plain function, not a React hook. Co-located with `smooth-scroll.ts` — both are DOM scroll utilities.
|
|
|
|
|
-
|
|
|
|
|
-#### use-hash-auto-scroll (rename + move)
|
|
|
|
|
-
|
|
|
|
|
-Renamed from `useContentAutoScroll` to `useHashAutoScroll`. The name reflects that this hook is hash-navigation–driven (`window.location.hash`). Logic is unchanged — the two-phase flow (target resolution → rendering watch) and generic options (`resolveTarget`, `scrollTo`) are preserved.
|
|
|
|
|
-
|
|
|
|
|
-- **Export**: `useHashAutoScroll` (named export), `UseHashAutoScrollOptions` (type export)
|
|
|
|
|
-- **Import update**: `watchRenderingAndReScroll` imported from `~/client/util/watch-rendering-and-rescroll`
|
|
|
|
|
-- **Consumer**: `PageView.tsx` updates its import path and function name
|
|
|
|
|
-
|
|
|
|
|
-#### use-keyword-rescroll (new — extracted from SearchResultContent)
|
|
|
|
|
-
|
|
|
|
|
-Extracts the keyword-scroll `useEffect` from `SearchResultContent.tsx` (lines 133–161) into a standalone hook.
|
|
|
|
|
-
|
|
|
|
|
-```typescript
|
|
|
|
|
-interface UseKeywordRescrollOptions {
|
|
|
|
|
- /** Ref to the scrollable container element */
|
|
|
|
|
- scrollElementRef: RefObject<HTMLElement | null>;
|
|
|
|
|
- /** Unique key that triggers re-execution (typically page._id) */
|
|
|
|
|
- key: string;
|
|
|
|
|
-}
|
|
|
|
|
-
|
|
|
|
|
-function useKeywordRescroll(options: UseKeywordRescrollOptions): void;
|
|
|
|
|
-```
|
|
|
|
|
-
|
|
|
|
|
-**Responsibilities:**
|
|
|
|
|
-- MutationObserver on container for keyword highlight detection (debounced 500ms)
|
|
|
|
|
-- `watchRenderingAndReScroll` integration for async renderer compensation
|
|
|
|
|
-- Cleanup of both MO and rendering watch on key change or unmount
|
|
|
|
|
-
|
|
|
|
|
-**Import**: `watchRenderingAndReScroll` from `~/client/util/watch-rendering-and-rescroll`
|
|
|
|
|
-
|
|
|
|
|
-`scrollToTargetWithinContainer` and `scrollToFirstHighlightedKeyword` helper functions move into the hook file (or stay in SearchResultContent if other code in the component also uses them).
|
|
|
|
|
-
|
|
|
|
|
-### Import Update Summary
|
|
|
|
|
-
|
|
|
|
|
-| File | Old Import | New Import |
|
|
|
|
|
-|------|-----------|------------|
|
|
|
|
|
-| `PageView.tsx` | `~/client/hooks/use-content-auto-scroll/use-content-auto-scroll` | `./use-hash-auto-scroll` |
|
|
|
|
|
-| `use-hash-auto-scroll.ts` | `./watch-rendering-and-rescroll` | `~/client/util/watch-rendering-and-rescroll` |
|
|
|
|
|
-| `use-keyword-rescroll.ts` | (inline code) | `~/client/util/watch-rendering-and-rescroll` |
|
|
|
|
|
-| `SearchResultContent.tsx` | `~/client/hooks/use-content-auto-scroll/watch-rendering-and-rescroll` | `./use-keyword-rescroll` |
|
|
|
|
|
-
|
|
|
|
|
-### Testing Strategy
|
|
|
|
|
-
|
|
|
|
|
-- **`watch-rendering-and-rescroll.spec.tsx`**: Move as-is; no test changes (same logic, new path)
|
|
|
|
|
-- **`use-hash-auto-scroll.spec.tsx`**: Move + rename imports; no test logic changes
|
|
|
|
|
-- **`use-keyword-rescroll.spec.tsx`**: Migrate relevant tests from `SearchResultContent.spec.tsx` (rendering watch assertions) and add hook-level tests for keyword scroll behavior
|
|
|
|
|
-- **`SearchResultContent.spec.tsx`**: Simplify — component tests verify that `useKeywordRescroll` is called with correct props; detailed scroll behavior tested in the hook's own spec
|
|
|
|
|
-
|
|
|
|
|
----
|
|
|
|
|
-
|
|
|
|
|
## Error Handling
|
|
## Error Handling
|
|
|
|
|
|
|
|
### Error Strategy
|
|
### Error Strategy
|
|
@@ -575,55 +464,3 @@ This feature operates entirely in the browser DOM layer with no server interacti
|
|
|
|
|
|
|
|
**Rendering Watch Timeout** (3.6): After 10s, all observers and timers are cleaned up regardless of remaining rendering elements. This prevents resource leaks from components that fail to signal completion.
|
|
**Rendering Watch Timeout** (3.6): After 10s, all observers and timers are cleaned up regardless of remaining rendering elements. This prevents resource leaks from components that fail to signal completion.
|
|
|
|
|
|
|
|
-## Testing Strategy
|
|
|
|
|
-
|
|
|
|
|
-### useHashAutoScroll Tests (co-located in `src/components/PageView/`)
|
|
|
|
|
-
|
|
|
|
|
-1. **Guard conditions**: Verify no-op when key is null, hash is empty, or container not found (1.3–1.5)
|
|
|
|
|
-2. **Immediate scroll**: Target exists in DOM → `scrollTo` called once (1.1)
|
|
|
|
|
-3. **Encoded hash**: URI-encoded hash decoded and resolved correctly (1.2)
|
|
|
|
|
-4. **Custom resolveTarget**: Provided closure is called instead of default `getElementById` (5.2)
|
|
|
|
|
-5. **Custom scrollTo**: Provided scroll function is called instead of default `scrollIntoView` (5.3)
|
|
|
|
|
-6. **Late-mounting renderers**: Rendering elements that appear after the initial scroll are detected and trigger a re-scroll
|
|
|
|
|
-7. **No spurious re-scroll when no renderers**: When no rendering elements ever appear, the watch times out without calling `scrollTo` again
|
|
|
|
|
-8. **Deferred scroll**: Target appears after initial render via MutationObserver (2.1, 2.2)
|
|
|
|
|
-9. **Target observation timeout**: 10s timeout when target never appears (2.3)
|
|
|
|
|
-10. **Key change cleanup**: Observers and timers from previous run are released (5.6)
|
|
|
|
|
-11. **Unmount cleanup**: All resources released (5.7, 6.1)
|
|
|
|
|
-
|
|
|
|
|
-### watchRenderingAndReScroll Tests (co-located in `src/client/util/`)
|
|
|
|
|
-
|
|
|
|
|
-1. **Rendering elements present**: Poll timer fires at 5s, re-scroll executes (3.1)
|
|
|
|
|
-2. **No rendering elements**: No timer scheduled (3.3)
|
|
|
|
|
-3. **Timer non-reset on DOM mutation**: Mid-timer mutation does not restart the 5s interval (3.5)
|
|
|
|
|
-4. **Late-appearing rendering elements**: Observer detects and schedules timer (3.4)
|
|
|
|
|
-5. **Multiple rendering elements**: Only one re-scroll per cycle (6.3)
|
|
|
|
|
-6. **Watch timeout**: All resources cleaned up after 10s (3.6, 6.2)
|
|
|
|
|
-7. **Cleanup prevents post-cleanup execution**: Stopped flag prevents race (6.1)
|
|
|
|
|
-8. **Rendering completes before first timer**: Immediate re-scroll fires via wasRendering path, no extra scroll after that
|
|
|
|
|
-9. **Active timer cancelled when rendering elements removed**: Avoids redundant re-scroll
|
|
|
|
|
-
|
|
|
|
|
-### useKeywordRescroll Tests (co-located in `features/search/.../SearchPage/`)
|
|
|
|
|
-
|
|
|
|
|
-1. **watchRenderingAndReScroll called with scroll container**: Correct container element passed
|
|
|
|
|
-2. **scrollToKeyword scrolls to first .highlighted-keyword**: Container-relative scroll calculation verified
|
|
|
|
|
-3. **scrollToKeyword returns false when no keyword found**: No scroll attempted
|
|
|
|
|
-4. **MutationObserver set up on container**: Correct observe config verified
|
|
|
|
|
-5. **Cleanup on unmount**: MO disconnected, rendering watch cleanup called, debounce cancelled
|
|
|
|
|
-6. **Key change re-runs effect**: New watch started for new key
|
|
|
|
|
-7. **Null container guard**: No-op when scrollElementRef.current is null
|
|
|
|
|
-
|
|
|
|
|
-### SearchResultContent Tests (co-located with component)
|
|
|
|
|
-
|
|
|
|
|
-1. **Hook integration**: `useKeywordRescroll` called with correct key and scroll container ref
|
|
|
|
|
-2. **Key change**: Hook re-called with new key on page change
|
|
|
|
|
-
|
|
|
|
|
-### MermaidViewer Tests
|
|
|
|
|
-
|
|
|
|
|
-1. **rAF cleanup on unmount**: Pending `requestAnimationFrame` cancelled on unmount
|
|
|
|
|
-2. **Rendering attribute lifecycle**: "true" initially → "false" via rAF after render → "false" immediately on error
|
|
|
|
|
-
|
|
|
|
|
-### PlantUmlViewer Tests
|
|
|
|
|
-
|
|
|
|
|
-1. **Rendering attribute lifecycle**: "true" initially → "false" on img load → "false" on img error
|
|
|
|
|
-2. **img src**: Correct src attribute rendered
|
|
|