-
Notifications
You must be signed in to change notification settings - Fork 447
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick review, i will run this code now and leave another round of feedback
Client/Frontend/Browser/BrowserViewController/BrowserViewController+ProductNotification.swift
Outdated
Show resolved
Hide resolved
Client/Frontend/Browser/BrowserViewController/BrowserViewController+ProductNotification.swift
Outdated
Show resolved
Hide resolved
Client/Frontend/Shields/ShieldsActivityItemSourceProvider.swift
Outdated
Show resolved
Hide resolved
Client/Frontend/Shields/ShieldsActivityItemSourceProvider.swift
Outdated
Show resolved
Hide resolved
let attachment = ViewTextAttachment(view: self.infoButton) | ||
string.append(NSAttributedString(attachment: attachment)) | ||
// Share UI only exist in locale JP | ||
if Locale.current.regionCode != "JP" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't this check for JP existence, not the other way around?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No we add the question mark for help navigation as text attachment normally. In JP locale we are changing this to new button design with share.
9a99c83
to
8287c09
Compare
60eac9d
to
8287c09
Compare
Client/Frontend/Shields/ShieldsActivityItemSourceProvider.swift
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice job :)
a9235bc
to
0f3e67b
Compare
…different item share is added
0f3e67b
to
06d1203
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
This Pull Request is implementing the changes on Brave Shields to promote sharing
The implementation currently adding all Sharing features like creating the tracker tier and changes to educational onboarding and changes to the Brave shield screen and actually sharing with Activity Controller.
For sharing Part we are getting a screenshot of Global Stats View and sharing it with additional text. For some sharing types like whatsapp, slack, instagram etc.., we are expanding the snapshotted image and drawing text inside the image with CoreGraphics,
Summary of Changes
This pull request fixes #2981
Submitter Checklist:
NSLocalizableString()
Test Plan:
Part1:
Press Brave Shields on Top Bar
Checkout the changes on blocked row
Press the share button and checkout the share type screen with row types
Part2:
Hit one of the block count tier number on Browser
Press the share button and checkout the share type screen with row types
Screenshots:
Reviewer Checklist:
QA/(Yes|No)
release-notes/(include|exclude)
bug
/enhancement