Repository navigation
fix(web): show hours in embed player duration - #2655
Merged
richiemcilroy merged 3 commits intoOct 19, 2026
Merged
richiemcilroy merged 3 commits into
richiemcilroy merged 3 commits into
Conversation
The embed player formatted the duration with its own MM:SS helper, so anything an hour or longer rendered as a bare minute count: a 2h 3m 4s video showed "123:04", and a missing duration showed "NaN:NaN". The share page already rolls over into hours via media-chrome's time utils, so the two surfaces disagreed on the same video. Move the formatter to lib/player-time.ts with hour rollover and a non-finite guard. Output below an hour is byte-identical to before.
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
This branch was previously deployed
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.
Problem
The embed player formats the video duration with its own
MM:SShelper, so any video an hour or longer renders as a bare minute count:60:001:00:0063:451:03:45123:042:03:04600:0010:00:00NaN:NaN00:00This disagrees with the rest of the app for the same video: the share page formats time through media-chrome's
timeUtils.formatTime, and dashboard search rolls over into hours too. Only the embed didn't.Free-plan recordings are length-capped, but Pro and desktop recordings aren't, so hour-plus videos are routine.
Fix
Move the formatter to
apps/web/lib/player-time.tswith hour rollover and a non-finite guard, and point the embed at it.Output below an hour is byte-identical to before — verified by sweeping every integer from 1 to 3599 plus 20,000 random fractional values against the old implementation: zero mismatches. Only hour-plus durations and non-finite input change.
I kept this to the one call site rather than also folding in dashboard search's near-duplicate, since that one uses an unpadded style and changing it would be an unrequested behaviour change.
Verification
bun run test(apps/web) — 3447 passed, 0 failedbun run typecheck— exit 0, no errorsbun run biome check— clean on all touched files__tests__/unit/player-time.test.ts, which fail against the previous implementationI wasn't able to boot the app itself (no local Docker for MySQL/MinIO), so this is verified by the typecheck, the unit tests and both embed test suites rather than by clicking through a running embed.
The PR appears safe to merge; no actionable issues were found.Summary
The embed now uses
formatPlayerTimeto show hour-long durations asH:MM:SS.Review checks were based on the code and test configuration. Tests and the running UI were not exercised during this review.
Reviews (1) · Last reviewed commit: "fix(web): show hours in embed player dur..." · Reviewed by Greptile