Skip to content

Add ad blocking variants to v3 onboarding #1623

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
Apr 15, 2025
Merged

Add ad blocking variants to v3 onboarding #1623

merged 7 commits into from
Apr 15, 2025

Conversation

noisysocks
Copy link
Contributor

@noisysocks noisysocks commented Apr 10, 2025 •
edited
Loading

Asana Task/Github Issue: https://app.asana.com/1/137249556945/project/1209121419454298/task/1209865676528006?focus=true

Description

Enhanced Ad Blocking YouTube Ad Blocking Updated Duck Player copy
127 0 0 1_8000__platform=windows page=systemSettings steps=adBlocking(iPad Air) 127 0 0 1_8000__platform=windows page=systemSettings steps=youTubeAdBlocking(iPad Air) 127 0 0 1_8000__platform=windows page=systemSettings steps=youTubeAdBlocking(iPad Air) (1)
  • Adds an optional Enhanced Ad Blocking (ad-blocking) row to the settings step.
  • Adds an optional YouTube Ad Blocking (youtube-ad-blocking) row to the settings step.
  • Updates Duck Player step's copy when YouTube Ad Blocking is enabled.
  • Both ad blocking rows call a new setAdBlocking message.

Testing Steps

Navigate to:

Manual instructions

First, build and serve the onboarding page:

npm ci
cd special-pages
npm run watch -- --page onboarding

Then, navigate to:

Checklist

Please tick all that apply:

  • I have tested this change locally
  • I have tested this change locally in all supported browsers
  • This change will be visible to users
  • I have added automated tests that cover this change
  • I have ensured the change is gated by config
  • This change was covered by a ship review
  • This change was covered by a tech design
  • Any dependent config has been merged

@noisysocks noisysocks added enhancement New feature or request minor Increment the minor version when merged labels Apr 10, 2025
@noisysocks noisysocks requested review from shakyShane, mgurgel and a team as code owners April 10, 2025 02:10
@netlify Netlify
Copy link

netlify bot commented Apr 10, 2025 •
edited
Loading

✅ Deploy Preview for content-scope-scripts ready!

Name Link
🔨 Latest commit 72aef78
🔍 Latest deploy log https://app.netlify.com/sites/content-scope-scripts/deploys/67fee58be12c130008e4a869
😎 Deploy Preview https://deploy-preview-1623--content-scope-scripts.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.

@github-actions GitHub Actions
Copy link

github-actions bot commented Apr 10, 2025 •
edited
Loading

Temporary Branch Update

The temporary branch has been updated with the latest changes. Below are the details:

Please use the above install command to update to the latest version.

@github-actions GitHub Actions
Copy link

github-actions bot commented Apr 10, 2025 •
edited
Loading

[Beta] Generated file diff

Time updated: Tue, 15 Apr 2025 23:03:12 GMT

Integration 
    - integration/pages/onboarding/dist/index.js
  • integration/pages/onboarding/locales/en/onboarding.json

File has changed

Windows 
    - windows/pages/onboarding/dist/index.js
  • windows/pages/onboarding/locales/en/onboarding.json

File has changed

Apple 
    - dist/pages/onboarding/dist/index.js
  • dist/pages/onboarding/locales/en/onboarding.json

File has changed

mgurgel
mgurgel requested changes Apr 10, 2025
View reviewed changes
Copy link
Contributor

@mgurgel mgurgel left a comment

Choose a reason for hiding this comment

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

Nice one! Works as advertised. Just a note on the preview implementation.

@noisysocks noisysocks force-pushed the randerson/ad-blocking-in-onboarding branch from b895b1c to d83348f Compare April 11, 2025 05:53
@noisysocks noisysocks requested a review from mgurgel April 11, 2025 05:53
mgurgel
mgurgel previously approved these changes Apr 11, 2025
View reviewed changes
Copy link
Contributor

@mgurgel mgurgel left a comment

Choose a reason for hiding this comment

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

LGTM 👍

I think we could start documenting the preview link query params in onboarding/readme.md. Not a blocker, can be done on your next PR.

@noisysocks
Copy link
Contributor Author

noisysocks commented Apr 14, 2025 •
edited
Loading

I think we could start documenting the preview link query params in onboarding/readme.md. Not a blocker, can be done on your next PR.

Good idea, done. The params are specific to the special page so this PR adds them for onboarding.

@noisysocks noisysocks requested a review from mgurgel April 14, 2025 05:58
@noisysocks noisysocks mentioned this pull request Apr 14, 2025
Closed
@mgurgel mgurgel force-pushed the randerson/ad-blocking-in-onboarding branch from ae71268 to e89136d Compare April 14, 2025 15:58
mgurgel
mgurgel previously approved these changes Apr 14, 2025
View reviewed changes
Copy link
Contributor

@mgurgel mgurgel 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, pending one non-blocking fix on the readme. I've rebased it and will leave it approved. If your Windows testing doesn't show any regressions (and if no future rebases invalidate my review), feel free to merge.

shakyShane
shakyShane reviewed Apr 15, 2025
View reviewed changes
Copy link
Contributor

@shakyShane shakyShane left a comment

Choose a reason for hiding this comment

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

LGTM!

@mgurgel
Copy link
Contributor

mgurgel commented Apr 15, 2025

@shakyShane Since this change is not part of the default step order in onboarding, and #1621 is behind a feature flag, I think it's safe to merge this, then add the translations once we get final copy approval in the Ship Review that @noisysocks started today.

@noisysocks noisysocks force-pushed the randerson/ad-blocking-in-onboarding branch from 9ad8757 to c07bd5c Compare April 15, 2025 09:53
@noisysocks
Copy link
Contributor Author

What Marcos says! I don’t want to submit translations until copy is finalised. There’s no changes here unless native apps pass ’ad-blocking’ as a step.

@noisysocks
Add ad blocking variants to v3 onboarding
ba0b979 
- Add optional 'Enhanced Ad Blocking' row to the settings step.
- Add optional 'YouTube Ad Blocking' row to the settings step.
- Update Duck Player step copy when YouTube Ad Blocking is enabled.
@noisysocks
Empty Netlify build cache
cc731d6
@noisysocks
Revert "Empty Netlify build cache"
d896d94 
This reverts commit 0049864.
@noisysocks
Use object shorthand
63215d1
@noisysocks
Move preview URL implementation to mock-transport.js
5b98514
@noisysocks
Document onboarding test URL parameters
d580704
@noisysocks noisysocks force-pushed the randerson/ad-blocking-in-onboarding branch from c07bd5c to 72aef78 Compare April 15, 2025 23:02
@noisysocks noisysocks merged commit d29cf78 into main Apr 15, 2025
13 checks passed
@noisysocks noisysocks deleted the randerson/ad-blocking-in-onboarding branch April 15, 2025 23:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shakyShane shakyShane shakyShane approved these changes

@mgurgel mgurgel mgurgel left review comments

Assignees
No one assigned
Labels
enhancement New feature or request minor Increment the minor version when merged
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@noisysocks @mgurgel @shakyShane