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 Video Blocks in non Chromium browsers #250

Merged
merged 1 commit into from
Jan 16, 2025

Conversation

decrek
Copy link
Member

@decrek decrek commented Jan 16, 2025

Changes

add feature check for navigator.connection since it is chromium only making video blocks fail outside of chromium browsers

Associated issue

Resolves #249

How to test

Test Video Block in non Chromium browser

Checklist

  • I have performed a self-review of my own code
  • I have made sure that my PR is easy to review (not too big, includes comments)
  • I have made updated relevant documentation files (in project README, docs/, etc)
  • I have added a decision log entry if the change affects the architecture or changes a significant technology
  • I have notified a reviewer

…is chromium only making video blocks fail outside of chromium browsers
@decrek decrek requested a review from jbmoelker January 16, 2025 11:50
@decrek decrek changed the title Fix Video Blocks in non CHromium browsers Fix Video Blocks in non Chromium browsers Jan 16, 2025
Copy link

Deploying head-start with  Cloudflare Pages  Cloudflare Pages

Latest commit: ef91768
Status: ✅  Deploy successful!
Preview URL: https://efc01ba8.head-start.pages.dev
Branch Preview URL: https://fix-video-block-fails-outsid.head-start.pages.dev

View logs

Copy link
Contributor

@jurgenbelien jurgenbelien left a comment

Choose a reason for hiding this comment

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

I would personally write the whole function as:

#useDataSaveMode = (() => {
  const navigator = window.navigator as Navigator & { connection?: { saveData: boolean } }
  return navigator.connection?.saveData || false; 
})()

But the outcome is the same.

Copy link
Member

@jbmoelker jbmoelker left a comment

Choose a reason for hiding this comment

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

❤️

@decrek decrek merged commit e0cbec4 into main Jan 16, 2025
5 checks passed
@decrek decrek deleted the fix/video-block-fails-outside-chrome branch January 16, 2025 12:31
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.

VideoBlock uses navigator.connection which might not be available in every browser
3 participants