feat(editor): entry tours (workspace-shell C3) - #223
Merged
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughAdds Compose and Transcribe first-run editor tours with persisted progress, Help-menu replay, action-driven advancement, entry-flow startup, load reset behavior, UI controls, and isolated tests. ChangesEditor entry tours
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Editor
participant TourState
participant LocalStorage
participant TourCard
Editor->>TourState: start Compose or Transcribe tour
TourState->>LocalStorage: read or persist lane state
TourState->>TourCard: render current step
Editor->>TourState: report matching editor action
TourState->>TourCard: advance and render next step
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/tour.js`:
- Around line 106-109: Update editorTourEscape to set LS_SEEN('compose') before
calling _tourStart('compose', 0), while preserving the existing current-lane
marking behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6c8f9c00-6577-49d4-9585-60a105f23eb5
📒 Files selected for processing (12)
CHANGELOG.mdscreen.htmlsrc/add-note.jssrc/audio.jssrc/create.jssrc/file-ops.jssrc/input.jssrc/main.jssrc/menu-bar.jssrc/tempo.jssrc/tour.jstests/tours.test.mjs
Charrette §3.5. Two ≤4-step, task-based first-run tours in a new src/tour.js,
seeded by how you entered the editor (the C1 lane signal):
- Compose (create-from-scratch): place → snap → play → loop.
- Transcribe (import): the reframe "the recording never moves — you line the
GRID up to it," taught by doing — turn on Onsets, put the first barline on
the first attack, tap tempo, switch snap to Onset.
A small state machine (start / advance / skip / resume / complete + the escape
transition) drives a non-modal card. Steps advance when the user actually does
the task — the tour listens on the existing action sites (note placement, snap
change, play, onset toggle, tempo-map entry, tap tempo, snap→onset) — or via
the card's Next button, so you're never stuck. Skippable, and resumable from
Help ▸ Editor tour (which replays a completed tour from the top). Transcribe's
"I'll align later" drops into the Compose tour. A tour auto-starts at most once
per lane; all state is editor-pref (localStorage), never the pack. Closes on a
fresh song load, keeping the resume point.
tests/tours.test.mjs (8): one-time auto-start, step advance (Next + matching
task action), completion, skip-keeps-resume, Help resume/replay, escape→Compose,
load-reset, ≤4 task-based steps. Verified live: Help opens the Transcribe tour;
toggling Onsets advances the task step; Next advances; "I'll align later"
switches to Compose; Skip closes it; no console errors. npm test 115 green,
lint 0 errors.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EBQCHCNA81E9tHmSDHSe2Q
ChrisBeWithYou
force-pushed
the
feat/editor-tours
branch
from
July 12, 2026 20:11
5f5a847 to
a822338
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main.js (1)
1311-1320: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winOnly advance the Play step when playback starts.
_tourNoteAction('play')currently runs after bothstartPlayback()andstopPlayback(). If the tour is on the Play step while playback is already active, stopping playback incorrectly advances the tour without satisfying the task.Proposed fix
- if (S.playing) stopPlayback(); else startPlayback(); - _tourNoteAction('play'); // C3 Compose tour: step 3 task + if (S.playing) { + stopPlayback(); + } else { + startPlayback(); + _tourNoteAction('play'); // C3 Compose tour: step 3 task + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main.js` around lines 1311 - 1320, Update editorTogglePlay so _tourNoteAction('play') is invoked only in the startPlayback branch, not after stopPlayback; preserve the existing recording-finalization and playback toggle behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/tours.test.mjs`:
- Around line 102-110: Add assertions to the test for the transcribe escape flow
to verify the Compose seen flag after completing the escaped-into Compose tour,
then call _editorMaybeStartTour('compose') again and assert it does not restart.
Preserve the existing assertions for the active Compose lane and transcribe seen
flag.
---
Outside diff comments:
In `@src/main.js`:
- Around line 1311-1320: Update editorTogglePlay so _tourNoteAction('play') is
invoked only in the startPlayback branch, not after stopPlayback; preserve the
existing recording-finalization and playback toggle behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3b9f482e-6bee-4dc4-9f46-2128a901fe77
📒 Files selected for processing (12)
CHANGELOG.mdscreen.htmlsrc/add-note.jssrc/audio.jssrc/create.jssrc/file-ops.jssrc/input.jssrc/main.jssrc/menu-bar.jssrc/tempo.jssrc/tour.jstests/tours.test.mjs
🚧 Files skipped from review as they are similar to previous changes (7)
- CHANGELOG.md
- src/menu-bar.js
- src/file-ops.js
- screen.html
- src/input.js
- src/create.js
- src/tour.js
# Conflicts: # CHANGELOG.md # screen.html # src/file-ops.js # src/input.js # src/main.js # src/tempo.js
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
2 participants
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.
Workspace-shell Phase C3 (charrette §3.5). Two short, task-based first-run tours in a new
src/tour.js, seeded by how you entered the editor (the same C1 lane signal). Requires B4/B5/C1 — all merged.The two tours (≤4 steps each)
How it works
Tests & verification
tests/tours.test.mjs(8): one-time auto-start, step advance (Next and the matching task action), completion, skip-keeps-resume, Help resume/replay, escape→Compose, load-reset, and ≤4 task-based steps.npm test115 green,npm run lint0 errors (3 pre-existing warnings = main's ratchet).screen_markup+menu_modelstay green (new card is a balanced sibling of the canvas; Help gains the "Editor tour" row).Completes the workspace-shell onboarding track (C1 #216 · C2 #222 · C3 this). Independent of the tempo track.
🤖 Generated with Claude Code
Summary by CodeRabbit