-
-
Notifications
You must be signed in to change notification settings - Fork 140
feat: Improve slide deck mobile responsiveness and speaker notes customization #1925
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Auto-enter fullscreen mode on mobile/tablet devices (touch + width ≤ 1024px) - Detect device orientation and screen dimensions for responsive behavior - Remove forced dark mode styling; fullscreen now respects current theme - Add responsive breakpoints for tablet (996px) and mobile (768px) - Implement viewport-based scaling for text and images on mobile - Maintain 2-column split layouts on mobile with scaled content - Increase z-index to 99999 to prevent navbar overlap in fullscreen - Improve padding and spacing for mobile screens - Use clamp() with viewport units (vw) for fluid typography 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Haiku 4.5 <[email protected]>
Dependency Review✅ No vulnerabilities or license issues found.Scanned FilesNone |
- Remove max-width constraint (1600px) on fullscreen slide wrapper - Use viewport-based sizing to fill entire screen while maintaining 16:9 - Scale slide content width from 800px to 85-90% in fullscreen - Add clamp() with vw units for text scaling in fullscreen: - Titles scale from 2.5rem to 4rem (4vw) - Title slides scale from 3.5rem to 5.5rem (5vw) - Content/lists scale from 1.25rem to 2rem (2vw) - Code scales from 0.9rem to 1.3rem (1.2vw) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Changes overflow from hidden to overflow-y: auto so YAML code blocks and other long content can be scrolled within the slide. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Remove 16:9 aspect ratio constraint on mobile so the slide background extends to fill the entire screen instead of showing black bars. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
…ansparent Make all fullscreen containers transparent so the slide's background extends to fill the entire viewport without visible borders. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Use solid background color instead of transparent to hide page behind - Add fixed positioning for toolbar at bottom of screen - Add fixed positioning for nav buttons with semi-transparent background - Add padding-bottom to slide content to avoid toolbar overlap 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
The left-area container was taking up space in the flex layout even though the nav buttons were fixed positioned, causing a dark strip on the left side of the slide. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Changed container align-items from stretch to center - Added flexbox centering to slide-wrapper - Changed slide height from 100% to auto with min-height: 100% - Added explicit flexbox centering to slide element 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Instead of hiding the left-area container completely (which also hides the prev button), collapse it to width: 0 but keep overflow: visible so the fixed-positioned nav button still renders. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Changed slide height back to 100% (from auto with min-height) - Added explicit centering override for content layout slides - Keep text-align: left for content readability 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Changed slide from flexbox to block display to allow overflow scrolling - Moved vertical centering to slide__inner using min-height + flexbox - margin: auto centers content when it's shorter than viewport - Long content can now scroll properly 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
|
Warning Rate limit exceeded@osterman has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 9 minutes and 54 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughAdds configurable, persistent speaker-notes (position, display mode, popout) with a synchronized popout window; introduces a TTS hook and player UI; large responsive/fullscreen refactor for the slide deck; a post-build slide-notes extractor plugin; and a single MDX image width tweak. Changes
Sequence Diagram(s)sequenceDiagram
participant Main as Main Window
participant SlideDeck as SlideDeck UI
participant Context as SlideDeckContext
participant Channel as BroadcastChannel
participant Popout as Notes Popout Window
participant TTS as useTTS / Audio
Note over Main,SlideDeck: App mounts and initializes
SlideDeck->>Context: load notes prefs, determine isMobile
Context-->>SlideDeck: notesPreferences + isMobile
alt Open notes popout
SlideDeck->>Context: setNotesPopout(true)
SlideDeck->>Popout: window.open() + write initial HTML
SlideDeck->>Channel: post {init, slide, notes}
Popout->>Channel: subscribe
Channel->>Popout: deliver init & updates
Popout->>Channel: post {navigate-next/prev, close}
Channel->>SlideDeck: deliver commands
SlideDeck->>Context: handle navigation / close
end
alt TTS playback
SlideDeck->>TTS: play(slide)
TTS-->>SlideDeck: playing, progress, ended
SlideDeck->>Context: onEnded may advance slide
SlideDeck->>Channel: broadcast slide changes (if popout open)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
- Removed flexbox from slide-wrapper (was preventing scroll) - Used absolute positioning on slide to fill container - Slide now has fixed dimensions and can scroll content 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
…croll - Use 'safe center' which centers when content fits, aligns to start when overflow - Keep flexbox display for proper centering - Remove conflicting display: block from Slide.css 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Split the 2-second delay between slides into two parts: - 1 second after the current slide's audio ends - 1 second before the next slide's audio starts This provides a more balanced pause that gives time for both the current slide to sink in and for the visual transition to the next slide before audio begins. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Rename AUTO_ADVANCE_DELAY_AFTER/BEFORE to AUTO_ADVANCE_DELAY and AUTO_PLAY_DELAY for clearer semantics: - AUTO_ADVANCE_DELAY: delay before advancing to next slide - AUTO_PLAY_DELAY: delay before starting audio on new slide 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Add isAutoPlaying state to track auto-play mode for UI updates. Previously, the TTSPlayer bar would disappear during the 1-second delay between slides because neither isPlaying nor isPaused was true. Now the bar stays visible when navigating via the drawer or during auto-advance transitions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
The play button now shows a loading spinner during the delay between slides when in auto-play mode. Previously it would briefly show the play icon which was jarring. Changes: - Always show the TTS button (not conditional on currentNotes) - Show spinner when isAutoPlaying but not yet playing/paused - Button stays active during auto-play transitions 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
iOS Safari blocks audio playback that isn't directly triggered by user interaction. Creating a new Audio() element for each slide broke the user-activation chain, causing "request is not allowed by the user agent" errors on mobile. Fix: Reuse a single persistent Audio element and update its src property instead of creating new elements. This preserves the user-activation state established on the first tap. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (4)
website/src/components/SlideDeck/TTSPlayer.tsx (2)
23-30: Consider deriving VOICES from TTSVoice type.The voice values here duplicate those in the
TTSVoicetype definition. If new voices are added to the type, this array won't update automatically.One option: use a
satisfiesassertion or derive from a shared source to keep them in sync.
112-118: Exit animation won't trigger when parent unmounts.
AnimatePresenceneeds the wrapped component to be conditionally rendered inside it for exit animations. SinceTTSPlayeris conditionally mounted from the parent (SlideDeck.tsx lines 414-435), the exit animation at line 118 won't fire—component unmounts beforeAnimatePresencecan animate out.If you want the exit animation, move the
AnimatePresencewrapper to the parent.website/src/components/SlideDeck/SlideDeck.tsx (1)
419-433: Consider extracting these callbacks.The pause/resume logic here duplicates
handleTTSPlayPause. ExtractinghandleTTSPauseandhandleTTSResumeas named callbacks would reduce duplication and make the JSX cleaner.🔎 Suggested extraction
+ const handleTTSPause = useCallback(() => { + autoPlayRef.current = false; + setIsAutoPlaying(false); + if (autoAdvanceTimerRef.current) { + clearTimeout(autoAdvanceTimerRef.current); + autoAdvanceTimerRef.current = null; + } + tts.pause(); + }, [tts]); + + const handleTTSResume = useCallback(() => { + autoPlayRef.current = true; + setIsAutoPlaying(true); + tts.resume(); + }, [tts]);Then in TTSPlayer:
<TTSPlayer tts={tts} currentSlide={currentSlide} onStop={handleStop} - onPause={() => { ... }} - onResume={() => { ... }} + onPause={handleTTSPause} + onResume={handleTTSResume} />website/src/components/SlideDeck/useTTS.ts (1)
161-175: External fetch has no timeout.If the TTS API is slow or hangs, users will see an indefinite loading state. Consider adding an
AbortControllerwith a reasonable timeout (e.g., 30s).🔎 Example with AbortController
try { const textUrl = getTextUrl(slideNumber); const apiUrl = `https://cloudposse.com/api/tts?url=${encodeURIComponent(textUrl)}&voice=${voice}`; + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), 30000); + - const response = await fetch(apiUrl); + const response = await fetch(apiUrl, { signal: controller.signal }); + clearTimeout(timeoutId); + if (!response.ok) {And in catch:
} catch (err) { setIsLoading(false); setIsPlaying(false); - setError(err instanceof Error ? err.message : 'Unknown error'); + if (err instanceof Error && err.name === 'AbortError') { + setError('Request timed out'); + } else { + setError(err instanceof Error ? err.message : 'Unknown error'); + } }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
website/plugins/slide-notes-extractor/index.jswebsite/src/components/SlideDeck/SlideDeck.tsxwebsite/src/components/SlideDeck/TTSPlayer.tsxwebsite/src/components/SlideDeck/useTTS.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- website/plugins/slide-notes-extractor/index.js
🧰 Additional context used
📓 Path-based instructions (1)
website/**
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
website/**: Update website documentation in thewebsite/directory when adding new features, ensure consistency between CLI help text and website documentation, and follow the website's documentation structure and style
Keep website code in thewebsite/directory, follow the existing website architecture and style, and test website changes locally before committing
Keep CLI documentation and website documentation in sync and document new features on the website with examples and use cases
Files:
website/src/components/SlideDeck/useTTS.tswebsite/src/components/SlideDeck/TTSPlayer.tsxwebsite/src/components/SlideDeck/SlideDeck.tsx
🧬 Code graph analysis (2)
website/src/components/SlideDeck/useTTS.ts (1)
website/src/components/SlideDeck/index.tsx (3)
TTSVoice(54-54)UseTTSReturn(54-54)useTTS(30-30)
website/src/components/SlideDeck/TTSPlayer.tsx (1)
website/src/components/SlideDeck/useTTS.ts (2)
UseTTSReturn(10-32)TTSVoice(3-3)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Summary
🔇 Additional comments (9)
website/src/components/SlideDeck/TTSPlayer.tsx (2)
87-103: Keyboard navigation looks good.The progress bar now supports ArrowLeft/Right for ±5s seeking plus Home/End for boundaries. Clean implementation.
44-64: Solid component structure.Clean separation of handlers, proper accessibility attributes, and good use of tooltips. The destructuring keeps the code readable.
Also applies to: 112-218
website/src/components/SlideDeck/SlideDeck.tsx (4)
106-114: Verify the intentional missing dependencies.The eslint-disable is likely intentional—you only want this effect on
currentSlidechanges, not whentts.playorcurrentNoteschange. Worth a brief inline comment explaining this to prevent future "cleanup."
52-55: Deck name extraction is reasonable.The fallback to
'unknown'prevents runtime failures. Just note that URLs like/slides/foo/bar/bazwould yield'foo', which may or may not be intended.
253-260: New keyboard shortcuts integrated correctly.P and M mappings are clean, dependencies updated. Good.
376-391: TTS button handles edge states well.The loader shows during slide transitions while auto-playing, and the disabled logic correctly accounts for both notes presence and auto-play state.
website/src/components/SlideDeck/useTTS.ts (3)
76-106: Good iOS audio element pattern.Reusing a single Audio element maintains user-activation state—smart approach for mobile Safari. The
onEndedRefpattern keeps the callback current without recreating the element.
55-63: Preference persistence is SSR-safe.The window checks and default merging handle edge cases well.
Also applies to: 108-114
231-243: Stop properly releases resources.Clearing
audio.srcand resetting state is the right approach. Clean.
Start the TTS API call immediately when a slide changes, running it in parallel with the AUTO_PLAY_DELAY. This way the delay is: max(delay, api_call_time) instead of: delay + api_call_time Added prefetch() function to useTTS that returns a playPrefetched() function, allowing the fetch and delay to run concurrently. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Add background prefetching of n+1 slide audio to eliminate API latency between slides during auto-play. Changes: - Add prefetch cache (Map keyed by slide+voice) - Add prefetchInBackground() for silent background fetching - Update play() and prefetch() to check cache first - Trigger background prefetch when audio starts playing Now while slide N plays, slide N+1 audio is fetched in parallel. When advancing, the cached audio is used immediately. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
The resume callback was calling audio.play() without handling the returned Promise, which could lead to unhandled rejections when autoplay is blocked or other playback errors occur. Now properly chains .then/.catch to update state appropriately on success or failure. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Address issues in mobile Safari portrait mode: - Use dvh units to account for dynamic browser UI (URL bar) - Add safe-area-inset padding for notched devices - Reduce font sizes for narrow portrait viewports - Stack split layouts vertically in portrait - Align content to top instead of center to prevent overlap 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Reverted to centered vertical alignment for slides in portrait mode. The previous top-alignment looked unbalanced for shorter content. Content will scroll if it overflows. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
|
These changes were released in v1.203.0. |
what
Mobile Responsiveness
Speaker Notes Customization
Technical Improvements
popup=yesto window.open() for better browser compatibilitywhy
Mobile users need fullscreen viewing by default without manual toggling. Content should remain readable at all viewport sizes while preserving intended layout structure (2-column splits don't stack vertically). Theme consistency improves visual experience across devices.
Speaker notes customization addresses different presenter preferences:
references
Addresses mobile responsiveness for slide deck presentations with improved handling of landscape phones and split layouts.
Summary by CodeRabbit
New Features
Style
Other
✏️ Tip: You can customize this high-level summary in your review settings.