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

Lit enablement #272

Merged
merged 3 commits into from Feb 14, 2023
Merged

Lit enablement #272

merged 3 commits into from Feb 14, 2023

Conversation

0xcadams
Copy link
Member

Description

Added features for WIP Lit protocol integration.

@0xcadams 0xcadams requested a review from a team as a code owner February 14, 2023 19:35
@vercel
Copy link

vercel bot commented Feb 14, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
livepeerjs-with-next ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 14, 2023 at 7:55PM (UTC)
livepeerjs-with-next-13 ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 14, 2023 at 7:55PM (UTC)
livepeerjs-with-svelte ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 14, 2023 at 7:55PM (UTC)

Copy link
Member

@victorges victorges left a comment

Choose a reason for hiding this comment

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

LGTM! Only some totally optional ideas

playsInline
muted={muted}
crossOrigin={
allowCrossOriginCredentials ? 'use-credentials' : 'anonymous'
Copy link
Member

Choose a reason for hiding this comment

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

Is anonymous the default? Either way, what happens if we set it as undefined instead to leave it for the browser to decide the default?

Comment on lines +123 to +125
xhrSetup(xhr, _url) {
xhr.withCredentials = Boolean(allowCrossOriginCredentials);
},
Copy link
Member

Choose a reason for hiding this comment

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

WDYT of only setting this if the prop is set?

Suggested change
xhrSetup(xhr, _url) {
xhr.withCredentials = Boolean(allowCrossOriginCredentials);
},
xhrSetup: !allowCrossOriginCredentials : undefined : (xhr, _url) => {
xhr.withCredentials = true
},

@@ -154,6 +160,9 @@ export const VideoPlayer = React.forwardRef<HTMLVideoElement, VideoPlayerProps>(
muted={muted}
poster={typeof poster === 'string' ? poster : undefined}
preload={priority ? 'auto' : 'metadata'}
crossOrigin={
allowCrossOriginCredentials ? 'use-credentials' : 'anonymous'
Copy link
Member

Choose a reason for hiding this comment

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

Same (low pri) comment about the default

@0xcadams 0xcadams merged commit b11ea90 into main Feb 14, 2023
@0xcadams 0xcadams deleted the 0xcadams/lit branch February 14, 2023 20:12
@github-actions github-actions bot mentioned this pull request Feb 14, 2023
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.

None yet

2 participants