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

feat(nx-dev): refactor and improve style of video-player #29749

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

juristr
Copy link
Member

@juristr juristr commented Jan 24, 2025

Removes obsolete unused parts and refactors how embedded videos are rendered

Current Behavior

image

Expected Behavior

image

@juristr juristr requested a review from a team as a code owner January 24, 2025 16:38
@juristr juristr requested a review from isaacplmann January 24, 2025 16:38
Copy link

vercel bot commented Jan 24, 2025

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

Name Status Preview Updated (UTC)
nx-dev ✅ Ready (Inspect) Visit Preview Jan 24, 2025 4:48pm

Copy link

nx-cloud bot commented Jan 24, 2025

View your CI Pipeline Execution ↗ for commit a1c522a.

Command Status Duration Result
nx affected --targets=lint,test,build,e2e,e2e-c... ✅ Succeeded 4m 45s View ↗
nx run-many -t check-imports check-commit check... ✅ Succeeded 1m 3s View ↗
nx-cloud record -- nx-cloud conformance:check ✅ Succeeded 1s View ↗
nx-cloud record -- nx format:check --base=1c8b3... ✅ Succeeded 25s View ↗
nx-cloud record -- nx sync:check ✅ Succeeded 24s View ↗
nx documentation --no-dte ✅ Succeeded 44s View ↗

☁️ Nx Cloud last updated this comment at 2025-01-24 16:49:40 UTC

src,
alt,
showControls = false,
autoPlay = true,
Copy link
Member

Choose a reason for hiding this comment

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

Can we set this to false by default? Also, on mobile we should not play at all the video.

Copy link
Member Author

@juristr juristr Jan 24, 2025

Choose a reason for hiding this comment

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

Note it only autoplays once the video player comes into your viewport. I left autoplay to true because that's the behavior we had so far. This video-player is mostly used to show some endless animations, where gif is really bad. That's why it is autoplay=true and showControls=false.

I've refactored it to allow showControls as well because I need it in the upcoming blog post where there are longer "videos" where you want the user to be able to go back / forth etc.

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