Skip to content

feat(core): add per-slide timer and keep present mode alive when opening presenter#193

Open
ridemountainpig wants to merge 2 commits into
1weiho:mainfrom
ridemountainpig:feat/per-slide-timer
Open

feat(core): add per-slide timer and keep present mode alive when opening presenter#193
ridemountainpig wants to merge 2 commits into
1weiho:mainfrom
ridemountainpig:feat/per-slide-timer

Conversation

@ridemountainpig

@ridemountainpig ridemountainpig commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add a per-slide timer next to the overall elapsed time in both present mode and the presenter view, so you can see how long you've spent on the current slide.
  • Reset the per-slide timer during render the moment the slide index changes, so the new timestamp lands before paint.
  • Keep present mode alive when opening the presenter window from fullscreen — flip to windowed first so the fullscreenchange listener doesn't treat the popup-induced fullscreen exit as a present-mode exit (which previously stranded the presenter on slide one).
  • Propagate slideStartedAt through PresenterState and add slideElapsed / slideElapsedTime strings to all locales (en, ja, zh-cn, zh-tw).
CleanShot 2026-05-29 at 15 59 20@2x

Test plan

  • pnpm typecheck passes
  • pnpm check clean for changed files
  • Enter present mode, advance slides, confirm the per-slide timer resets each slide while the overall timer keeps running
  • From fullscreen present mode, open the presenter window and confirm present mode stays alive and in sync (not stranded on slide one)

🤖 Generated with Claude Code

Summary by CodeRabbit

Summary by CodeRabbit

  • New Features

    • Added a per-slide timer in present mode, showing “time on slide” alongside the overall elapsed presentation time in both player and presenter views.
  • Bug Fixes

    • Improved presenter window opening so present mode stays in sync when launched from fullscreen, keeping the popup properly on top and preventing it from landing on slide one.
  • Documentation

    • Added localization strings for the new per-slide timer in English, Japanese, and Chinese (Simplified and Traditional).

…ing presenter

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@vercel

vercel Bot commented Jun 2, 2026

Copy link
Copy Markdown

@ridemountainpig is attempting to deploy a commit to the Yiwei Ho Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Jun 2, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 906cde46-2234-4261-99c0-cfc697f4c4c9

📥 Commits

Reviewing files that changed from the base of the PR and between c525585 and a3a8910.

📒 Files selected for processing (8)
  • packages/core/src/app/components/player.tsx
  • packages/core/src/app/components/present/use-presenter-channel.ts
  • packages/core/src/app/routes/presenter.tsx
  • packages/core/src/locale/en.ts
  • packages/core/src/locale/ja.ts
  • packages/core/src/locale/types.ts
  • packages/core/src/locale/zh-cn.ts
  • packages/core/src/locale/zh-tw.ts
✅ Files skipped from review due to trivial changes (1)
  • packages/core/src/locale/zh-tw.ts
🚧 Files skipped from review as they are similar to previous changes (7)
  • packages/core/src/locale/types.ts
  • packages/core/src/locale/en.ts
  • packages/core/src/locale/ja.ts
  • packages/core/src/locale/zh-cn.ts
  • packages/core/src/app/components/present/use-presenter-channel.ts
  • packages/core/src/app/routes/presenter.tsx
  • packages/core/src/app/components/player.tsx

Walkthrough

This PR adds per-slide elapsed time tracking to the presenter mode UI. The Player component now maintains a per-slide timestamp synchronized with slide changes and includes it in the presenter state payload. The ElapsedClock component is refactored to accept flexible title and muting props, enabling dual-timer displays in both PresentControlBar and PresenterTopBar. Locale types and translations across four languages are extended to support the new "slide elapsed" labels. Presenter window opening behavior is centralized to ensure windowed mode before popup creation.

Changes

Per-slide timer feature

Layer / File(s) Summary
Per-slide timing state and presenter window behavior
packages/core/src/app/components/player.tsx, packages/core/src/app/components/present/use-presenter-channel.ts
Player introduces slideStartedAt and timedIndex state, synchronizing per-slide timestamps when the slide index changes. A new openPresenter callback forces windowed mode before opening the presenter popup, and presenterState now includes slideStartedAt. Window focus behavior is improved when opening the popup. PresenterState type gains slideStartedAt field.
ElapsedClock refactoring and PresentControlBar dual-timer UI
packages/core/src/app/components/present/control-bar.tsx
ElapsedClock is refactored to accept title and optional muted props instead of deriving the title internally. PresentControlBar now accepts slideStartedAt and renders two clocks—one unmuted for overall presentation time and one muted for current-slide time.
Presenter view top bar with labeled timer stats
packages/core/src/app/routes/presenter.tsx
PresenterTopBar derives and accepts slideStartedAt from presenter state, and renders two labeled timer stats using a new TimerStat component. Each stat wraps an ElapsedClock with a label displayed above the elapsed time.
Localization schema and multi-language translations
packages/core/src/locale/types.ts, packages/core/src/locale/en.ts, packages/core/src/locale/ja.ts, packages/core/src/locale/zh-cn.ts, packages/core/src/locale/zh-tw.ts
Locale type contract is extended with presenter.slideElapsed and present.slideElapsedTime string fields. English, Japanese, and Chinese (Simplified and Traditional) locales are updated with corresponding translations for the per-slide timer labels.
Release notes
.changeset/per-slide-timer.md
Changeset entry documents the new per-slide timer feature as a minor version bump and describes dual-timer rendering and fullscreen-to-popup sync behavior.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • 1weiho/open-slide#119: Both PRs modify presenter window opening behavior and Player/PresentControlBar integration; this PR adds per-slide timing while the related PR introduces windowed mode controls.
  • 1weiho/open-slide#208: Both PRs extend PresenterState type and Player/presenter-channel communication; this PR adds slideStartedAt for per-slide timers while the related PR adds step reveal state for step-aware preview synchronization.

Suggested reviewers

  • 1weiho

🐰 Per-slide timers now tick away,
Marking moments slide by slide,
Presenter windows stay on top with pride,
Translations dance in every tongue,
The presenter's view just got more fun!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the two main features introduced: a per-slide timer and presenter mode persistence when opening the presenter window.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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 and usage tips.

@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 @.changeset/per-slide-timer.md:
- Line 5: Replace the current multi-sentence changeset description for
"per-slide-timer" with a single present-tense line that describes the
user-facing change and removes rationale; e.g. "Add a per-slide timer and keep
present mode synced when opening the presenter view." Ensure the file's body
contains only that one-line summary and no extra sentences or explanation.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2ad68ca1-a577-43df-a3d2-b2e2fb954b9b

📥 Commits

Reviewing files that changed from the base of the PR and between 9b8202e and c525585.

📒 Files selected for processing (10)
  • .changeset/per-slide-timer.md
  • packages/core/src/app/components/player.tsx
  • packages/core/src/app/components/present/control-bar.tsx
  • packages/core/src/app/components/present/use-presenter-channel.ts
  • packages/core/src/app/routes/presenter.tsx
  • packages/core/src/locale/en.ts
  • packages/core/src/locale/ja.ts
  • packages/core/src/locale/types.ts
  • packages/core/src/locale/zh-cn.ts
  • packages/core/src/locale/zh-tw.ts

"@open-slide/core": minor
---

Add a per-slide timer alongside the overall elapsed time in present mode and the presenter view. Keep present mode alive when opening the presenter window from fullscreen, so the popup surfaces on top and stays in sync instead of stranding on slide one.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Shorten to one line without rationale.

The description contains two sentences and includes rationale ("so the popup surfaces on top..."), which violates the changeset guideline. As per coding guidelines, changeset descriptions must be "one line, present-tense, describing what changed from a user's perspective. No paragraphs, no rationale".

📝 Proposed fix
-Add a per-slide timer alongside the overall elapsed time in present mode and the presenter view. Keep present mode alive when opening the presenter window from fullscreen, so the popup surfaces on top and stays in sync instead of stranding on slide one.
+Add per-slide timer in present mode and presenter view, and keep present mode active when opening presenter window from fullscreen

As per coding guidelines: "Changeset descriptions must be short and direct: one line, present-tense, describing what changed from a user's perspective. No paragraphs, no rationale, no 'this PR…'."

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
Add a per-slide timer alongside the overall elapsed time in present mode and the presenter view. Keep present mode alive when opening the presenter window from fullscreen, so the popup surfaces on top and stays in sync instead of stranding on slide one.
Add per-slide timer in present mode and presenter view, and keep present mode active when opening presenter window from fullscreen
🤖 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 @.changeset/per-slide-timer.md at line 5, Replace the current multi-sentence
changeset description for "per-slide-timer" with a single present-tense line
that describes the user-facing change and removes rationale; e.g. "Add a
per-slide timer and keep present mode synced when opening the presenter view."
Ensure the file's body contains only that one-line summary and no extra
sentences or explanation.

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.

1 participant