From 48c9bf831339d2e8ea22b4806b3b0d033e39b499 Mon Sep 17 00:00:00 2001 From: Nuwan Date: Wed, 25 Feb 2026 19:05:14 +0530 Subject: [PATCH] docs(26): create phase plan for JamTrack Polish Phase 26: JamTrack Polish - 2 plan(s) in 2 wave(s) - Wave 1: 26-01 (UI fixes - sizing, navigation) - Wave 2: 26-02 (state/callback fixes - cleanup, deferred rendering) - Ready for execution Requirements covered: - JT-01: Defer UI rendering until track ready - JT-02: Fix WindowPortal sizing (460x350) - JT-03: Implement create custom mix navigation - JT-04: Add callback cleanup for jamTrackDownload* globals Co-Authored-By: Claude Opus 4.5 --- .planning/ROADMAP.md | 7 +- .../phases/26-jamtrack-polish/26-01-PLAN.md | 127 +++++++++++ .../phases/26-jamtrack-polish/26-02-PLAN.md | 209 ++++++++++++++++++ 3 files changed, 340 insertions(+), 3 deletions(-) create mode 100644 .planning/phases/26-jamtrack-polish/26-01-PLAN.md create mode 100644 .planning/phases/26-jamtrack-polish/26-02-PLAN.md diff --git a/.planning/ROADMAP.md b/.planning/ROADMAP.md index 3b063c537..3aeea8f7c 100644 --- a/.planning/ROADMAP.md +++ b/.planning/ROADMAP.md @@ -31,10 +31,11 @@ v1.6 addresses usability issues reported in three media features: JamTrack (load 2. JamTrack player fits properly in popup window without scrollbars 3. "Create custom mix" button opens JamTrack editor in new tab 4. No console warnings about leaked callbacks when closing JamTrack or navigating away -**Plans**: TBD +**Plans**: 2 plans Plans: -- [ ] 26-01: TBD +- [ ] 26-01-PLAN.md - Fix window sizing and create custom mix navigation +- [ ] 26-02-PLAN.md - Add callback cleanup and defer controls rendering ### Phase 27: Backing Track Sync **Goal**: Backing Track appears in session screen when opened @@ -67,7 +68,7 @@ Plans: | Phase | Milestone | Plans Complete | Status | Completed | |-------|-----------|----------------|--------|-----------| -| 26. JamTrack Polish | v1.6 | 0/TBD | Not started | - | +| 26. JamTrack Polish | v1.6 | 0/2 | Planned | - | | 27. Backing Track Sync | v1.6 | 0/TBD | Not started | - | | 28. Metronome Responsiveness | v1.6 | 0/TBD | Not started | - | diff --git a/.planning/phases/26-jamtrack-polish/26-01-PLAN.md b/.planning/phases/26-jamtrack-polish/26-01-PLAN.md new file mode 100644 index 000000000..cfb0cdca0 --- /dev/null +++ b/.planning/phases/26-jamtrack-polish/26-01-PLAN.md @@ -0,0 +1,127 @@ +--- +phase: 26-jamtrack-polish +plan: 01 +type: execute +wave: 1 +depends_on: [] +files_modified: + - jam-ui/src/components/client/JKSessionScreen.js + - jam-ui/src/components/client/JKSessionJamTrackPlayer.js +autonomous: true + +must_haves: + truths: + - "JamTrack player fits properly in popup window without scrollbars" + - "Create custom mix button opens JamTrack editor in new tab" + artifacts: + - path: "jam-ui/src/components/client/JKSessionScreen.js" + provides: "Corrected WindowPortal dimensions for JamTrack player" + contains: "width=460,height=350" + - path: "jam-ui/src/components/client/JKSessionJamTrackPlayer.js" + provides: "Working create custom mix navigation" + contains: "window.open" + key_links: + - from: "JKSessionJamTrackPlayer.js" + to: "/jamtracks/{id}" + via: "window.open on create custom mix click" + pattern: "window\\.open.*jamtracks" +--- + + +Fix JamTrack player window sizing and implement "create custom mix" navigation. + +Purpose: Address two visual/navigation bugs that impact user experience - popup window has scrollbars due to oversized dimensions, and "create custom mix" button does nothing. +Output: JamTrack player that fits its popup window and allows users to create custom mixes via the JamTrack editor. + + + +@/Users/nuwan/.claude/get-shit-done/workflows/execute-plan.md +@/Users/nuwan/.claude/get-shit-done/templates/summary.md + + + +@.planning/PROJECT.md +@.planning/ROADMAP.md +@.planning/STATE.md +@jam-ui/src/components/client/JKSessionScreen.js +@jam-ui/src/components/client/JKSessionJamTrackPlayer.js + + + + + + Task 1: Fix WindowPortal sizing for JamTrack player + jam-ui/src/components/client/JKSessionScreen.js + +Find the WindowPortal for JamTrack player (around line 1750) with `windowFeatures="width=600,height=500..."`. + +Change the dimensions to `width=460,height=350` to properly fit the player content (which is 420px wide with 20px padding on each side). + +Before: +``` +windowFeatures="width=600,height=500,left=200,top=200,menubar=no,toolbar=no,status=no,scrollbars=yes,resizable=yes,location=no, addressbar=no" +``` + +After: +``` +windowFeatures="width=460,height=350,left=200,top=200,menubar=no,toolbar=no,status=no,scrollbars=no,resizable=yes,location=no,addressbar=no" +``` + +Note: Also change `scrollbars=yes` to `scrollbars=no` since the window should now fit content without scrolling. + + +Grep for the updated window dimensions: +```bash +grep -n "width=460,height=350" jam-ui/src/components/client/JKSessionScreen.js +``` +Should find the JamTrack WindowPortal with the new dimensions. + + WindowPortal for JamTrack player uses width=460, height=350 with scrollbars=no + + + + Task 2: Implement "create custom mix" navigation + jam-ui/src/components/client/JKSessionJamTrackPlayer.js + +Find the "create custom mix" link (around line 851-861) which currently has: +```jsx +onClick={() => console.log('TODO: Open custom mix creator')} +``` + +Replace with proper navigation that opens the JamTrack editor in a new tab: +```jsx +onClick={() => window.open(`/jamtracks/${jamTrack?.id}`, '_blank', 'noopener,noreferrer')} +``` + +The JamTrack editor route at `/jamtracks/{id}` already exists in the application and provides the full stem mixing interface. + +Note: Use `noopener,noreferrer` for security best practices when opening external windows. + + +Grep for the window.open implementation: +```bash +grep -n "window.open.*jamtracks" jam-ui/src/components/client/JKSessionJamTrackPlayer.js +``` +Should find the onClick handler with proper navigation. + + Clicking "create custom mix" opens /jamtracks/{id} in a new browser tab + + + + + +1. Build check: `cd jam-ui && npm run build` completes without errors +2. Code verification: + - JKSessionScreen.js has `width=460,height=350` for JamTrack WindowPortal + - JKSessionJamTrackPlayer.js has `window.open` for create custom mix link + + + +- JamTrack player WindowPortal uses correct dimensions (460x350) +- "Create custom mix" link navigates to /jamtracks/{id} in new tab +- No regressions in existing functionality + + + +After completion, create `.planning/phases/26-jamtrack-polish/26-01-SUMMARY.md` + diff --git a/.planning/phases/26-jamtrack-polish/26-02-PLAN.md b/.planning/phases/26-jamtrack-polish/26-02-PLAN.md new file mode 100644 index 000000000..6384760df --- /dev/null +++ b/.planning/phases/26-jamtrack-polish/26-02-PLAN.md @@ -0,0 +1,209 @@ +--- +phase: 26-jamtrack-polish +plan: 02 +type: execute +wave: 2 +depends_on: ["26-01"] +files_modified: + - jam-ui/src/store/features/mediaSlice.js + - jam-ui/src/components/client/JKSessionJamTrackPlayer.js +autonomous: true + +must_haves: + truths: + - "No console warnings about leaked callbacks when closing JamTrack" + - "User sees loading indicator while backend processes track (not premature controls)" + artifacts: + - path: "jam-ui/src/store/features/mediaSlice.js" + provides: "Callback cleanup function for jamTrackDownload* globals" + contains: "cleanupJamTrackCallbacks" + - path: "jam-ui/src/components/client/JKSessionJamTrackPlayer.js" + provides: "Callback cleanup on unmount and deferred controls rendering" + contains: "cleanupJamTrackCallbacks" + key_links: + - from: "JKSessionJamTrackPlayer.js" + to: "mediaSlice.js" + via: "dispatch(cleanupJamTrackCallbacks()) in useEffect cleanup" + pattern: "cleanupJamTrackCallbacks" + - from: "JKSessionJamTrackPlayer.js render" + to: "downloadState.state" + via: "conditional rendering of controls" + pattern: "downloadState\\.state.*synchronized" +--- + + +Add callback cleanup for JamTrack download globals and defer player controls rendering until track is ready. + +Purpose: Prevent memory leaks from orphaned window callbacks when closing JamTrack, and fix the UX issue where player controls appear before the track is actually ready to play. +Output: Clean callback lifecycle and proper loading state management for JamTrack player. + + + +@/Users/nuwan/.claude/get-shit-done/workflows/execute-plan.md +@/Users/nuwan/.claude/get-shit-done/templates/summary.md + + + +@.planning/PROJECT.md +@.planning/ROADMAP.md +@.planning/STATE.md +@.planning/phases/26-jamtrack-polish/26-01-SUMMARY.md +@jam-ui/src/store/features/mediaSlice.js +@jam-ui/src/components/client/JKSessionJamTrackPlayer.js +@jam-ui/src/hooks/useRecordingHelpers.js (lines 465-498 for cleanup pattern) + + + + + + Task 1: Add callback cleanup action to mediaSlice + jam-ui/src/store/features/mediaSlice.js + +The downloadJamTrack thunk (lines 234-244) sets window globals but never cleans them up: +- window.jamTrackDownloadProgress +- window.jamTrackDownloadSuccess +- window.jamTrackDownloadFail + +Add a new action to clean up these callbacks. Add this after the existing slice actions (around line 400-450 where other actions are defined): + +```javascript +// Add to slice actions or as a standalone function +export const cleanupJamTrackCallbacks = () => { + // Clean up global callbacks set during download + if (typeof window !== 'undefined') { + delete window.jamTrackDownloadProgress; + delete window.jamTrackDownloadSuccess; + delete window.jamTrackDownloadFail; + } +}; +``` + +This follows the same pattern used in useRecordingHelpers.js (lines 488-496) for cleaning up window.JK callbacks. + +Note: This is a simple function, not a thunk, since it doesn't need dispatch or getState. + + +Grep for the cleanup function: +```bash +grep -n "cleanupJamTrackCallbacks" jam-ui/src/store/features/mediaSlice.js +``` +Should find the export and implementation. + + mediaSlice exports cleanupJamTrackCallbacks function that removes window.jamTrackDownload* globals + + + + Task 2: Call cleanup on unmount and defer controls rendering + jam-ui/src/components/client/JKSessionJamTrackPlayer.js + +Two changes needed: + +**1. Import and call cleanup on unmount:** + +Add to imports (around line 4-9): +```javascript +import { + loadJamTrack, + checkJamTrackSync, + closeJamTrack, + setJamTrackState, + setDownloadState, + cleanupJamTrackCallbacks // Add this +} from '../../store/features/mediaSlice'; +``` + +Update the cleanup useEffect (around lines 163-172) to also call cleanup: +```javascript +useEffect(() => { + return () => { + mountedRef.current = false; + + if (fqIdRef.current && jamClient) { + dispatch(closeJamTrack({ fqId: fqIdRef.current, jamClient })); + dispatch(clearOpenJamTrack()); + } + + // Clean up download callbacks + cleanupJamTrackCallbacks(); + }; +}, [dispatch, jamClient]); +``` + +**2. Defer controls rendering until track is ready:** + +Find the Controls Section (around line 720) and wrap it in a condition that checks if the track is ready: + +```jsx +{/* Controls Section - only show when not in loading/download states */} +{(downloadState.state === 'idle' || downloadState.state === 'synchronized') && !isLoadingSync && ( +
+ {/* ... existing controls ... */} +
+)} +``` + +This ensures the playback controls only appear when: +- downloadState is 'idle' (local file already synced) OR 'synchronized' (download complete) +- AND not in initial loading state + +The download progress UI (lines 593-711) already shows during download states, so this creates proper UX flow: loading -> download progress -> controls appear. + + +```bash +# Check cleanup is imported and called +grep -n "cleanupJamTrackCallbacks" jam-ui/src/components/client/JKSessionJamTrackPlayer.js + +# Check conditional rendering is in place +grep -n "downloadState.state.*synchronized" jam-ui/src/components/client/JKSessionJamTrackPlayer.js +``` +Both should return matches. + + JKSessionJamTrackPlayer calls cleanupJamTrackCallbacks on unmount and only renders controls when track is ready + + + + Task 3: Build verification and manual test notes + None (verification only) + +Run build to verify no syntax errors: +```bash +cd jam-ui && npm run build +``` + +Document manual test steps for verification: +1. Open a JamTrack that needs downloading +2. Observe: Loading indicator shown, controls hidden +3. Wait for download to complete +4. Observe: Controls appear after download +5. Close JamTrack window +6. Open browser console, verify no warnings about leaked callbacks +7. Verify no "state update on unmounted component" warnings + + +Build command exits with code 0 and no TypeScript/compilation errors. + + Build passes with no errors, manual test steps documented + + + + + +1. Build check: `cd jam-ui && npm run build` completes without errors +2. Code verification: + - mediaSlice.js exports cleanupJamTrackCallbacks + - JKSessionJamTrackPlayer.js imports and calls cleanupJamTrackCallbacks in cleanup + - JKSessionJamTrackPlayer.js conditionally renders controls based on downloadState +3. No eslint errors in modified files + + + +- cleanupJamTrackCallbacks function exists and is exported from mediaSlice +- JKSessionJamTrackPlayer calls cleanup function on unmount +- Player controls only render when downloadState is 'idle' or 'synchronized' +- Build passes with no errors +- No console warnings about leaked callbacks or unmounted component updates + + + +After completion, create `.planning/phases/26-jamtrack-polish/26-02-SUMMARY.md` +