fix(notes): make mirrored teleprompter content read-only - #228
Conversation
While mirrored, caret placement and text selection are horizontally reversed on screen, so editing there is not a usable mode. The note body now locks whenever it is being presented rather than edited — during playback, as before, and now while mirrored — through a single derived editingLocked state; the formatting buttons disable with it via an explicit formattingDisabled prop instead of piggybacking on isPlaying. The play button's label and icon still track isPlaying alone, the teleprompter controls (play/pause, speed, font size, mirror) stay live while the note is locked, and turning the mirror off with playback paused restores editing. Mirroring never starts playback, pausing never lifts the mirror, and the persisted settings shape is unchanged (speed/fontSize/mirrored only).
|
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 (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughNotes editing is locked during playback or mirrored presentation. Formatting controls use the combined lock state. Teleprompter controls remain available. Tests cover initial state and state transitions. ChangesNotes editing lock
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant NotesWindow
participant NotesEditor
participant NotesToolbar
participant TeleprompterControls
NotesWindow->>NotesEditor: Set editability from playback or mirroring
NotesWindow->>NotesToolbar: Pass formattingDisabled
NotesToolbar->>NotesEditor: Disable formatting actions
NotesToolbar->>TeleprompterControls: Keep playback and display controls enabled
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR makes Notes teleprompter “Mirror” mode presentation-only by locking the editor (read-only) whenever the note is either playing or mirrored, and by disabling formatting controls during those locked states while keeping teleprompter controls usable.
Changes:
- Introduced a single derived
editingLockedstate (isPlaying || settings.mirrored) to drive editor editability. - Added a
formattingDisabledprop toNotesToolbarso formatting buttons disable whenever the note body is locked (playing or mirrored). - Expanded unit/component tests to cover the full state matrix (playing × mirrored) and to assert mirror/playback independence.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/components/launch/NotesWindow.tsx | Derives editingLocked and applies it to editor.setEditable, passing it down to the toolbar to disable formatting while locked. |
| src/components/launch/NotesWindow.test.tsx | Adds coverage ensuring the note is locked for any mirrored/playing combination and only unlocks when both are off; asserts mirror doesn’t auto-play and pause doesn’t unmirror. |
| src/components/launch/NotesToolbar.tsx | Adds formattingDisabled to disable formatting buttons independently of teleprompter controls. |
| src/components/launch/NotesToolbar.test.tsx | Updates toolbar tests to validate formatting lock behavior while keeping teleprompter controls enabled and play labeling tied to isPlaying. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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/components/launch/NotesWindow.tsx`:
- Around line 52-62: Initialize the Tiptap editor through useEditor with
editable set to !editingLocked so mirrored or playing state is enforced on the
first render; alternatively, replace the synchronization useEffect with
useLayoutEffect. Preserve the existing setEditable update behavior when editor
or editingLocked changes.
🪄 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: 687b609c-329e-4a65-bbe8-6ef99d15b3de
📒 Files selected for processing (4)
src/components/launch/NotesToolbar.test.tsxsrc/components/launch/NotesToolbar.tsxsrc/components/launch/NotesWindow.test.tsxsrc/components/launch/NotesWindow.tsx
The lock was applied in an effect, which runs only after the first commit - with a persisted mirrored setting the first painted frame was briefly editable. The editor is now created with editable: !editingLocked, and the effect keeps handling every later transition. The mount test asserts the creation-time option as well.
The teleprompter suite stubs @tiptap/react wholesale, so its read-only assertions pin the props handed to Tiptap rather than what Tiptap does with them. useEditor honours `editable` at creation and then pins every later option pass to the editor own isEditable, which leaves setEditable as the single path carrying a mirror toggle. Losing it - to a refactor or a Tiptap upgrade - would leave every mocked assertion green with a mirrored note still editable, the exact bug this branch fixes. NotesWindow.editable.test.tsx drives the real editor through a mirror toggle and reads contenteditable back off the DOM; removing the setEditable effect fails it. The constructor option stays pinned at the mocked boundary, since React flushes the effect before a test can observe the first paint - the comment there now says so rather than claiming the mock proves creation-time behaviour. Also stop the playback-label test from rendering isPlaying with formatting still enabled, a state NotesWindow cannot produce.
Summary
Follow-up to #152. Mirrored Notes content stayed editable, but while the
mirror is on, caret placement and text selection are horizontally reversed on
screen — editing in that state is not usable. This treats mirrored mode as
presentation-only:
editingLockedstate, and editing returns only when playback is paused andthe mirror is off.
isPlayingalone.Related issue
Follow-up to #152, where the reversed caret placement and selection were
flagged in review. Related to #67.
Type of change
Release impact
Desktop impact
Screenshots / video
Not included.
Testing
the full unit suite passes 1500 tests, with the 6 known Windows-only
webm-seek-indexfailures also failing identically on a clean upstreammaincheckout on the same machine (CI runs them green on Linux).npx tsc --noEmitandnpx tsc -p tsconfig.test.json --noEmitare bothclean; Biome is clean on the changed files;
npm run i18n:checkpasses;npm run build-vitepasses;git diff --checkis clean.removing the playing lock fails the playback read-only test, and bypassing
the toolbar's lock prop fails the formatting-lock test — none of the new
assertions are vacuous.
(playing x mirrored): caret placement, drag selection, typing, and deletion
work only while paused and unmirrored; while mirrored, typing, drag
selection, select-all, Backspace, and Delete leave the content
byte-identical; the mirror transform applies to the content only, never the
toolbar; the teleprompter controls stay operable while locked; a persisted
mirror locks the note from the first render without starting playback; the
control rows stay reachable in a 400px-wide layout; and auto-scroll,
stop-at-end, replay-from-bottom, and minimum-speed accumulation behave as
before (scroll motion exercised with deterministic
requestAnimationFrametimestamp stepping, not real-time VSync).
No persistence schema, IPC, native, recorder, modal, banner, or new mode
changes.
Summary by CodeRabbit