Skip to content
This repository has been archived by the owner on May 10, 2024. It is now read-only.

[ads] Unify iOS kBraveNTPBrandedWallpaper #8702

Merged
merged 1 commit into from
Feb 6, 2024

Conversation

tmancey
Copy link
Collaborator

@tmancey tmancey commented Jan 25, 2024

Summary of Changes

This pull request fixes #8701

Requires brave/brave-core#21751

Submitter Checklist:

  • Unit Tests are updated to cover new or changed functionality
  • User-facing strings use NSLocalizableString()
  • New or updated UI has been tested across:
    • Light & dark mode
    • Different size classes (iPhone, landscape, iPad)
    • Different dynamic type sizes

Test Plan:

  • Fresh install
  • Launch browser
  • Wait a few minutes for the seed to be fetched (or you can background/foreground the browser)
  • Open Settings > Version #.# (#.#.#.#) > View all version info > Active Variations*
  • Check if Active Variations: list contains the enabled studies for iOS.
  • If enabled studies for iOS are not listed in Active Variations: then terminate and start browser. Make sure that Active Variations: list contains the enabled studies for iOS.
  • Open new tabs until an New Tab Takeover ad is shown
    • EXPECTED RESULT: New Tab Takeover ad should be shown on the nth BraveNTPBrandedWallpaper/initial_count_to_branded_wallpaper Griffin feature param
  • Open new tabs until an New Tab Takeover ad is shown
    • EXPECTED RESULT: New Tab Takeover ad should be shown on the nth BraveNTPBrandedWallpaper/count_to_branded_wallpaper Griffin feature param

We should also test the default values if the Griffin feature does not exist. Default value is 2 for BraveNTPBrandedWallpaper/initial_count_to_branded_wallpaper and 3 for BraveNTPBrandedWallpaper/count_to_branded_wallpaper.

And that the position is persisted across browser restarts.

Screenshots:

Reviewer Checklist:

  • Issues include necessary QA labels:
    • QA/(Yes|No)
    • bug / enhancement
  • Necessary security reviews have taken place.
  • Adequate unit test coverage exists to prevent regressions.
  • Adequate test plan exists for QA to validate (if applicable).
  • Issue and pull request is assigned to a milestone (should happen at merge time).

@tmancey tmancey requested a review from aseren January 25, 2024 16:26
@tmancey tmancey self-assigned this Jan 25, 2024
@tmancey tmancey requested a review from a team as a code owner January 25, 2024 16:26
@tmancey
Copy link
Collaborator Author

tmancey commented Jan 25, 2024

Tests are failing because we first need to merge brave-core changes with brave/brave-core#21751

@tmancey tmancey force-pushed the issues/brave-core-35522-related-changes branch 3 times, most recently from c8e0e0d to 558d25e Compare January 26, 2024 20:11
@tmancey tmancey closed this Jan 27, 2024
@tmancey tmancey force-pushed the issues/brave-core-35522-related-changes branch from 558d25e to 0aaeab1 Compare January 27, 2024 13:21
@tmancey tmancey reopened this Jan 27, 2024
@iccub iccub added this to the 1.62 milestone Jan 30, 2024
@tmancey tmancey force-pushed the issues/brave-core-35522-related-changes branch from 78d516b to 43d4883 Compare January 31, 2024 18:05
@tmancey tmancey requested a review from kylehickinson January 31, 2024 18:06
@tmancey tmancey force-pushed the issues/brave-core-35522-related-changes branch 2 times, most recently from 9892ec0 to bda4d60 Compare January 31, 2024 18:20
@aseren aseren force-pushed the issues/brave-core-35522-related-changes branch from bda4d60 to be9ee10 Compare February 1, 2024 20:36
App/iOS/Delegates/AppState.swift Outdated Show resolved Hide resolved
@aseren aseren force-pushed the issues/brave-core-35522-related-changes branch from be9ee10 to c0345be Compare February 5, 2024 23:20
@aseren aseren requested a review from kylehickinson February 5, 2024 23:21
@aseren aseren merged commit 0a6fd8b into development Feb 6, 2024
11 checks passed
@aseren aseren deleted the issues/brave-core-35522-related-changes branch February 6, 2024 03:11
linhkikuchi pushed a commit to brave/brave-core that referenced this pull request Feb 13, 2024
…35522-related-changes

[ads] Unify iOS kBraveNTPBrandedWallpaper
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ads] Unify iOS kBraveNTPBrandedWallpaper
5 participants