Skip to content

Conversation

@ShayLevi
Copy link
Contributor

@ShayLevi ShayLevi commented Dec 10, 2024

Relevant task - https://cloudinary.atlassian.net/browse/ME-17957
This test is navigating to autoplay on scroll page (autoplay-on-scroll.html) and make sure that video is playing automatically on scroll.
First testing that the video is not playing (before scrolling), then scrolling until video element is visible and test that it is playing (checking that the video is not paused).

@ShayLevi ShayLevi requested a review from a team as a code owner December 10, 2024 12:45
@ShayLevi ShayLevi requested a review from refael-m December 10, 2024 12:45
@netlify
Copy link

netlify bot commented Dec 10, 2024

Deploy Preview for cld-video-player ready!

Name Link
🔨 Latest commit 32576dd
🔍 Latest deploy log https://app.netlify.com/sites/cld-video-player/deploys/675993ab24015f0008106a9f
😎 Deploy Preview https://deploy-preview-763--cld-video-player.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@netlify
Copy link

netlify bot commented Dec 10, 2024

Deploy Preview for cld-vp-esm-pages ready!

Name Link
🔨 Latest commit 32576dd
🔍 Latest deploy log https://app.netlify.com/sites/cld-vp-esm-pages/deploys/675993abbc6d2f000978f6f2
😎 Deploy Preview https://deploy-preview-763--cld-vp-esm-pages.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

async scrollToVideoElement() {
await this.page.locator(AUTOPLAY_ON_SCROLL_PAGE_VIDEO_SELECTOR).scrollIntoViewIfNeeded();
// Wait briefly to give time for the autoplay to apply
await this.page.waitForTimeout(1000);
Copy link
Contributor

Choose a reason for hiding this comment

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

don't use waitForTimeout, use expect.toPass.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed the waitForTimeout and used expect.toPass in test step of validating that the video is auto playing after scrolling

Copy link
Contributor

@refael-m refael-m left a comment

Choose a reason for hiding this comment

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

see comments

@ShayLevi ShayLevi requested a review from refael-m December 11, 2024 09:53
* This action ensures the autoplay behavior is triggered when the video comes into view.
*/
async scrollToVideoElement() {
await this.page.locator(AUTOPLAY_ON_SCROLL_PAGE_VIDEO_SELECTOR).scrollIntoViewIfNeeded();
Copy link
Contributor

Choose a reason for hiding this comment

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

you don't need it, you can use something like autoplayOnScrollVideoComponent. scrollIntoViewIfNeeded() in the test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added BaseComponent and modified the videoComponent to extend it. Removed the scrollToVideoElement function and used scrollIntoViewIfNeeded() in the test.

await test.step('Validating that the video is auto playing after scrolling (in case isPause is false)', async () => {
await expect(async () => {
expect(await pomPages.autoplayOnScrollPage.autoplayOnScrollVideoComponent.isPaused()).toEqual(false);
}).toPass({ intervals: [2], timeout: 1000 });
Copy link
Contributor

Choose a reason for hiding this comment

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

why did you add 2 here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed it to intervals: [500], timeout: 3000

Copy link
Contributor

@refael-m refael-m left a comment

Choose a reason for hiding this comment

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

see comments

@ShayLevi ShayLevi requested a review from refael-m December 11, 2024 13:31
@ShayLevi ShayLevi merged commit 845e178 into edge Dec 11, 2024
10 checks passed
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