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

Apply lazy load to hosted videos #542

Merged
merged 1 commit into from
Feb 11, 2025

Conversation

yileifeng
Copy link
Member

@yileifeng yileifeng commented Feb 7, 2025

Related Item(s)

https://github.com/ramp4-pcar4/tcei-tmx-cwa-storylines/issues/172

Changes

  • [FEATURE] apply lazy load for hosted video files
  • other changes need to be made directly in Dev

Testing

Steps:

  1. Check network tab and filter for media
  2. Ensure the hosted video file (one in the dynamic slide) is not downloaded until intersected

This change is Reviewable

Copy link
Member

@spencerwahl spencerwahl left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @yileifeng)


src/components/panels/video-panel.vue line 139 at r1 (raw file):

        observer.value = new IntersectionObserver(([video]) => {
            if (video.isIntersecting) {
                console.log('LOAD VIDEO');

lurking console log

Copy link

github-actions bot commented Feb 7, 2025

Copy link
Member Author

@yileifeng yileifeng left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @spencerwahl)


src/components/panels/video-panel.vue line 139 at r1 (raw file):

Previously, spencerwahl (Spencer Wahl) wrote…

lurking console log

removed

Copy link
Member

@RyanCoulsonCA RyanCoulsonCA left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 2 files reviewed, 2 unresolved discussions (waiting on @spencerwahl and @yileifeng)


src/components/panels/video-panel.vue line 114 at r2 (raw file):

const videoBlobSrc = ref('');
const captionsBlobSrc = ref('');
// const transcriptBlobSrc = ref('');

This can probably be removed too if we're not using it

Copy link
Member Author

@yileifeng yileifeng left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 2 files reviewed, 2 unresolved discussions (waiting on @RyanCoulsonCA and @spencerwahl)


src/components/panels/video-panel.vue line 114 at r2 (raw file):

Previously, RyanCoulsonCA (Ryan Coulson) wrote…

This can probably be removed too if we're not using it

removed

Copy link
Member

@RyanCoulsonCA RyanCoulsonCA left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 2 files at r1, 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @spencerwahl)

Copy link
Member

@spencerwahl spencerwahl left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 2 files at r1, 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @yileifeng)

Copy link
Member

@spencerwahl spencerwahl left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @yileifeng)

@spencerwahl spencerwahl merged commit e33022e into ramp4-pcar4:main Feb 11, 2025
3 checks passed
@yileifeng yileifeng deleted the lazy-load-video branch February 11, 2025 15:30
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.

3 participants