Add CLAUDE.md modular architecture refactor game plan#251
Open
Add CLAUDE.md modular architecture refactor game plan#251
Conversation
Complete codebase audit (24 findings) and 30 modularization recommendations covering visual framework separation, logic modularization, testing infrastructure, performance and accessibility hardening, and repo cleanup. 13-session execution plan with per-task specs, acceptance criteria, and dependency ordering. This serves as the persistent roadmap for incremental refactoring across multiple sessions. https://claude.ai/code/session_012fJP8VEtRL2fj1Fdgo6aEV
…timize icon - Delete 5 dead Vite boilerplate files (App.css, react.svg, 3 public SVGs) - Gate FogOfWarLayer diagnostic logging behind DEBUG_VISION flag (was ~40 lines of console.log firing on every render in dev mode) - Optimize icon.png from 927KB to 72KB (512x512, 128-color quantized) - Add coverage/ to .gitignore (generated reports shouldn't be tracked) - Document test coverage baseline in CLAUDE.md session notes: 33% statements, 27% branches, 40% functions across 792 passing tests https://claude.ai/code/session_012fJP8VEtRL2fj1Fdgo6aEV
…action - Consolidate root docs: delete duplicate ARCHITECTURE.md, move 8 docs to docs/ subdirs (guides/, features/, architecture/, planning/) - Move diagnose-dungeon.ts to tests/helpers/, update tsconfig.json - Update all internal cross-references across moved docs - Create 4 component subdirectories: ErrorBoundaries/, Dialogs/, Managers/, Mobile/ - Move 32 files (17 error boundaries, 6 dialogs, 6 managers, 3 mobile) - Update ~35 import paths in consumer files and moved files - Fix vi.mock() paths in test files, fix import ordering via eslint --fix - Extract 14 domain types + 2 constants from gameStore.ts to src/types/domain.ts - gameStore.ts re-exports all types for backward compatibility - Update IStorageService.ts to import from types/domain per architecture contract All 792 tests pass. Zero TypeScript errors. Zero lint errors. Build succeeds. https://claude.ai/code/session_012fJP8VEtRL2fj1Fdgo6aEV
… contrast - Add ~120 CSS custom properties to theme.css covering canvas, doors, stairs, minimap, measurement, touch, tokens, walls, toolbar, monitor, home, dialogs, error boundaries, logo, and library categories with dark-mode variants - Sweep 20+ component files replacing hardcoded hex/rgba values with theme tokens; use *_COLORS constant objects for Konva canvas components (can't use CSS vars) - Remove 7 redundant Radix dark-mode CSS imports (values already declared in [data-theme='dark'] block) - Scope global * transition rule to specific UI classes + .themed-transition opt-in - Add @media (prefers-contrast: more) block for WCAG 2.2 AA high-contrast support - Create brand.css configuration layer for single-file rebranding (accent colors, fonts, logo paths) All 792 tests pass. TypeScript 0 errors. Build succeeds. https://claude.ai/code/session_012fJP8VEtRL2fj1Fdgo6aEV
Create the design system primitive layer (src/components/primitives/): - Button: 5 variants (primary, secondary, ghost, destructive, tool), 3 sizes (sm, md, lg), isActive/isLoading/leftIcon/rightIcon support, focus-visible ring, aria-disabled/aria-busy attributes - Input: label, error state with role="alert", helper text, aria-invalid, auto-generated IDs via useId() - Card: 3 variants (surface, elevated, outlined), configurable padding - ToggleSwitch: moved to primitives/ with re-export at old path Add primitives.css with all component styles using theme tokens. Migrate 7 buttons across 3 files (ConfirmDialog, DoorControls, MapSettingsSheet) as proof-of-concept. Update DesignSystemPlayground to showcase all new primitives. All 792 tests pass. TypeScript 0 errors. Build succeeds. https://claude.ai/code/session_012fJP8VEtRL2fj1Fdgo6aEV
- Create Dialog primitive with full a11y: focus trap (Tab/Shift+Tab), Escape/overlay-click to close, auto-focus on open, return-focus on close, scroll lock, aria-modal/labelledby/describedby, 4 size variants, prefers-reduced-motion support - Migrate ConfirmDialog and PreferencesDialog to use Dialog primitive, eliminating duplicate overlay/keyboard/focus handling - Extract 1,032 lines of inline <style> from HomeScreen.tsx to src/styles/home-screen.css — HomeScreen drops from 1,776 to 745 lines - JS bundle drops 26KB (912KB→886KB) since CSS string no longer in JS All 792 tests pass. TypeScript 0 errors. Build succeeds. https://claude.ai/code/session_012fJP8VEtRL2fj1Fdgo6aEV
…t boundaries Install eslint-plugin-jsx-a11y and fix 74 accessibility violations across 15 files. Upgrade 5 TypeScript rules from warn to error (no-explicit-any, no-misused-promises, no-unsafe-member-access, no-unsafe-call, no-unsafe-assignment) after fixing ~115 violations. Add import/no-restricted-paths enforcing Design System Contract boundaries. Add ExposedIpcRenderer type to window.d.ts for typed IPC access. https://claude.ai/code/session_012fJP8VEtRL2fj1Fdgo6aEV
Created src/store/uiStore.ts with all UI ephemeral state (toast, confirmDialog, showResourceMonitor, dungeonDialog, isGamePaused, isMobileSidebarOpen, isCommandPaletteOpen) extracted from gameStore. gameStore is now domain-pure (607 lines, down from 836). Migrated 23 source files and 9 test files. SyncManager subscription no longer fires on UI state changes. https://claude.ai/code/session_012fJP8VEtRL2fj1Fdgo6aEV
…ic hooks Extracted 4 pure functions (calculateVisibilityPolygon, castRay, lineSegmentIntersection, getWallSegments) from FogOfWarLayer.tsx to src/utils/vision.ts — zero React/Konva dependencies, 100% test coverage (26 tests). Created useRecentCampaigns and usePlatformDetection hooks to remove direct localStorage/navigator access from HomeScreen. https://claude.ai/code/session_012fJP8VEtRL2fj1Fdgo6aEV
- useToolState: tool selection, colors, door orientation, measurement mode, keyboard shortcuts - useMenuCommands: Electron IPC menu handler registration (save/load/new/about) - useLibraryLoader: token library index loading on startup - campaignService: save/load/new campaign orchestration (zero React imports) - Toolbar component: extracted desktop toolbar JSX from App.tsx - App.tsx reduced from 770 to 283 lines (63% reduction) https://claude.ai/code/session_012fJP8VEtRL2fj1Fdgo6aEV
Extract keyboard, drop, selection, and drawing concerns from the 1,892-line CanvasManager into focused, testable hooks. Extract door context menu into a standalone component. CanvasManager reduced to 1,450 lines (23% reduction). New files: - useCanvasKeyboard.ts: keyboard shortcuts + modifier key state - useCanvasDrop.ts: file/token drop handling + image crop flow - useCanvasSelection.ts: selection rect, transformer, hover tracking - useCanvasDrawing.ts: drawing refs/state (isDrawing, tempLine, etc.) - DoorContextMenu.tsx: right-click door actions (open/close/lock/delete) Also: gate diagnostic logging behind DEBUG_CANVAS flag, fix production console.log leak, remove stray DoorLayer render log. https://claude.ai/code/session_012fJP8VEtRL2fj1Fdgo6aEV
Lazy-load 5 infrequent components with React.lazy, reducing main bundle from 891KB to 810KB (gzip 259KB → 238KB, 9% reduction). Add Konva performance budget: pixelRatio capped at 2 (1 on low-end devices detected via deviceMemory/hardwareConcurrency), explored fog regions cached at Konva level to avoid re-executing sceneFunc on unchanged regions. Lazy chunks: DesignSystemPlayground (46KB), DungeonGeneratorDialog (12KB), UpdateManager (11KB), CommandPalette (8KB), ResourceMonitor (6KB). https://claude.ai/code/session_012fJP8VEtRL2fj1Fdgo6aEV
… focus indicators Add CanvasAccessibility component with screen reader announcements for token, door, and tool state changes. Enable keyboard token navigation (Tab to cycle, Enter to activate, Arrow keys to move by grid cell). Add skip-to-content link, landmark roles (nav/main), visible :focus-visible indicators on all interactive elements, and keyboard-accessible sidebar token items with tabIndex/role/aria-label. Fix MapNavigator and Sidebar edit buttons to show on focus-within (not hover-only). https://claude.ai/code/session_012fJP8VEtRL2fj1Fdgo6aEV
All extracted modules now have comprehensive unit test coverage: - uiStore (100%), campaignService (100%), vision (100% — existing) - useToolState (94%), useMenuCommands (96%), useRecentCampaigns (100%) - Button (100%), Dialog (92%), Input (100%) Total: 969 tests passing across 48 test files. https://claude.ai/code/session_012fJP8VEtRL2fj1Fdgo6aEV
- useToolState: wrap DOM element tests in try/finally to prevent leak on failure - Button: add icons.length guard, use toHaveClass instead of toContain - Dialog: remove unused queryDialog, document aria-hidden pattern - campaignService: suppress expected console.error in error-path tests https://claude.ai/code/session_012fJP8VEtRL2fj1Fdgo6aEV
- useToolState: Hoist vi.fn() refs to module scope so gameStore selectors return stable identity (prevents spurious useEffect re-fires). Add test verifying setActiveMeasurement(null) on mode change. - useRecentCampaigns: Replace mockReturnValueOnce with mockReturnValue for mount-phase mocks (StrictMode double-invokes useState initializers). - Dialog: Add auto-focus tests with rAF mocking (first focusable, closed dialog no-op) and focus-restoration tests (return focus to trigger on close). 975 tests passing, 48 test files. https://claude.ai/code/session_012fJP8VEtRL2fj1Fdgo6aEV
- Remove all backward-compat type re-exports from gameStore.ts (ADR-005 cleanup). Migrate 25+ files to import domain types from types/domain directly instead of gameStore. - Delete ToggleSwitch.tsx re-export shim (0 consumers remained). - Move MapSettingsSheet to import ToggleSwitch from primitives/. - Move 6 loose docs from docs/ root to proper subdirectories (features/, guides/, planning/). Fix cross-reference in HYBRID_TESTING_WORKFLOW.md. - Update CHANGELOG.md with comprehensive modular architecture refactor summary covering all 13 sessions. - Clean unused type imports from gameStore.ts (TokenMetadata, ToastMessage, ConfirmDialog). 975 tests passing, TypeScript 0 errors, ESLint 0 errors. https://claude.ai/code/session_012fJP8VEtRL2fj1Fdgo6aEV
Fix gridColor not persisting across map switch/create/delete/IPC sync, keyboard trap in canvas accessibility widget (WCAG 2.1.2), Dialog focus trap edge case, Object URL memory leak, stale Konva cache, dead code, CSS layout mismatch, missing ARIA labels, and wire keyboard token placement. https://claude.ai/code/session_012fJP8VEtRL2fj1Fdgo6aEV
- README.md: Coverage badge 100% → 36% to match actual coverage - CONTRIBUTING.md: Update project structure with new directories (Dialogs/, ErrorBoundaries/, Managers/, Mobile/, primitives/, hooks/, services/, styles/, types/) and document store split (gameStore + uiStore) and campaignService - CLAUDE.md: Update Quick Reference line counts to match actual file sizes after Sessions 12-14 https://claude.ai/code/session_012fJP8VEtRL2fj1Fdgo6aEV
📚 Documentation Check (Rule-Based)This PR was analyzed for potential documentation impacts. 🔌 IPC Changes DetectedFiles in Please review:
Check if you need to:
⚛️ Component Changes DetectedFiles in Please review:
Check if you need to:
🗄️ State Management Changes DetectedFiles in Please review:
Check if you need to:
🛠️ Utility Changes DetectedFiles in Please review:
Check if you need to: 📦 Dependencies Changed
Please review:
Check if you need to:
📖 Documentation StandardsAll changes should follow Hyle's documentation standards:
Complete documentation guide: DOCUMENTATION.md Rule-based check • For AI-powered analysis, see documentation-check.yml |
📚 Documentation CheckGitHub Copilot has analyzed this PR for documentation impact. null 📝 Documentation Files ReferenceRoot docs: View all See DOCUMENTATION.md for complete documentation inventory. Powered by GitHub Copilot • Documentation Guide |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a long-form refactor roadmap (CLAUDE.md) and applies a broad set of modularization + accessibility + performance changes across the UI, canvas subsystem, linting, and documentation.
Changes:
- Extracts canvas concerns into dedicated hooks/components (drop/crop, drawing state, selection, keyboard, door context menu) and moves domain types into
src/types/domain. - Improves accessibility (ARIA roles/labels, keyboard interactions, skip-link) and adds
eslint-plugin-jsx-a11yto enforce a11y linting. - Refactors app composition (lazy-loaded modals/managers), consolidates docs/paths, and removes legacy scaffold files.
Reviewed changes
Copilot reviewed 38 out of 192 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/components/Canvas/hooks/useCanvasInteraction.ts | Switches canvas hook type imports to types/domain. |
| src/components/Canvas/hooks/useCanvasDrop.ts | New hook for canvas drag/drop + crop flow. |
| src/components/Canvas/hooks/useCanvasDrawing.ts | New hook centralizing drawing/measurement/calibration refs/state. |
| src/components/Canvas/TouchVisualFeedback.tsx | Centralizes touch feedback colors into constants. |
| src/components/Canvas/StairsShape.tsx | Moves Stairs type import + centralizes Konva colors. |
| src/components/Canvas/StairsLayer.tsx | Moves Stairs type import to types/domain. |
| src/components/Canvas/MovementRangeOverlay.tsx | Moves GridType import + centralizes overlay colors. |
| src/components/Canvas/Minimap.tsx | Centralizes minimap colors into constants. |
| src/components/Canvas/MeasurementOverlay.tsx | Centralizes measurement overlay colors into constants. |
| src/components/Canvas/GridOverlay.tsx | Moves GridType import + centralizes highlight colors. |
| src/components/Canvas/FogOfWarLayer.tsx | Extracts vision utils usage + adds explored-region caching + gates diagnostics. |
| src/components/Canvas/DoorShape.tsx | Moves Door type import + centralizes door rendering colors. |
| src/components/Canvas/DoorLayer.tsx | Moves Door type import to types/domain. |
| src/components/Canvas/DoorInteraction.test.ts | Updates Door type import path. |
| src/components/Canvas/DoorContextMenu.tsx | New extracted door context menu component. |
| src/components/Canvas/DoorBlocking.test.ts | Updates Door type import path (currently incorrect). |
| src/components/Canvas/CanvasManager.tsx | Major wiring: extracted hooks, a11y layer, performance config, type import moves. |
| src/components/Canvas/CanvasAccessibility.tsx | New screen-reader bridge + keyboard token navigation. |
| src/components/AutoSaveManager.tsx | Removes old location (moved to Managers). |
| src/components/AssetLibrary/TokenMetadataEditor.tsx | Moves toast from gameStore to uiStore + improves label/input associations. |
| src/components/AssetLibrary/TokenMetadataEditor.test.tsx | Updates tests to match uiStore toast source. |
| src/components/AssetLibrary/LibraryModalErrorBoundary.tsx | Adds role="presentation" for overlay click handling (a11y lint). |
| src/components/AssetLibrary/LibraryManager.tsx | Moves confirm/toast to uiStore + wraps async callbacks for lint rules. |
| src/components/AssetLibrary/CommandPalette.tsx | Adds presentation roles + attempts keyboard activation for result items. |
| src/components/AssetLibrary/AddToLibraryDialog.tsx | Moves toast to uiStore + wraps async handler calls for lint rules. |
| src/App.tsx | Splits responsibilities into hooks/managers + adds lazy loading + adds skip link/landmarks. |
| src/App.css | Deletes unused Vite scaffold CSS. |
| package.json | Adds eslint-plugin-jsx-a11y. |
| docs/guides/LINTING_MIGRATION_GUIDE.md | Fixes paths/references after doc moves. |
| docs/guides/HYBRID_TESTING_WORKFLOW.md | Fixes doc link path. |
| docs/features/TOUCH_SUPPORT_MIGRATION.md | Fixes doc link paths. |
| docs/architecture/IPC_SYNC_VERIFICATION.md | Fixes doc link path. |
| README.md | Updates coverage badge + fixes linting doc link path. |
| CONTRIBUTING.md | Updates repo structure docs + fixes doc paths. |
| CLAUDE.md | Adds detailed modular architecture refactor plan and session log. |
| CHANGELOG.md | Adds release notes describing modular refactor outcomes. |
| ARCHITECTURE.md | Deletes duplicate root architecture doc. |
| .eslintrc.cjs | Enables jsx-a11y rules + tightens TypeScript safety rules + adds boundary enforcement. |
Comment on lines
2
to
3
| import type { Door } from '../types/domain'; | ||
| import { WallSegment, Point } from '../types/geometry'; |
There was a problem hiding this comment.
These relative imports look incorrect for a file located at src/components/Canvas/... — ../types/* would resolve under src/components/types/* (which likely doesn't exist). Update the paths to point at src/types/domain and src/types/geometry (e.g., ../../types/domain and ../../types/geometry) so tests compile and run.
Suggested change
| import type { Door } from '../types/domain'; | |
| import { WallSegment, Point } from '../types/geometry'; | |
| import type { Door } from '../../types/domain'; | |
| import { WallSegment, Point } from '../../types/geometry'; |
Comment on lines
+136
to
+141
| // Create Object URL for cropping | ||
| const objectUrl = URL.createObjectURL(file); | ||
| setPendingCrop({ src: objectUrl, x, y }); | ||
| } | ||
| }, | ||
| [isWorldView, containerRef, position, scale, gridSize, gridType, addToken], |
There was a problem hiding this comment.
If the user drops a second file while a previous pendingCrop exists, the previous objectUrl is overwritten without being revoked, leaking memory. Revoke the existing pendingCrop.src (if present) before calling setPendingCrop(...), or block subsequent drops until the current crop flow is resolved.
Suggested change
| // Create Object URL for cropping | |
| const objectUrl = URL.createObjectURL(file); | |
| setPendingCrop({ src: objectUrl, x, y }); | |
| } | |
| }, | |
| [isWorldView, containerRef, position, scale, gridSize, gridType, addToken], | |
| // Clean up any existing object URL before creating a new one | |
| if (pendingCrop) { | |
| URL.revokeObjectURL(pendingCrop.src); | |
| } | |
| // Create Object URL for cropping | |
| const objectUrl = URL.createObjectURL(file); | |
| setPendingCrop({ src: objectUrl, x, y }); | |
| } | |
| }, | |
| [isWorldView, containerRef, position, scale, gridSize, gridType, addToken, pendingCrop], |
Comment on lines
+151
to
+170
| // Convert blob to base64 for storage/rendering | ||
| const reader = new FileReader(); | ||
| reader.readAsDataURL(blob); | ||
| reader.onloadend = () => { | ||
| const base64data = reader.result as string; | ||
|
|
||
| addToken({ | ||
| id: crypto.randomUUID(), | ||
| x: pendingCrop.x, | ||
| y: pendingCrop.y, | ||
| src: base64data, | ||
| name: 'New Token', | ||
| type: 'NPC', | ||
| scale: 1, | ||
| }); | ||
|
|
||
| // Revoke the object URL to prevent memory leak | ||
| URL.revokeObjectURL(pendingCrop.src); | ||
| setPendingCrop(null); | ||
| }; |
There was a problem hiding this comment.
Converting cropped images to base64 strings can significantly increase memory usage, store size, and IPC sync payloads (base64 expands data ~33% and stays in JS heap). Prefer persisting the cropped blob via the existing asset pipeline (e.g., resize/WebP + temp asset URL) and store a file/asset URL on the token instead of an inline base64 string.
Suggested change
| // Convert blob to base64 for storage/rendering | |
| const reader = new FileReader(); | |
| reader.readAsDataURL(blob); | |
| reader.onloadend = () => { | |
| const base64data = reader.result as string; | |
| addToken({ | |
| id: crypto.randomUUID(), | |
| x: pendingCrop.x, | |
| y: pendingCrop.y, | |
| src: base64data, | |
| name: 'New Token', | |
| type: 'NPC', | |
| scale: 1, | |
| }); | |
| // Revoke the object URL to prevent memory leak | |
| URL.revokeObjectURL(pendingCrop.src); | |
| setPendingCrop(null); | |
| }; | |
| // Create an object URL for the cropped image instead of a base64 data URL | |
| const objectUrl = URL.createObjectURL(blob); | |
| addToken({ | |
| id: crypto.randomUUID(), | |
| x: pendingCrop.x, | |
| y: pendingCrop.y, | |
| src: objectUrl, | |
| name: 'New Token', | |
| type: 'NPC', | |
| scale: 1, | |
| }); | |
| // Revoke the temporary preview object URL to prevent memory leaks | |
| URL.revokeObjectURL(pendingCrop.src); | |
| setPendingCrop(null); |
Comment on lines
286
to
+297
| <div | ||
| key={cmd.id} | ||
| role="option" | ||
| tabIndex={-1} | ||
| aria-selected={index === selectedIndex} | ||
| onClick={() => handleSelectItem(index)} | ||
| onKeyDown={(e) => { | ||
| if (e.key === 'Enter' || e.key === ' ') { | ||
| e.preventDefault(); | ||
| handleSelectItem(index); | ||
| } | ||
| }} |
There was a problem hiding this comment.
With tabIndex={-1}, these options won’t be reachable via keyboard Tab navigation, and since focus typically remains in the search input, the onKeyDown handler on the option div will not fire in normal use. To make keyboard activation genuinely work, consider either (a) using actual <button> elements for selectable rows, or (b) implementing a roving tabindex / aria-activedescendant pattern where the listbox manages the active option while focus stays on the input.
Comment on lines
+185
to
+200
| // Arrow keys move selected token by one grid cell | ||
| if (['ArrowUp', 'ArrowDown', 'ArrowLeft', 'ArrowRight'].includes(e.key)) { | ||
| if (selectedTokenIds.length === 0) { | ||
| return; | ||
| } | ||
| e.preventDefault(); | ||
|
|
||
| const dx = e.key === 'ArrowLeft' ? -gridSize : e.key === 'ArrowRight' ? gridSize : 0; | ||
| const dy = e.key === 'ArrowUp' ? -gridSize : e.key === 'ArrowDown' ? gridSize : 0; | ||
|
|
||
| for (const id of selectedTokenIds) { | ||
| const token = tokens.find((t) => t.id === id); | ||
| if (token) { | ||
| updateTokenPosition(id, token.x + dx, token.y + dy); | ||
| } | ||
| } |
There was a problem hiding this comment.
This allows token movement via keyboard even when isWorldView is true. In World View, edits are intended to be blocked (and even if state doesn’t sync back, it can still create confusing local divergence until the next sync). Consider gating the Arrow-key movement behavior behind !isWorldView (or behind an explicit 'allow movement in world view' flag) while still keeping announcements/selection available.
Comment on lines
+60
to
+65
| const handleDrop = useCallback( | ||
| (e: React.DragEvent) => { | ||
| // BLOCKED in World View (no file drops allowed) | ||
| if (isWorldView) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
This new drop/crop handler introduces several behavior branches (World View blocking, JSON payload parsing for multiple token types, file drops creating object URLs, crop confirm/cancel cleanup). Given the repo uses Vitest/RTL, it would be valuable to add unit tests covering the main branches and ensuring object URLs are revoked on all exit paths (confirm, cancel, repeated drop).
Owner
Author
|
@copilot open a new pull request to apply changes based on the comments in this thread |
Contributor
Copilot AI
added a commit
that referenced
this pull request
Feb 10, 2026
…essibility
- Fix DoorBlocking.test.ts import paths (../types → ../../types)
- Revoke existing object URL before creating new one in useCanvasDrop
- Use asset pipeline (processImage) instead of base64 for cropped images
- Improve CommandPalette keyboard accessibility (tabIndex={0} + aria-activedescendant)
- Gate arrow key movement in World View in CanvasAccessibility
Addresses feedback from PR #251 review thread.
Co-authored-by: kocheck <7952000+kocheck@users.noreply.github.com>
…lity, and add tests (#252) * Initial plan * Fix PR review issues: import paths, memory leaks, asset pipeline, accessibility - Fix DoorBlocking.test.ts import paths (../types → ../../types) - Revoke existing object URL before creating new one in useCanvasDrop - Use asset pipeline (processImage) instead of base64 for cropped images - Improve CommandPalette keyboard accessibility (tabIndex={0} + aria-activedescendant) - Gate arrow key movement in World View in CanvasAccessibility Addresses feedback from PR #251 review thread. Co-authored-by: kocheck <7952000+kocheck@users.noreply.github.com> * Add comprehensive unit tests for useCanvasDrop hook - Test drag over behavior (World View blocking) - Test library token drop - Test generic token drop - Test file drop and object URL creation - Test object URL revocation on multiple drops (memory leak prevention) - Test crop confirm with asset pipeline integration - Test crop confirm error handling - Test crop cancel cleanup 12 tests covering all main branches and edge cases. Addresses Issue #6 from PR review thread. Co-authored-by: kocheck <7952000+kocheck@users.noreply.github.com> * Fix async handler linting error in CanvasManager Wrap handleCropConfirm with void operator to satisfy TypeScript linter when passing async function to prop expecting void return. Co-authored-by: kocheck <7952000+kocheck@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: kocheck <7952000+kocheck@users.noreply.github.com>
📚 Documentation Check (Rule-Based)This PR was analyzed for potential documentation impacts. 🔌 IPC Changes DetectedFiles in Please review:
Check if you need to:
⚛️ Component Changes DetectedFiles in Please review:
Check if you need to:
🗄️ State Management Changes DetectedFiles in Please review:
Check if you need to:
🛠️ Utility Changes DetectedFiles in Please review:
Check if you need to: 📦 Dependencies Changed
Please review:
Check if you need to:
📖 Documentation StandardsAll changes should follow Hyle's documentation standards:
Complete documentation guide: DOCUMENTATION.md Rule-based check • For AI-powered analysis, see documentation-check.yml |
📚 Documentation CheckGitHub Copilot has analyzed this PR for documentation impact. null 📝 Documentation Files ReferenceRoot docs: View all See DOCUMENTATION.md for complete documentation inventory. Powered by GitHub Copilot • Documentation Guide |
Rewrites FogOfWarLayer from Konva/raycasting to a PixiJS Sprite with FogOfWarFilter applied. Filter is wired into CanvasManager with isDMView = !isWorldView so Architect view sees full map; World View gets GPU fog. Fog is gated on !isDaylightMode matching the existing daylight flag logic.
Adds buildStrokeGeometry() which converts ordered StrokeSamples into a Float32Array/Uint32Array triangle mesh (2 verts per sample, 2 tris per segment) with pressure-scaled ribbon width, ready for PixiJS MeshGeometry.
…etry Replaces the Konva Shape implementation with an imperative PixiJS Mesh that builds a triangle-ribbon via buildStrokeGeometry() and a custom GlProgram (GLSL 300 es, flat colour uniform). pressureToWidth() helper is extracted and fully unit-tested.
Create DrawingLayer.tsx that reads drawings from gameStore and renders each as a PressureSensitiveLine PixiJS Mesh Fragment; remove Konva.Line type from tempLineRef in useCanvasDrawing (replaced by null placeholder); wire DrawingLayer into CanvasManager after FogOfWarLayer.
Rewrites DoorLayer from Konva react-konva to PixiJS Graphics (Option A: all drawing inline, DoorShape component eliminated). DoorLayer now: - Creates a zIndex=60 Container on worldContainer mount/unmount - Redraws all doors imperatively on each doors/selectedIds/tool change - Renders closed doors as filled rectangles, open doors as donut-sector swing arcs + hinge edge rectangles, locked doors with a padlock overlay - Wires pointerdown (toggle/delete) and rightclick (context menu) per-door - Returns null (no React DOM output) Wires DoorLayer into CanvasManager with all required props; also un-comments handleDoorContextMenu, adds stairs selector (with void stub pending Task 5.2 StairsLayer wiring). DoorShape.tsx retained on disk but is no longer imported anywhere.
… 5.2) Rewrites StairsLayer from Konva react-konva to PixiJS Graphics (all drawing inline, StairsShape component eliminated). StairsLayer now: - Creates a zIndex=55 Container on worldContainer mount/unmount (below doors at 60, above tokens at 50) - Redraws all staircases imperatively on each stairs array change - Renders background rectangle (light gray = up, dark gray = down), 4 interior tread lines (horizontal for N/S, vertical for E/W), and a filled directional arrow triangle indicating stairs.direction - Non-interactive (no event handlers — stairs are static elements) - Returns null (no React DOM output) Wires StairsLayer into CanvasManager with worldContainer, stairs, and isWorldView props. Removes the void stairs stub added in Task 5.1.
…PixiJS Rewrites both overlay components from react-konva to the PixiJS v8 imperative pattern (worldContainer + containerRef + useEffect), matching the pattern established by DoorLayer, StairsLayer, and other Phase 5 layers. - MeasurementOverlay: ruler (line+label), blast (circle+dot+label), and cone (poly+dot+label) shapes drawn with PixiJS Graphics + Text; zIndex 150; parseRgba helper converts CSS rgba/hex strings to PixiJS numbers - MovementRangeOverlay: BFS flood-fill logic unchanged; each reachable cell drawn as a closed polygon via Graphics.poly(); zIndex 140 - Both test files rewritten using vi.hoisted + vi.mock pattern (no WebGL needed) - CanvasManager: wires MeasurementOverlay; MovementRangeOverlay kept as TODO import until token-selection context is available in a future phase
…to MovementRangeOverlay - Create src/utils/pixiColor.ts with parseRgba (fixes /./g → /[0-9a-fA-F]/g bug in 3-digit hex expansion) - Add src/utils/pixiColor.test.ts with 5 cases covering hex, 3-digit hex, rgba, rgb, and invalid fallback - Update MeasurementOverlay and MovementRangeOverlay to import from utils and remove local copies - Add named export `export function MovementRangeOverlay` alongside existing default export
… packages
Delete 5 dead-code files (DoorShape, StairsShape, URLImage, CanvasUtils,
useCanvasInteraction) — all replaced by PixiJS equivalents with no live imports.
Remove react-konva import from TokenErrorBoundary; replace canvas indicator
with auto-shown DOM Portal overlay in dev mode, removing showDebugOverlay
toggle state and TOKEN_ERROR_COLORS constant.
Remove vi.mock('react-konva') block from test setup — no longer needed.
Remove Konva migration ESLint override from .eslintrc.cjs — permanent
no-restricted-imports guardrail now enforced across all files.
Uninstall konva, react-konva, use-image packages.
Zero konva/react-konva imports remain in src/. 1005 tests pass.
…nager - Consolidate drawRuler/drawBlast/drawCone params into DrawStyle object (max-params) - Replace non-null assertions in strokeGeometry.ts and PressureSensitiveLine.tsx with safe guards (no-non-null-assertion) - Add eslint-disable-next-line import/no-unused-modules to all public-API exports, test helpers, and not-yet-wired layer components - Add eslint-disable-next-line react-refresh/only-export-components to buildGridGeometry in GridOverlay.tsx - Add eslint-disable comments to glsl.d.ts ambient module declarations - Remove stale TODO comments referencing deleted files useCanvasInteraction, URLImage, and TokenErrorBoundary import from CanvasManager.tsx
…, hexToRgbFloats; pre-allocate uniforms; remove dead props
- Extract usePixiContainer hook (hooks/usePixiContainer.ts) and replace
duplicated mount/unmount useEffect in DoorLayer, StairsLayer,
MeasurementOverlay, and MovementRangeOverlay
- Extract clearContainer to src/utils/pixiUtils.ts; replace all
container.removeChildren().forEach(c => c.destroy({children:true})) calls
- Add hexToRgbFloats to pixiColor.ts; remove private hexToRgb copy from
PressureSensitiveLine
- Pre-allocate _lightsA/_lightsB Float32Arrays in FogOfWarFilter as instance
fields; reuse with fill(0) on every updateLights call
- Remove dead isWorldView prop from StairsLayerProps and CanvasManager callsite
- GridOverlay: replace parseInt hex parse with parseRgba(gridColor).color
- MovementRangeOverlay: return geometry from useMemo to eliminate double
createGridGeometry call; add geometry to useEffect deps
- TextureCache: remove resolved promises from inFlight map via .finally() so
it tracks only truly in-flight loads
Replace clearContainer + full rebuild with a Map<string, Graphics> ref. Stairs are static elements — only add/remove logic needed. Exports stairsKey() pure function and clears the map on worldContainer change to avoid stale refs.
…irsKey export Add worldContainer to the stairs effect dependency array so the diff loop re-runs when usePixiContainer swaps the container, preventing the new container from remaining empty until stairs itself changes. Remove the stairsKey export (a one-liner alias for stair.id) along with its two eslint-disable comments and the test file that existed solely to cover it. Remove the unused default export as well.
…hader recreate on geometry change
… prevent GPU resource leak
📚 Documentation CheckGitHub Copilot has analyzed this PR for documentation impact. null 📝 Documentation Files ReferenceRoot docs: View all See DOCUMENTATION.md for complete documentation inventory. Powered by GitHub Copilot • Documentation Guide |
📚 Documentation Check (Rule-Based)This PR was analyzed for potential documentation impacts. 🔌 IPC Changes DetectedFiles in Please review:
Check if you need to:
⚛️ Component Changes DetectedFiles in Please review:
Check if you need to:
🗄️ State Management Changes DetectedFiles in Please review:
Check if you need to:
🛠️ Utility Changes DetectedFiles in Please review:
Check if you need to: 📦 Dependencies Changed
Please review:
Check if you need to:
📖 Documentation StandardsAll changes should follow Hyle's documentation standards:
Complete documentation guide: DOCUMENTATION.md Rule-based check • For AI-powered analysis, see documentation-check.yml |
❌ Accessibility Audit FailedThis pull request introduces WCAG AA contrast violations. Please fix the following issues before merging: See uploaded report for details. ResourcesReminder: Always use semantic CSS variables (e.g., |
Update package-lock.json to reflect upgraded dev dependencies: flatted 3.4.2 and tar 7.5.11. Resolved URLs and integrity hashes were updated in the lockfile.
📚 Documentation Check (Rule-Based)This PR was analyzed for potential documentation impacts. 🔌 IPC Changes DetectedFiles in Please review:
Check if you need to:
⚛️ Component Changes DetectedFiles in Please review:
Check if you need to:
🗄️ State Management Changes DetectedFiles in Please review:
Check if you need to:
🛠️ Utility Changes DetectedFiles in Please review:
Check if you need to: 📦 Dependencies Changed
Please review:
Check if you need to:
📖 Documentation StandardsAll changes should follow Hyle's documentation standards:
Complete documentation guide: DOCUMENTATION.md Rule-based check • For AI-powered analysis, see documentation-check.yml |
📚 Documentation CheckGitHub Copilot has analyzed this PR for documentation impact. null 📝 Documentation Files ReferenceRoot docs: View all See DOCUMENTATION.md for complete documentation inventory. Powered by GitHub Copilot • Documentation Guide |
❌ Accessibility Audit FailedThis pull request introduces WCAG AA contrast violations. Please fix the following issues before merging: See uploaded report for details. ResourcesReminder: Always use semantic CSS variables (e.g., |
… screen
The package uses dynamic require('url') which crashes Vite's ESM renderer
in dev mode, preventing React from mounting (white screen). The Transformer
was a Phase 2 feature targeting PixiJS v6 with as-any casts — not functional
with v8. Will reimplement with a v8-native solution later.
📚 Documentation Check (Rule-Based)This PR was analyzed for potential documentation impacts. 🔌 IPC Changes DetectedFiles in Please review:
Check if you need to:
⚛️ Component Changes DetectedFiles in Please review:
Check if you need to:
🗄️ State Management Changes DetectedFiles in Please review:
Check if you need to:
🛠️ Utility Changes DetectedFiles in Please review:
Check if you need to: 📦 Dependencies Changed
Please review:
Check if you need to:
📖 Documentation StandardsAll changes should follow Hyle's documentation standards:
Complete documentation guide: DOCUMENTATION.md Rule-based check • For AI-powered analysis, see documentation-check.yml |
📚 Documentation CheckGitHub Copilot has analyzed this PR for documentation impact. null 📝 Documentation Files ReferenceRoot docs: View all See DOCUMENTATION.md for complete documentation inventory. Powered by GitHub Copilot • Documentation Guide |
❌ Accessibility Audit FailedThis pull request introduces WCAG AA contrast violations. Please fix the following issues before merging: See uploaded report for details. ResourcesReminder: Always use semantic CSS variables (e.g., |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Complete codebase audit (24 findings) and 30 modularization recommendations
covering visual framework separation, logic modularization, testing
infrastructure, performance and accessibility hardening, and repo cleanup.
13-session execution plan with per-task specs, acceptance criteria, and
dependency ordering. This serves as the persistent roadmap for incremental
refactoring across multiple sessions.
https://claude.ai/code/session_012fJP8VEtRL2fj1Fdgo6aEV