feat: add Notes teleprompter mode#152
Open
My-Denia wants to merge 2 commits into
Open
Conversation
📝 WalkthroughWalkthroughAdds teleprompter mode to the notes window with persisted speed, font size, and mirror settings; animation-driven scrolling; expanded toolbar controls; legacy note migration; presentation styling; localized labels; and unit, component, and browser accessibility tests. ChangesNotes teleprompter
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant NotesToolbar
participant NotesWindow
participant TipTapEditor
participant requestAnimationFrame
NotesToolbar->>NotesWindow: Toggle playback or change settings
NotesWindow->>requestAnimationFrame: Schedule playback tick
requestAnimationFrame->>NotesWindow: Return frame timestamp
NotesWindow->>TipTapEditor: Update scroll position
NotesWindow->>NotesToolbar: Render current playback state
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
src/components/launch/NotesToolbar.browser.test.tsx (2)
56-77: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStub editor is recreated on every render.
createEditor()runs in the render body, so each state update passes a new object identity anduseEditorRevision's effect (deps: [editor]) re-runsoff/onon every click. Hold it stable so the harness matches real usage.♻️ Proposed fix
function ToolbarHarness() { + const [editor] = useState(createEditor); const [isPlaying, setIsPlaying] = useState(false); const [speed, setSpeed] = useState(40); const [fontSize, setFontSize] = useState(16); const [mirrored, setMirrored] = useState(false); return ( <NotesToolbar - editor={createEditor()} + editor={editor}🤖 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/components/launch/NotesToolbar.browser.test.tsx` around lines 56 - 77, Update ToolbarHarness to create the editor once and preserve its identity across state updates, using a stable hook or equivalent initialization outside the render-time state changes. Pass that stable editor to NotesToolbar instead of calling createEditor() directly during each render, so useEditorRevision does not repeatedly rebind listeners after clicks.
116-129: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePositional assumptions make this brittle.
The tab loop requires DOM order to equal tab order and every control to be enabled — it only holds because the harness starts at speed 40 / font 16. Likewise
controls.at(-1)assumes mirror is rendered last. Selecting byaria-labelwould survive reordering and bound changes.🤖 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/components/launch/NotesToolbar.browser.test.tsx` around lines 116 - 129, The NotesToolbar focus test relies on DOM positions and ordering that change with control state and layout. Update the test to identify controls by their aria-labels, and verify tab/focus behavior using the expected labeled controls rather than controls array indices; select the mirror control by its label instead of controls.at(-1), while preserving the Pause and aria-pressed assertions.src/components/launch/NotesToolbar.test.tsx (2)
54-69: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider covering the active states too.
createPropsonly ever rendersisPlaying: false/mirrored: false. A case with bothtruewould pin the Pause label andaria-pressed="true"in the jsdom suite rather than relying solely on the browser test.As per coding guidelines, "Add a test for every new behavior in the same package as the code under test."
🤖 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/components/launch/NotesToolbar.test.tsx` around lines 54 - 69, Add a NotesToolbar test using createProps with both isPlaying and mirrored set to true, and assert the active-state behavior: the Pause label and aria-pressed="true". Keep the existing inactive-state coverage unchanged.Source: Coding guidelines
12-52: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winToolbar test fixtures are copy-pasted between the two suites. The
createEditorstub and the i18n label map are byte-identical in both files; when a label key or a TipTap command is added, both copies must be edited or one suite silently drifts. Extract them into a shared fixture module (e.g.notesToolbarTestFixtures.tsnext to the component).
src/components/launch/NotesToolbar.test.tsx#L12-L52: move the label map andcreateEditorinto the shared fixture and import them here.src/components/launch/NotesToolbar.browser.test.tsx#L10-L54: import the same fixture instead of redeclaring the mock and stub.🤖 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/components/launch/NotesToolbar.test.tsx` around lines 12 - 52, Extract the shared i18n label map and createEditor fixture from src/components/launch/NotesToolbar.test.tsx lines 12-52 into a shared notesToolbarTestFixtures module next to the component, then import and use that fixture here. Update src/components/launch/NotesToolbar.browser.test.tsx lines 10-54 to import the same shared fixture and remove its duplicate mock and editor stub declarations.src/components/launch/NotesToolbar.tsx (1)
208-219: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the duplicate toggle state signaling.
ToolbarButtonalways emitsaria-pressed={active}, so this Play/Pause teleprompter control reads as both “Pause” and pressed. Keep a static stateless label like “Auto-scroll” witharia-pressed, or keep the label swap and droparia-pressed.🤖 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/components/launch/NotesToolbar.tsx` around lines 208 - 219, Update the teleprompter ToolbarButton using active={isPlaying} and onTogglePlaying so it does not simultaneously expose a changing Play/Pause label and aria-pressed state; either use a static “Auto-scroll” label while retaining active/aria-pressed, or preserve the label swap and remove the active prop.
🤖 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/NotesToolbar.tsx`:
- Around line 235-241: Update both readouts in NotesToolbar: remove their
aria-label and redundant aria-live attributes so the dynamic values are
announced directly, relying on the parent group labels. Add localized unit keys
to launch.json for all 13 locales, and use the existing
translation/locale-formatting mechanism to render the speed and font-size values
with translated units instead of hardcoded “px/s” and “px”.
---
Nitpick comments:
In `@src/components/launch/NotesToolbar.browser.test.tsx`:
- Around line 56-77: Update ToolbarHarness to create the editor once and
preserve its identity across state updates, using a stable hook or equivalent
initialization outside the render-time state changes. Pass that stable editor to
NotesToolbar instead of calling createEditor() directly during each render, so
useEditorRevision does not repeatedly rebind listeners after clicks.
- Around line 116-129: The NotesToolbar focus test relies on DOM positions and
ordering that change with control state and layout. Update the test to identify
controls by their aria-labels, and verify tab/focus behavior using the expected
labeled controls rather than controls array indices; select the mirror control
by its label instead of controls.at(-1), while preserving the Pause and
aria-pressed assertions.
In `@src/components/launch/NotesToolbar.test.tsx`:
- Around line 54-69: Add a NotesToolbar test using createProps with both
isPlaying and mirrored set to true, and assert the active-state behavior: the
Pause label and aria-pressed="true". Keep the existing inactive-state coverage
unchanged.
- Around line 12-52: Extract the shared i18n label map and createEditor fixture
from src/components/launch/NotesToolbar.test.tsx lines 12-52 into a shared
notesToolbarTestFixtures module next to the component, then import and use that
fixture here. Update src/components/launch/NotesToolbar.browser.test.tsx lines
10-54 to import the same shared fixture and remove its duplicate mock and editor
stub declarations.
In `@src/components/launch/NotesToolbar.tsx`:
- Around line 208-219: Update the teleprompter ToolbarButton using
active={isPlaying} and onTogglePlaying so it does not simultaneously expose a
changing Play/Pause label and aria-pressed state; either use a static
“Auto-scroll” label while retaining active/aria-pressed, or preserve the label
swap and remove the active prop.
🪄 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: f423f790-e66c-4f39-aa80-533f0695f1a9
📒 Files selected for processing (21)
src/components/launch/NotesToolbar.browser.test.tsxsrc/components/launch/NotesToolbar.test.tsxsrc/components/launch/NotesToolbar.tsxsrc/components/launch/NotesWindow.module.csssrc/components/launch/NotesWindow.test.tsxsrc/components/launch/NotesWindow.tsxsrc/components/launch/notesTeleprompter.test.tssrc/components/launch/notesTeleprompter.tssrc/i18n/locales/ar/launch.jsonsrc/i18n/locales/en/launch.jsonsrc/i18n/locales/es/launch.jsonsrc/i18n/locales/fr/launch.jsonsrc/i18n/locales/it/launch.jsonsrc/i18n/locales/ja-JP/launch.jsonsrc/i18n/locales/ko-KR/launch.jsonsrc/i18n/locales/pt-BR/launch.jsonsrc/i18n/locales/ru/launch.jsonsrc/i18n/locales/tr/launch.jsonsrc/i18n/locales/vi/launch.jsonsrc/i18n/locales/zh-CN/launch.jsonsrc/i18n/locales/zh-TW/launch.json
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.
Summary
This PR adds a manual teleprompter mode to the existing Notes window.
mirroring the toolbar.
playback paused.
component, and browser coverage.
Speech-adaptive scrolling, speech recognition, and speed presets are optional
enhancements and are intentionally not included in this MVP.
Related issue
Fixes #67
Type of change
Release impact
Desktop impact
Screenshots / video
Not included.
Testing
npx tsc --noEmitnpm run build-vitegit diff --checkThe project-wide
npm run i18n:checkstill reports 259 pre-existingcross-locale discrepancies. The discrepancy set is identical to the base
commit, and dedicated validation confirms that all 11 Notes teleprompter
tooltip and unit keys are present and non-empty in all 13 launch locales, with
both unit templates retaining their
{{value}}placeholder.End-to-end tests, native capture checks, and full Electron packaging were not
run because this change is limited to the renderer Notes window.
Summary by CodeRabbit
New Features
Bug Fixes
Tests