Skip to content
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: volume control showing up on iOS #7550

Merged
merged 1 commit into from
Dec 1, 2021
Merged

fix: volume control showing up on iOS #7550

merged 1 commit into from
Dec 1, 2021

Conversation

gkatsev
Copy link
Member

@gkatsev gkatsev commented Dec 1, 2021

This is a follow-up to #7514. But turns out, that we still had a timing
issue around when we were doing the check and when the volume control
was created.

Instead, we should make featuresVolumeControl not be a lazy property,
so, that we do that check as early as possible. Also, we should
default this property to false in this case, so, that we assume we
can't until we confirm we can.

Additionally, added a null check for Html5, to be extra defensive since
the timeout isn't to a player.

This is a follow-up to #7514. But turns out, that we still had a timing
issue around when we were doing the check and when the volume control
was created.

Instead, we should make `featuresVolumeControl` not be a lazy property,
so, that we do that check as early as possible. Also, we should
default this property to `false` in this case, so, that we assume we
can't until we confirm we can.

Additionally, added a null check for Html5, to be extra defensive since
the timeout isn't to a player.
@codecov
Copy link

codecov bot commented Dec 1, 2021

Codecov Report

Merging #7550 (9b54570) into main (d38806d) will decrease coverage by 0.01%.
The diff coverage is 25.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #7550      +/-   ##
==========================================
- Coverage   80.26%   80.24%   -0.02%     
==========================================
  Files         116      116              
  Lines        7321     7324       +3     
  Branches     1769     1770       +1     
==========================================
+ Hits         5876     5877       +1     
- Misses       1445     1447       +2     
Impacted Files Coverage Δ
src/js/tech/html5.js 63.30% <25.00%> (-0.20%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d38806d...9b54570. Read the comment docs.

@gkatsev gkatsev merged commit 3c21345 into main Dec 1, 2021
@gkatsev gkatsev deleted the ios-volume branch December 1, 2021 22:33
edirub pushed a commit to edirub/video.js that referenced this pull request Jun 8, 2023
This is a follow-up to videojs#7514. But turns out, that we still had a timing
issue around when we were doing the check and when the volume control
was created.

Instead, we should make `featuresVolumeControl` not be a lazy property,
so, that we do that check as early as possible. Also, we should
default this property to `false` in this case, so, that we assume we
can't until we confirm we can.

Additionally, added a null check for Html5, to be extra defensive since
the timeout isn't tied to a player.
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.

2 participants