WARNING: THIS SITE IS A MIRROR OF GITHUB.COM / IT CANNOT LOGIN OR REGISTER ACCOUNTS / THE CONTENTS ARE PROVIDED AS-IS / THIS SITE ASSUMES NO RESPONSIBILITY FOR ANY DISPLAYED CONTENT OR LINKS / IF YOU FOUND SOMETHING MAY NOT GOOD FOR EVERYONE, CONTACT ADMIN AT ilovescratch@foxmail.com
Skip to content

Conversation

@dskamiotis
Copy link
Contributor

@dskamiotis dskamiotis commented Sep 27, 2024

What does this change?

Converts the Fabric Video template to Svelte.

Its important to note, we need to perform setup logic after the component renders.

  • Using the IntersectionObserver, video play logic, depends on the DOM being fully initialised, meant we needed to use the subscribe to subscribe to a store, so we can perform actions whenever the value of the store changes.
  • ad.json removed from legacy fabric video to prevent it being detected by GAM.

How to test

How can we measure success?

We can further check the functionality of the pause and the play methods by adding console.logs in the "observer" if and navigate the viewport in Preview so the fabric video is out of view.

We have added the IntersectionObserver, which is responsible for detecting when the video element enters the viewport and playing or pausing the video accordingly. Updating the previous functions in the legacy code.

Have we considered potential risks?

Images

Screen.Recording.2024-10-21.at.12.40.58.mov

Accessibility

co-authored: @Jakeii

@dskamiotis dskamiotis requested review from Jakeii and arelra September 27, 2024 13:12
@github-actions
Copy link

github-actions bot commented Sep 27, 2024

Visual regression testing results 🔍

If any tests are failing, please check that any visual changes are intentional before merging your PR.

Template Visual test status
CAPI Multiple Hosted
CAPI Multiple Paidfor
Fabric Custom
Fabric
Manual Multiple
Manual Single

@dskamiotis dskamiotis force-pushed the migrate-fabric-video-to-svelte branch from 2463675 to 9df688e Compare September 27, 2024 13:16
@dskamiotis dskamiotis force-pushed the migrate-fabric-video-to-svelte branch 2 times, most recently from a1adcba to 60b0633 Compare October 17, 2024 09:05
@dskamiotis dskamiotis force-pushed the migrate-fabric-video-to-svelte branch from e2c07b1 to 9f80b57 Compare October 28, 2024 11:43
@dskamiotis dskamiotis force-pushed the migrate-fabric-video-to-svelte branch from 9f80b57 to fa4fd96 Compare October 28, 2024 14:36
@dskamiotis dskamiotis mentioned this pull request Oct 28, 2024
4 tasks
@dskamiotis dskamiotis force-pushed the migrate-fabric-video-to-svelte branch from 8ee21bd to e2b7531 Compare November 13, 2024 11:08
Copy link
Member

@Jakeii Jakeii left a comment

Choose a reason for hiding this comment

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

Looks good!

@dskamiotis dskamiotis merged commit 30c6660 into main Nov 14, 2024
9 checks passed
@dskamiotis dskamiotis deleted the migrate-fabric-video-to-svelte branch November 14, 2024 09:28
@SiAdcock SiAdcock requested a review from a team November 15, 2024 12:28
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.

4 participants