Yuki Takei 1 неделя назад
Родитель
Сommit
d3029092b9

+ 31 - 0
.kiro/specs/collaborative-editor-awareness/research.md

@@ -104,8 +104,39 @@
 - **`y-protocols` not a direct dependency**: `y-protocols/awareness` exports the `Awareness` class, but neither `@growi/editor` nor `apps/app` list `y-protocols` as a direct dependency. `import type { Awareness } from 'y-protocols/awareness'` fails under strict pnpm resolution. Mitigated by deriving the type from the existing `y-websocket` dependency: `type Awareness = WebsocketProvider['awareness']`.
 - **`view.viewport` vs `view.visibleRanges`** (discovered during validation): CodeMirror's `view.viewport` returns the **rendered** content range, which includes a pre-render buffer beyond the visible area for smooth scrolling. Using it for off-screen classification causes cursors in the buffer zone to be treated as in-viewport, resulting in invisible widget decorations instead of off-screen indicators. Must use `view.visibleRanges` (the ranges actually visible to the user) for accurate classification. Precedent: `setDataLine.ts` in the same package already uses `view.visibleRanges`.
 
+## Implementation Discoveries
+
+### Multi-Mode Viewport Classification
+
+- **Context**: Off-screen cursor classification using `view.visibleRanges` worked in tests (jsdom with fixed-height containers) but failed in GROWI production.
+- **Finding**: In GROWI's page-scroll editor setup, CodeMirror's `view.visibleRanges` and `view.viewport` return the **same** range (the full document), because the editor expands to content height and scrolling is handled by the browser page — not CodeMirror's own scroller. Character-position comparison is therefore useless for off-screen detection.
+- **Solution**: Three-mode classification strategy in `plugin.ts`:
+  1. **rangedMode** (`visibleRanges < viewport`): internal-scroll editor (jsdom tests, fixed-height editors) — use character-position boundaries from `visibleRanges`
+  2. **coords mode** (`visibleRanges == viewport`, `scrollDOM.height > 0`): page-scroll editor (GROWI production) — use `view.lineBlockAt(pos)` + `scrollDOM.getBoundingClientRect()` to compute screen Y coordinates
+  3. **degenerate** (`scrollDOM.height == 0`): jsdom with 0-height container — skip classification, all cursors get widget decorations
+- **Constraint**: `view.coordsAtPos()` calls `readMeasured()` internally, which throws "Reading the editor layout isn't allowed during an update". Must use `view.lineBlockAt()` (reads stored height map, safe during update) + raw `getBoundingClientRect()` (not CodeMirror-restricted) instead.
+
+### Material Symbols Font Loading
+
+- **Context**: Off-screen indicator arrow (`arrow_drop_up`/`arrow_drop_down`) rendered as literal text instead of icon.
+- **Finding**: GROWI loads Material Symbols Outlined via Next.js `next/font` in `use-material-symbols-outlined.tsx`. Next.js registers the font with a **hashed family name** (e.g., `__MaterialSymbolsOutlined_xxxxx`), stored in the CSS variable `--grw-font-family-material-symbols-outlined`. Hardcoding `font-family: 'Material Symbols Outlined'` in CodeMirror's `baseTheme` causes a mismatch — the browser cannot find the font.
+- **Solution**: Use `fontFamily: 'var(--grw-font-family-material-symbols-outlined)'` in `theme.ts` so the hashed name is resolved at runtime.
+
+### Parent Container `overflow-y: hidden` Limitation
+
+- **Context**: Off-screen indicator arrow tip was clipped when positioned a few pixels beyond the editor border.
+- **Finding**: `.page-editor-editor-container` inherits `overflow-y: hidden` from `.flex-expand-vert` within the `.flex-expand-vh-100` context (`packages/core-styles/scss/helpers/_flex-expand.scss` + `apps/app/src/styles/scss/layout/_editor.scss`). This clips any content extending beyond `.cm-editor`'s border box. `.cm-editor` itself has no overflow restriction.
+- **Implication**: Off-screen indicators must stay within `.cm-editor`'s border box. Arrow icons use `clip-path` and negative margins to visually align with the border without extending past it.
+
+### Horizontal Positioning via `requestMeasure`
+
+- **Context**: Off-screen indicators should reflect the remote cursor's column position horizontally.
+- **Finding**: `view.coordsAtPos()` cannot be called during `update()` (throws "Reading the editor layout" error). Horizontal positioning must be deferred.
+- **Solution**: After `replaceChildren()`, call `view.requestMeasure()` to schedule a read phase (`coordsAtPos` → screen X) and write phase (`style.left` + `transform: translateX(-50%)`). For virtualized positions (outside viewport), fall back to `contentDOM.getBoundingClientRect().left + col * view.defaultCharacterWidth`.
+
 ## References
 
 - y-codemirror.next v0.3.5 source: `node_modules/.pnpm/y-codemirror.next@0.3.5_.../src/`
 - Yjs awareness protocol: https://docs.yjs.dev/api/about-awareness
 - CodeMirror WidgetType: https://codemirror.net/docs/ref/#view.WidgetType
+- CodeMirror EditorView.lineBlockAt: https://codemirror.net/docs/ref/#view.EditorView.lineBlockAt

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

@@ -1,8 +1,8 @@
 {
   "feature_name": "collaborative-editor-awareness",
   "created_at": "2026-04-07T00:00:00.000Z",
-  "updated_at": "2026-04-08T15:00:00.000Z",
-  "language": "ja",
+  "updated_at": "2026-04-10T13:00:00.000Z",
+  "language": "en",
   "phase": "implementation-complete",
   "approvals": {
     "requirements": {

+ 26 - 126
.kiro/specs/collaborative-editor-awareness/tasks.md

@@ -1,148 +1,48 @@
 # Implementation Plan
 
 - [x] 1. Stabilize the Editing User List
-- [x] 1.1 Fix awareness state filter so undefined entries never reach the editor list renderer
-  - Filter the awareness state values to exclude any entry that does not have a valid `editors` field before passing the list to the editing user list callback
-  - Replace the existing array mapping that produces `undefined` for uninitialized clients with a filter that skips those entries entirely
-  - Ensure the filtered list contains only valid `EditingClient` values
-  - _Requirements: 1.1, 1.4_
-
-- [x] 1.2 Remove direct mutation of the Yjs-managed awareness map on client disconnect
-  - Remove the `awareness.getStates().delete(clientId)` calls that incorrectly mutate Yjs-internal state when a client ID appears in the `removed` list
-  - Rely on Yjs to clean up disconnected client entries before emitting the `update` event, as per the Yjs awareness contract
-  - _Requirements: 1.2_
+- [x] 1.1 Fix awareness state filter (undefined → skip) — _Req 1.1, 1.4_
+- [x] 1.2 Remove direct mutation of Yjs-managed awareness map — _Req 1.2_
 
 - [x] 2. Build the Rich Cursor Extension (Initial)
-- [x] 2.1 (P) Implement cursor widget DOM with name label, avatar image, and initials fallback
-  - _Requirements: 3.4, 3.5_
-
-- [x] 2.2 (P) Broadcast local cursor position to awareness on each selection change
-  - _Requirements: 3.6, 3.7_
-
-- [x] 2.3 (P) Render remote cursor decorations rebuilt from awareness state changes
-  - _Requirements: 3.6, 3.7_
+- [x] 2.1 (P) Cursor widget DOM: name label, avatar image, initials fallback — _Req 3.4, 3.5_
+- [x] 2.2 (P) Broadcast local cursor position to awareness — _Req 3.6, 3.7_
+- [x] 2.3 (P) Render remote cursor decorations from awareness — _Req 3.6, 3.7_
 
-- [x] 3. Integrate Rich Cursor Extension into the Editor Configuration
-  - Suppress the default cursor plugin by passing `null` as the awareness argument to `yCollab`
-  - Add the rich cursor extension as a sibling extension alongside `yCollab` output
-  - Verify `yUndoManagerKeymap` is not duplicated
-  - _Requirements: 1.3, 2.4, 3.6_
+- [x] 3. Integrate Rich Cursor Extension into Editor Configuration — _Req 1.3, 2.4, 3.6_
 
 - [x] 4. Unit Tests for Core Behaviors (Initial)
-- [x] 4.1 (P) Test awareness state filtering and mutation-free disconnect handling in the hook
-  - _Requirements: 1.1, 1.2, 1.4_
+- [x] 4.1 (P) Awareness state filtering and mutation-free disconnect — _Req 1.1, 1.2, 1.4_
+- [x] 4.2 (P) Cursor widget construction, equality, avatar fallback — _Req 3.4, 3.5_
 
-- [x] 4.2 (P) Test cursor widget construction, equality, and avatar fallback behavior
-  - _Requirements: 3.4, 3.5_
-
-- [x] 5. Integration Tests for Multi-Client Collaborative Scenarios (Initial)
-- [x] 5.1 Test awareness update flow to EditingUserList with multiple simulated clients
-  - _Requirements: 1.3, 2.1, 2.4_
-
-- [x] 5.2 Test cursor position broadcasting and remote cursor rendering in the editor view
-  - _Requirements: 3.6, 3.7_
+- [x] 5. Integration Tests for Multi-Client Collaborative Scenarios
+- [x] 5.1 Awareness update flow to EditingUserList — _Req 1.3, 2.1, 2.4_
+- [x] 5.2 Cursor position broadcasting and remote rendering — _Req 3.6, 3.7_
 
 - [x] 6. Add baseTheme with Overlay Positioning, Hover, and Opacity Rules
-- [x] 6.1 (P) Create the EditorView.baseTheme defining all cursor overlay CSS rules
-  - Define overlay positioning for the cursor flag element: absolute below the caret, centered on the 1px caret line
-  - Set avatar and initials fallback sizes to 16×16 pixels with circular clipping
-  - Set up the two-step hover cascade: pointer-events none by default on the flag, enabled on caret hover
-  - Define the name label as hidden by default, shown on flag hover
-  - Set the default opacity to semi-transparent with a smooth transition, full opacity on caret hover or when the active class is present
-  - Include the theme extension in the return value of the rich cursors factory function
-  - _Requirements: 3.1, 3.2, 3.3, 3.8, 3.9_
-
-- [x] 6.2 (P) Define off-screen container and indicator styles in the same baseTheme
-  - Define the top and bottom off-screen containers as absolute-positioned, flex-layout, pointer-events none
-  - Define the off-screen indicator as flex with gap, semi-transparent by default, full opacity with the active class
-  - Define the off-screen avatar and initials with 16×16 sizing matching the in-editor widget
-  - Define the arrow indicator styling
-  - _Requirements: 4.5, 4.7_
+- [x] 6.1 (P) Cursor overlay CSS rules — _Req 3.1, 3.2, 3.3, 3.8, 3.9_
+- [x] 6.2 (P) Off-screen container and indicator styles — _Req 4.5, 4.7_
 
 - [x] 7. Rework RichCaretWidget for Overlay Avatar with Activity State
-- [x] 7.1 Rebuild the widget DOM to render as an overlay with avatar, initials fallback, and hover-revealed name label
-  - Restructure the widget DOM to wrap the avatar and name label inside a flag container element positioned as an overlay below the caret
-  - Render the avatar image at 16×16 pixels when the image URL is available, with an error handler that swaps in the initials fallback
-  - When no image URL is provided, render the initials fallback directly as a colored circle with the user's initial letters
-  - Render the name label element inside the flag container (visibility controlled by the baseTheme hover rule)
-  - Accept an `isActive` parameter and apply the active CSS class to the flag element when true
-  - Update the equality check to include the `isActive` parameter alongside color, name, and image URL
-  - _Requirements: 3.1, 3.2, 3.3, 3.4, 3.5, 3.10_
-
-- [x] 7.2 Add activity tracking to the ViewPlugin with per-client timers
-  - Maintain a map of each remote client's last awareness change timestamp
-  - Maintain a map of per-client timer handles for the 3-second inactivity window
-  - On awareness change for a remote client, record the current timestamp and reset the client's timer to dispatch a decoration rebuild after 3 seconds
-  - When building decorations in the update method, compute each client's active state by comparing the current time against the last activity timestamp
-  - Pass the computed active state to the widget constructor so the DOM reflects the current activity
-  - Clear all timers on plugin destruction
-  - _Requirements: 3.10_
+- [x] 7.1 Widget DOM: overlay flag, avatar/initials, hover name label, isActive — _Req 3.1–3.5, 3.10_
+- [x] 7.2 Activity tracking with per-client timers (3s inactivity) — _Req 3.10_
 
 - [x] 8. Build Off-Screen Cursor Indicators
-- [x] 8.1 Create persistent off-screen containers attached to the editor DOM
-  - Create top and bottom container elements in the ViewPlugin constructor and append them to the editor's outer DOM element
-  - The containers remain in the DOM for the plugin's lifetime (empty when no off-screen cursors exist)
-  - Remove both containers in the plugin's destroy method
-  - _Requirements: 4.7_
-
-- [x] 8.2 Classify remote cursors by visible range position and render off-screen indicators
-  - After computing absolute positions for all remote cursors in the update method, compare each position against `view.visibleRanges` (NOT `view.viewport`, which includes CodeMirror's pre-render buffer beyond the visible area)
-  - Use `visibleRanges[0].from` as the top boundary and `visibleRanges[visibleRanges.length - 1].to` as the bottom boundary
-  - For cursors above the visible range, build an indicator element (arrow up + avatar or initials fallback) and add it to the top container
-  - For cursors below the visible range, build an indicator element (arrow down + avatar or initials fallback) and add it to the bottom container
-  - For cursors within the visible range, render the in-editor widget decoration as before (no off-screen indicator)
-  - Replace container children on each relevant update cycle using a batch DOM operation
-  - Apply the active CSS class to off-screen indicators when the corresponding client's activity state is active
-  - Rebuild containers when the viewport changes or awareness changes
-  - _Requirements: 4.1, 4.2, 4.3, 4.4, 4.5, 4.6_
+- [x] 8.1 Persistent off-screen containers on editor DOM — _Req 4.7_
+- [x] 8.2 Classify cursors by visible range, render indicators — _Req 4.1–4.6_
 
 - [x] 9. Unit Tests for Updated Widget and Off-Screen Indicators
-- [x] 9.1 (P) Test the updated widget DOM structure, overlay flag, sizing, and isActive class behavior
-  - Verify the widget renders a flag container with position absolute styling inside the caret element
-  - Verify the avatar image renders at 16×16 when image URL is provided
-  - Verify the initials fallback renders with the user's color as background when no image URL is given
-  - Verify the image error handler replaces the image with the initials fallback
-  - Verify the name label element exists inside the flag container
-  - Verify the flag element receives the active CSS class when isActive is true, and does not when false
-  - Verify the equality check returns false when isActive differs
-  - _Requirements: 3.1, 3.2, 3.3, 3.4, 3.5, 3.10_
-
-- [x] 9.2 (P) Test off-screen indicator DOM construction and avatar fallback
-  - Verify an off-screen indicator element contains an arrow element and an avatar image when image URL is provided
-  - Verify an off-screen indicator falls back to an initials element when image URL is absent
-  - Verify the active CSS class is applied to the indicator element when the client is active
-  - _Requirements: 4.1, 4.2, 4.4_
+- [x] 9.1 (P) Widget DOM structure, sizing, isActive, borderColor — _Req 3.1–3.5, 3.10_
+- [x] 9.2 (P) Off-screen indicator DOM, Material Symbols arrow, avatar fallback — _Req 4.1, 4.2, 4.4, 4.9_
 
 - [x] 10. Integration Tests for Viewport Classification and Activity Tracking
-- [x] 10.1 Test that remote cursors outside the visible range are excluded from widget decorations
-  - Simulate a remote client with a cursor position beyond `view.visibleRanges` and verify that no widget decoration is created for that client
-  - _Requirements: 4.3, 4.6_
-
-- [x] 10.2 Test activity tracking timer lifecycle with fake timers
-  - Simulate an awareness change for a remote client and verify the client is marked as active
-  - Advance fake timers by 3 seconds and verify a decoration rebuild is triggered, resulting in the client being marked as inactive
-  - Simulate a new awareness change before the timer expires and verify the timer is reset
-  - _Requirements: 3.10_
+- [x] 10.1 Off-screen exclusion from widget decorations — _Req 4.3, 4.6_
+- [x] 10.2 Activity tracking timer lifecycle (fake timers) — _Req 3.10_
 
 - [x] 12. Fix Off-Screen Visibility Classification
-- [x] 12.1 Replace `view.viewport` with `view.visibleRanges` in the off-screen cursor classification logic
-  - In `plugin.ts`, replace `viewUpdate.view.viewport` with `viewUpdate.view.visibleRanges` for the top/bottom boundary comparison
-  - Use `visibleRanges[0].from` as the top boundary and `visibleRanges[visibleRanges.length - 1].to` as the bottom boundary
-  - Guard against an empty `visibleRanges` array (skip classification if empty)
-  - _Requirements: 4.1, 4.2, 4.3, 4.6_
-
-- [x] 12.2 Update integration test to assert off-screen indicator renders for cursor in the render buffer
-  - Add a test case that places a remote cursor within `view.viewport` but outside `view.visibleRanges` and verifies the off-screen container has an indicator element
-  - _Requirements: 4.3, 4.6_
-
-- [ ]\* 11. E2E Tests for Hover, Opacity, and Off-Screen Transitions
-- [ ]\* 11.1 (P) Test hover behavior on the cursor overlay flag
-  - Hover over a remote user's caret area and verify the name label becomes visible
-  - Move the cursor away and verify the name label is hidden
-  - Verify that clicking on text underneath the overlay correctly places the editor cursor
-  - _Requirements: 3.3, 3.9_
+- [x] 12.1 Multi-mode classification: rangedMode / coordsMode / degenerate — _Req 4.1–4.3, 4.6_
+- [x] 12.2 Integration test for render-buffer cursor → off-screen indicator — _Req 4.3, 4.6_
 
-- [ ]\* 11.2 (P) Test off-screen indicator visibility on scroll
-  - Scroll the editor so a remote user's cursor goes above the viewport and verify the top off-screen container shows an indicator with the correct avatar and arrow
-  - Scroll back to reveal the cursor and verify the off-screen indicator disappears and the in-editor widget reappears
-  - _Requirements: 4.1, 4.2, 4.3, 4.6_
+- [ ]\* 11. E2E Tests for Hover, Opacity, and Off-Screen Transitions (deferred)
+- [ ]\* 11.1 (P) Hover behavior on cursor overlay flag — _Req 3.3, 3.9_
+- [ ]\* 11.2 (P) Off-screen indicator visibility on scroll — _Req 4.1–4.3, 4.6_