-
Notifications
You must be signed in to change notification settings - Fork 56
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
fix: clean up media-time-display conditions for duration vs. seekable… #850
fix: clean up media-time-display conditions for duration vs. seekable… #850
Conversation
…End usage (display and aria label).
@cjpillsbury is attempting to deploy a commit to the Mux Team on Vercel. A member of the Team first needs to authorize it. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #850 +/- ##
==========================================
- Coverage 78.55% 72.34% -6.22%
==========================================
Files 59 79 +20
Lines 11080 15427 +4347
==========================================
+ Hits 8704 11160 +2456
- Misses 2376 4267 +1891 ☔ View full report in Codecov by Sentry. |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
I don't honestly understand what the specific problem was. Maybe Infinity
was sneaking through and causing problems? But it looks like a non problematic change.
src/js/media-time-display.js
Outdated
@@ -50,7 +55,12 @@ const DEFAULT_MISSING_TIME_PHRASE = 'video not loaded, unknown time.'; | |||
const updateAriaValueText = (el) => { | |||
const currentTime = el.mediaCurrentTime; | |||
const [, seekableEnd] = el.mediaSeekable ?? []; | |||
const endTime = el.mediaDuration || seekableEnd; | |||
let endTime; |
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.
We check for endTime == null
later on, which makes me feel like this should now either default to null, or we should check for undefined
or !endTime
later.
…End usage (display and aria label).
Mostly relevant for "DVR" cases.