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

DuckPlayer - Add Pixel events in refactor #3470

Closed
wants to merge 8 commits into from

Conversation

afterxleep
Copy link
Collaborator

@afterxleep afterxleep commented Oct 22, 2024

Task/Issue URL: https://app.asana.com/0/0/1208563377656004/f

Description:

  • Updates refactored DuckPlayerHander with missing pixels
  • Add tests for all relevant pixels

Steps to test this PR:
The update pixels include new unit tests, but if you want to test the implementation, follow these steps.

  • Launch the app, Set DuckPlayer to 'Always'

  • Hit Fire Button

  • Search for Metallica videos

  • Confirm duckplayer.view-from.serp is fired

  • Hit Fire Button

  • Go to youtube.com

  • Search for metallica

  • Tap a video

  • Confirm Pixel fired duckplayer.view-from.youtube.automatic is fired

  • Hit Fire Button

  • Go to https://randomtests.netlify.app/ddg/duckplayer.html

  • Tap "Regular Youtube link"

  • Confirm Pixel fired duckplayer.view-from.other is fired

@afterxleep afterxleep requested a review from Bunn October 22, 2024 20:53
Copy link
Contributor

@Bunn Bunn left a comment

Choose a reason for hiding this comment

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

I'm getting an issue when playing from the SERP. If I go to the videos vertical it doesn't trigger the video and it things it's playing from youtube. I assume this is a breaking bug so marking as "request changes".

Bug.mp4

@afterxleep
Copy link
Collaborator Author

afterxleep commented Oct 24, 2024

No, you've found a bug, and I am not able to repro. I think the intermediate visit to Images is causing the refferer to be setup incorrectly.

We can do two things:

Merge this PR as is and open a follow up task. (This is not gonna be merged to main)
Close the PR to prevent from blocking upcoming one with more pixel changes, and re-test pixels in the next one.

WDYT?

@afterxleep
Copy link
Collaborator Author

Closing as upcoming PR includes a fix. I'll add pixel test instructions there

@afterxleep afterxleep closed this Oct 24, 2024
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