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

Add Brave Player button to navigation bar #8327

Closed
wants to merge 1 commit into from

Conversation

cuba
Copy link
Contributor

@cuba cuba commented Oct 27, 2023

Summary of Changes

This pull request fixes #8326

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:

See issue for STR

Screenshots:

Screenshot 2023-10-26 at 9 09 07 PM Screenshot 2023-10-26 at 9 09 33 PM
Simulator.Screen.Recording.-.iPhone.15.Pro.-.2023-10-26.at.21.08.23.mp4

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).

@cuba cuba requested a review from a team as a code owner October 27, 2023 02:57
return
}

if ShieldPreferences.hasSeenAntiAdBlockWarning.value, let etldP1 = url.baseDomain, let components = URLComponents(url: url, resolvingAgainstBaseURL: false) {
Copy link
Contributor

Choose a reason for hiding this comment

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

So is it correct user can only see Brave Player if user pressed "Adjust Shields For Me" button on youtube pop-over ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, the user always sees brave player icon. The adjust shields is just informing the user they should disable shields for YouTube

@diracdeltas
Copy link
Member

doesn't this require a security review? @stoletheminerals

@iccub iccub modified the milestone: 1.58.1 Oct 30, 2023
@cuba cuba force-pushed the js/youtube-warning branch from d2e538b to b4a7f85 Compare November 9, 2023 20:19
@cuba cuba force-pushed the js/youtube-warning branch from b4a7f85 to eb111c4 Compare November 14, 2023 19:45
@diracdeltas
Copy link
Member

reminder again that the youtube player work on iOS still needs a security review.

@cuba
Copy link
Contributor Author

cuba commented Nov 14, 2023

All combined here: #8416

@cuba cuba closed this Nov 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants