Skip to content

fix(notes): make mirrored teleprompter content read-only - #228

Merged
EtienneLescot merged 4 commits into
getopenscreen:mainfrom
My-Denia:feat/notes-mirror-readonly
Aug 3, 2026
Merged

fix(notes): make mirrored teleprompter content read-only#228
EtienneLescot merged 4 commits into
getopenscreen:mainfrom
My-Denia:feat/notes-mirror-readonly

Conversation

@My-Denia

@My-Denia My-Denia commented Aug 3, 2026

Copy link
Copy Markdown

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:

  • The note body locks while playing or mirrored, through a single derived
    editingLocked state, and editing returns only when playback is paused and
    the mirror is off.
  • The formatting buttons disable whenever the body is locked.
  • Play/pause, speed, font size, and mirror controls stay usable while locked.
  • The play button's icon and label still track isPlaying alone.
  • Mirroring never starts playback, and pausing never lifts the mirror.

Related issue

Follow-up to #152, where the reversed caret placement and selection were
flagged in review. Related to #67.

Type of change

  • Bug fix
  • Feature
  • Enhancement
  • Documentation
  • Refactor / maintenance
  • Performance
  • Security

Release impact

  • Patch
  • Minor
  • Major / breaking change
  • No release note needed

Desktop impact

  • Windows
  • macOS
  • Linux
  • Installer / packaging
  • Not platform-specific

Screenshots / video

Not included.

Testing

  • Committed lanes at the PR head: the focused Notes suite passes (76 tests);
    the full unit suite passes 1500 tests, with the 6 known Windows-only
    webm-seek-index failures also failing identically on a clean upstream
    main checkout on the same machine (CI runs them green on Linux).
  • npx tsc --noEmit and npx tsc -p tsconfig.test.json --noEmit are both
    clean; Biome is clean on the changed files; npm run i18n:check passes;
    npm run build-vite passes; git diff --check is clean.
  • Mutation checks: removing the mirror lock fails the mirrored-state tests,
    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.
  • Driven manually in a real Chrome window through the full four-state matrix
    (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 requestAnimationFrame
    timestamp stepping, not real-time VSync).

No persistence schema, IPC, native, recorder, modal, banner, or new mode
changes.

Summary by CodeRabbit

  • Bug Fixes
    • Note editing and formatting controls are now locked during teleprompter playback or mirrored presentation mode.
    • Teleprompter controls remain available, allowing playback, mirroring, speed, and font-size adjustments without changing note content.
    • Improved transitions between playback and mirroring, including restored mirrored settings.
    • Notes now reliably unlock only after both playback and mirroring have ended.

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).
@My-Denia
My-Denia requested a review from EtienneLescot as a code owner August 3, 2026 04:54
Copilot AI review requested due to automatic review settings August 3, 2026 04:54
@coderabbitai

coderabbitai Bot commented Aug 3, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: e7a970b1-e5b0-4f00-ad4d-32449a37733c

📥 Commits

Reviewing files that changed from the base of the PR and between 54ba303 and a551545.

📒 Files selected for processing (3)
  • src/components/launch/NotesToolbar.test.tsx
  • src/components/launch/NotesWindow.editable.test.tsx
  • src/components/launch/NotesWindow.test.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/components/launch/NotesToolbar.test.tsx
  • src/components/launch/NotesWindow.test.tsx

📝 Walkthrough

Walkthrough

Notes 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.

Changes

Notes editing lock

Layer / File(s) Summary
Lock orchestration and toolbar formatting
src/components/launch/NotesWindow.tsx, src/components/launch/NotesToolbar.tsx
NotesWindow derives the editing lock from playback or mirroring and passes it to NotesToolbar. Formatting actions use this lock, while teleprompter controls remain independent.
Lock behavior tests
src/components/launch/NotesWindow.test.tsx, src/components/launch/NotesWindow.editable.test.tsx, src/components/launch/NotesToolbar.test.tsx
Tests cover editor editability, formatting locks, mirroring, playback, restored settings, state transitions, and control availability.

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
Loading

Possibly related PRs

Suggested reviewers: etiennelescot

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly states that mirrored teleprompter notes become read-only, which is the primary change.
Description check ✅ Passed The description covers the change, issue context, impact, testing, and behavior across playback and mirroring states.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 editingLocked state (isPlaying || settings.mirrored) to drive editor editability.
  • Added a formattingDisabled prop to NotesToolbar so 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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between e383f9b and c159880.

📒 Files selected for processing (4)
  • src/components/launch/NotesToolbar.test.tsx
  • src/components/launch/NotesToolbar.tsx
  • src/components/launch/NotesWindow.test.tsx
  • src/components/launch/NotesWindow.tsx

Comment thread src/components/launch/NotesWindow.tsx Outdated
My-Denia and others added 3 commits August 3, 2026 13:05
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.
@EtienneLescot
EtienneLescot merged commit 21ea97e into getopenscreen:main Aug 3, 2026
25 of 26 checks passed
@My-Denia
My-Denia deleted the feat/notes-mirror-readonly branch August 3, 2026 07:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants