Share tracker stats #859

joeyg · 2018-01-31T23:57:54Z

Fixes #858.

To do:

Make the button on the home screen look tappable - ideas welcome
Only show on devices of Phone 6, 6+, or X form factor

joeyg · 2018-01-31T23:58:23Z

Blockzilla/BrowserViewController.swift

+        // TODO: Telemetry
+
+        let numberOfTrackersBlocked = getNumberOfLifetimeTrackersBlocked()
+        let appStoreUrl = String(format: "https://itunes.apple.com/app/id%@", AppInfo.isKlar ? "1073435754" : "1055677337")


There has got to be a better way to do this.

We will get you adjust links to track this install channel. Stay tuned.

joeyg · 2018-01-31T23:59:38Z

Blockzilla/BrowserViewController.swift

@@ -244,6 +246,13 @@ class BrowserViewController: UIViewController {
            homeView.removeFromSuperview()
        }
        self.homeView = homeView
+
+        if shouldShowTrackerStatsShareButton() {


This is run after a user taps "Erase" so there is a chance they would not see it on launch but then see it after they erased a browsing session. Is that ok?

A chance they wouldn't see it on launch? And then they would see it every time they 'Erase'?

Ahh i miss understood! I thought the 10% rule was just to not show it all the time and annoy users. I will change it to bucket users into groups.

Sdaswani · 2018-02-01T01:08:39Z

Blockzilla/BrowserViewController.swift

@@ -244,6 +246,13 @@ class BrowserViewController: UIViewController {
            homeView.removeFromSuperview()
        }
        self.homeView = homeView
+
+        if shouldShowTrackerStatsShareButton() {


A chance they wouldn't see it on launch? And then they would see it every time they 'Erase'?

Sdaswani · 2018-02-01T01:10:31Z

Blockzilla/BrowserViewController.swift

+
+        // 10% chance we show the button
+        // arc4random_uniform(10 returns an integer 0 through 9 (inclusive)
+        return arc4random_uniform(10) == 0


So the way I understand this, you are a flipping a 10% coin every time a user presses Erase, so that means that all users would eventually see the message? We need to only show it to maximum of 10% of users - so we have to put users in buckets and save that bucket across the install. This is something we do for onboarding too.

Sdaswani · 2018-02-01T01:11:31Z

Blockzilla/BrowserViewController.swift

+        // TODO: Telemetry
+
+        let numberOfTrackersBlocked = getNumberOfLifetimeTrackersBlocked()
+        let appStoreUrl = String(format: "https://itunes.apple.com/app/id%@", AppInfo.isKlar ? "1073435754" : "1055677337")


We will get you adjust links to track this install channel. Stay tuned.

boek

The one last missing thing would be adding some telemetry around how many users are in the bucket and the share button. Other than that the code looks good :) Will defer UX to @lime124

boek · 2018-02-01T22:28:52Z

Blockzilla/BrowserViewController.swift

@@ -747,6 +785,15 @@ extension BrowserViewController: WebControllerDelegate {
    func webController(_ controller: WebController, stateDidChange state: BrowserState) {}

    func webController(_ controller: WebController, didUpdateTrackingProtectionStatus trackingStatus: TrackingProtectionStatus) {
+        // Calculate the number of trackers blocked and add that to lifetime total
+        if case .on(let info) = trackingStatus {
+            if case .on(let oldInfo) = trackingProtectionStatus {


small nit: We could do this in one if statement

boek · 2018-02-01T22:30:48Z

Blockzilla/BrowserViewController.swift

+        // Calculate the number of trackers blocked and add that to lifetime total
+        if case .on(let info) = trackingStatus {
+            if case .on(let oldInfo) = trackingProtectionStatus {
+                let numberOfAdditionalTrackersBlocked = max(0, info.total - oldInfo.total)


This took me a minute to understand what/why we're doing it this way. I wonder if we named it something like differenceSinceLastUpdate it would be more clear?

joeyg · 2018-02-02T20:24:17Z

@boek @Sdaswani Updated the PR with changes to the logic around showing the button and added some Telemetry. Let me know if the telemetry names should be changed to be more clear or if the logic still seems off.

I can also pull the code out of BrowserViewController if that would help.

Sdaswani · 2018-02-02T20:41:09Z

@joeyg thanks for the video! Can you move the message closer to the keyboard, make the text smaller, and add ‘Click here’ to the tell your friends message? I’ll look at the code soon.

joeyg · 2018-02-02T21:05:27Z

lime124 · 2018-02-02T23:22:16Z

Hey guys! As always you are super fast @joeyg! I think there are a few usability concerns here that should be addressed. I made a quick mockup to illustrate what I'm thinking.

1. We should avoid having something tappable too close to the keyboard, we don't want accidental hits when people are trying to tap. That spacing is 24px in my mock
2. Having two blocks of same sized text makes it difficult to wade through. We should remove the tagline to clean things up. This means SE's will probably work now
3. Even though it says "click here" there isn't an obvious call to action/place to tap. I separated out the action into a button so it's very clear where people tap. The button is 36px high x 80px. But we want our tap area to extend a bit beyond that so it meets min touch target guidelines.
4. I added in the word "total" because it isn't super clear what this number represents. We could probably ditch the word "by" but I'll ping @BrianNJones about that one.
5. I added in the TP shield so that it a) brings balance to the screen with the share button but also does a bit of reinforcing of what a tracker is. It connects that bit of UI people see in the URL bar with this concept.

We definitely need to run the suggested text (what was on the notepad) by Brian and Cherry as well. Thanks as always!

joeyg · 2018-02-03T00:22:23Z

Thanks @lime124 ! I’ll make those changes and upload a new screenshot.

BrianNJones · 2018-02-03T01:30:20Z

For #4, we can get down to a single line with: 120 trackers blocked so far (I don't think it's critical to mention Focus in the string.)

…

On Fri, Feb 2, 2018 at 5:22 PM, Tiffanie Shakespeare < ***@***.***> wrote: Hey guys! As always you are super fast @joeyg <https://github.com/joeyg>! I think there are a few usability concerns here that should be addressed. I made a quick mockup to illustrate what I'm thinking. - 1. We should avoid having something tappable too close to the keyboard, we don't want accidental hits when people are trying to tap. That spacing is 24px in my mock - 2. Having two blocks of same sized text makes it difficult to wade through. We should remove the tagline to clean things up. This means SE's will probably work now - 3. Even though it says "click here" there isn't an obvious call to action/place to tap. I separated out the action into a button so it's very clear where people tap. The button is 36px high x 80px. But we want our tap area to extend a bit beyond that so it meets min touch target guidelines. - 4. I added in the word "total" because it isn't super clear what this number represents. We could probably ditch the word "by" but I'll ping @BrianNJones <https://github.com/briannjones> about that one. - 5. I added in the TP shield so that it a) brings balance to the screen with the share button but also does a bit of reinforcing of what a tracker is. It connects that bit of UI people see in the URL bar with this concept. We definitely need to run the suggested text (what was on the notepad) by Brian and Cherry as well. Thanks as always! [image: screen shot 2018-02-02 at 2 50 19 pm] <https://user-images.githubusercontent.com/287322/35758980-b664c55c-082b-11e8-8f65-94f317139417.png> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#859 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/Ad38lxxS6F6BVLf8CpC2CVoOPZwlxdF5ks5tQ5iqgaJpZM4R0188> .

joeyg · 2018-02-03T05:17:01Z

@lime124 here are some updated screenshots.

iPhone 6 Portrait:

iPhone 6 Landscape:

iPhone 6 Plus Portrait:

iPhone 6 Plus Landscape:

Sdaswani · 2018-02-03T05:26:51Z

looking awesome @joeyg !

Sdaswani

code looking ok so far, @boek is the approver. still needs integration with the share sheet, right?

joeyg · 2018-02-03T05:33:36Z

Updated screencast http://recordit.co/xlmzogfW3Y

BrianNJones · 2018-02-05T17:15:29Z

@lime124, I recommend this for the suggested text:
Firefox Focus, the privacy browser from Mozilla, has already blocked XXX trackers for me. Fewer ads and trackers following me around means faster browsing! Get Focus for yourself here: https://itunes.apple.com/app/id1055677337

(236 characters, spaces inclusive)

Sdaswani · 2018-02-05T19:22:43Z

@joeyg please use this as the app store link: https://app.adjust.com/b8s7qo?campaign=shareblocked&creative=022018

also one more requirement - not only is this EN only, but it should be Focus only. So please don't show for Klar.

joeyg · 2018-02-05T20:53:07Z

@Sdaswani I'll make that change. Thanks.

@BrianNJones, @lime124 Would you like this shared as just text? The app can share the URL separately instead of inline. Generally this results in a preview of the URL being rendered instead of just the URL.

Sdaswani · 2018-02-05T21:01:29Z

@joeyg the important thing is to fill out as much as you can, like the NYTimes does.

joeyg · 2018-02-06T05:24:52Z

@Sdaswani Share via twitter and iMessage

lime124 · 2018-02-06T21:42:05Z

This looks super awesome @joeyg ! Will the release version have our app icon instead of the grey box? /cc maybe @Sdaswani knows?

Sdaswani · 2018-02-06T21:55:47Z

Ugh @lime124 good point, I'm not sure what we can do as since we are using a Adjust link (which is a redirect) the content pre-fetch isn't nice.

@joeyg can you use this link and see if it renders better: https://itunes.apple.com/app/apple-store/id1055677337?pt=373246&ct=ShareTracker&mt=8

joeyg · 2018-02-06T22:01:10Z

@Sdaswani this is what it looks like with the itunes URL that you provided:

Sdaswani · 2018-02-06T22:02:21Z

@lime124 is that better? we lose some analytics by not using Adjust. cc @bbinto

@joeyg is the feature done, or do you still need to do more coding?

joeyg · 2018-02-12T19:56:31Z

@Sdaswani I think that issue you ran into is the last thing that needs to be looked at. I can try and fix that this evening.

Sdaswani · 2018-02-12T20:38:30Z

As usual, thanks @joeyg - we can't wait to ship this!

…creen

joeyg · 2018-02-13T05:42:23Z

@Sdaswani I think the update to the PR I pushed this evening should fix the overlay issue you were seeing.

Sdaswani · 2018-02-13T06:12:27Z

Thanks @joeyg I'll push a build to iTunes tonight!

Sdaswani · 2018-02-14T20:37:46Z

@joeyg we need one last change - otherwise @bbinto says let's ship this!

In shouldShowTrackerStatsShareButton, if shouldShowTrackerStatsToUser, can you append a key value to the dictionary, like is done here: https://github.com/mozilla-mobile/firefox-ios/blob/master/Client/Telemetry/UnifiedTelemetry.swift#L35-L44

Perhaps:
outputDict[userDefaultsShareTrackerStatsKey] = shouldShowTrackerStatsToUser as AnyObject?

You can also then remove the recordEvent after you determine the true/false value of shouldShowTrackerStatsToUser

Don't worry about this firing too much - we will pare those down when we run the search query. It's fine to have it for users who are true or false - that will allow us to put those folks into different buckets.

joeyg · 2018-02-14T22:39:03Z

@Sdaswani just to clarify. Instead of sending a shareStatsCoinFlip event we should include the value of the coin flip with every CorePing?

Sdaswani · 2018-02-14T22:45:53Z

Yes, exactly @joeyg . That will allow @bbinto to run crazy metrics ;) .

joeyg · 2018-02-14T23:28:28Z

@Sdaswani I made those changes. Not sure the best way to test core ping telemetry so let me know if there is a good way to test this.

Sdaswani · 2018-02-14T23:46:13Z

Blockzilla/TelemetryIntegration.swift

@@ -30,6 +30,7 @@ class TelemetryEventMethod {
    public static let share = "share"
    public static let customDomainRemoved = "removed"
    public static let customDomainReordered = "reordered"
+    public static let shareStatsCoinFlip = "share_stats_coin_flip"


this can be removed now, right?

good catch. will fix these

Sdaswani · 2018-02-14T23:46:20Z

Blockzilla/TelemetryIntegration.swift

@@ -48,4 +49,6 @@ class TelemetryEventObject {
    public static let trackingProtectionToggle = "tracking_protection_toggle"
    public static let websiteLink = "website_link"
    public static let autofill = "autofill"
+    public static let showStatsShareButton = "show_share_share_button"


and this too?

Sdaswani · 2018-02-14T23:46:40Z

@joeyg we will test the telemetry, thanks!

boek

We just need to unwrap that optional :)

boek · 2018-02-15T18:09:04Z

Blockzilla/AppDelegate.swift

+            var outputDict = inputDict // make a mutable copy
+
+            let shouldShowTrackerStatsToUser = UserDefaults.standard.object(forKey: BrowserViewController.userDefaultsShareTrackerStatsKey) as! Bool?
+            outputDict["showTrackerStatsShare"] = shouldShowTrackerStatsToUser


We'll want to make sure shouldShowTrackerStatsToUser is a Boolean, not an Optional. (I don't think Telemetry unwraps things for us, just uses the description when serializing to JSON.

This could work

let shouldShowTrackerStatsToUser = (UserDefaults.standard.object(forKey: BrowserViewController.userDefaultsShareTrackerStatsKey) as? Bool) ?? false

Would that report back as False even if the value is nil? Should we just not include the param if it is nil(meaning the coin flip hasn’t been done)

You know what? We have a reference to BrowserViewController. We could just do

outputDict["showTrackerStatsShare"] = browserViewController.shouldShowTrackerStatsShareButton()

I'm kinda okay with making that public for now since it will encourage us to clean it up if we decide to turn this on for all users.

great! made that change.

boek · 2018-02-15T18:56:02Z

@Sdaswani I wonder if we limit the experiment to just en-US locales if we need to run this through l10n?

Sdaswani · 2018-02-15T20:11:11Z

@boek by only doing en_us was hoping to avoid localization - any concerns there?

* start on sharing of tracker stats * add bold font * adjust logic and add telemetry * adjust share button * update share stats ui * round the corners of the share button * Change share text and how the url is shared * change url * bump version to 4.0.3 * update share url * Fix overlay from showing up when returning from tracker stats share screen * change telemtry for tracker share button * change name to be more clear * remove unused statics * use BrowserViewController instance

bbinto · 2018-02-18T14:29:00Z

@Sdaswani I made those changes. Not sure the best way to test core ping telemetry so let me know if there is a good way to test this.

@joeyg would you mind letting me know what key and value you've used in the core ping?

I've tried looking for key "userDefaultsShareTrackerStatsKey" in the core ping but couldn't find anything? Or is that the value you'd set for the "experiments" key?

joeyg · 2018-02-18T14:51:34Z

@bbinto it should be

showTrackerStatsShare

If working properly

bbinto · 2018-02-18T14:56:18Z

Thanks @joeyg - also confirmed with @Sdaswani -- we need to get the data pipeline folks involved, back into our hands!

…ile/focus-ios#859) * start on sharing of tracker stats * add bold font * adjust logic and add telemetry * adjust share button * update share stats ui * round the corners of the share button * Change share text and how the url is shared * change url * bump version to 4.0.3 * update share url * Fix overlay from showing up when returning from tracker stats share screen * change telemtry for tracker share button * change name to be more clear * remove unused statics * use BrowserViewController instance

…ozilla-mobile/focus-ios#859) * start on sharing of tracker stats * add bold font * adjust logic and add telemetry * adjust share button * update share stats ui * round the corners of the share button * Change share text and how the url is shared * change url * bump version to 4.0.3 * update share url * Fix overlay from showing up when returning from tracker stats share screen * change telemtry for tracker share button * change name to be more clear * remove unused statics * use BrowserViewController instance

joeyg added 2 commits January 31, 2018 15:49

start on sharing of tracker stats

cc4cf40

add bold font

99f73ca

joeyg commented Jan 31, 2018

View reviewed changes

Sdaswani reviewed Feb 1, 2018

View reviewed changes

boek suggested changes Feb 1, 2018

View reviewed changes

adjust logic and add telemetry

3f79b19

adjust share button

62d4a50

joeyg added 2 commits February 2, 2018 21:11

update share stats ui

cc66fc8

round the corners of the share button

19002ab

Sdaswani reviewed Feb 3, 2018

View reviewed changes

Change share text and how the url is shared

9a095ae

bbinto self-requested a review February 6, 2018 21:38

Fix overlay from showing up when returning from tracker stats share s…

04e81fe

…creen

change telemtry for tracker share button

9da05db

change name to be more clear

78cced5

Sdaswani suggested changes Feb 14, 2018

View reviewed changes

remove unused statics

0f73554

boek self-requested a review February 15, 2018 18:06

boek suggested changes Feb 15, 2018

View reviewed changes

use BrowserViewController instance

c549f8f

boek self-requested a review February 16, 2018 17:13

boek approved these changes Feb 16, 2018

View reviewed changes

boek merged commit b753227 into mozilla-mobile:master Feb 16, 2018

oliviabrown9 mentioned this pull request Aug 2, 2018

Start Screen Tips #1165

Closed

sblatz mentioned this pull request Oct 23, 2018

Let Users Share How Many Trackers They Have Blocked #677

Closed

Share tracker stats #859

Share tracker stats #859

Conversation

joeyg commented Jan 31, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

boek left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joeyg commented Feb 2, 2018

Sdaswani commented Feb 2, 2018

joeyg commented Feb 2, 2018

lime124 commented Feb 2, 2018

joeyg commented Feb 3, 2018

BrianNJones commented Feb 3, 2018 via email

joeyg commented Feb 3, 2018 • edited Loading

Sdaswani commented Feb 3, 2018

Sdaswani left a comment

Choose a reason for hiding this comment

joeyg commented Feb 3, 2018

BrianNJones commented Feb 5, 2018

Sdaswani commented Feb 5, 2018

joeyg commented Feb 5, 2018

Sdaswani commented Feb 5, 2018

joeyg commented Feb 6, 2018

lime124 commented Feb 6, 2018

Sdaswani commented Feb 6, 2018

joeyg commented Feb 6, 2018

Sdaswani commented Feb 6, 2018

joeyg commented Feb 12, 2018

Sdaswani commented Feb 12, 2018

joeyg commented Feb 13, 2018

Sdaswani commented Feb 13, 2018

Sdaswani commented Feb 14, 2018 • edited Loading

joeyg commented Feb 14, 2018

Sdaswani commented Feb 14, 2018

joeyg commented Feb 14, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sdaswani commented Feb 14, 2018

boek left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

boek commented Feb 15, 2018

Sdaswani commented Feb 15, 2018

bbinto commented Feb 18, 2018

joeyg commented Feb 18, 2018 • edited Loading

bbinto commented Feb 18, 2018

joeyg commented Jan 31, 2018 •

edited

Loading

joeyg commented Feb 3, 2018 •

edited

Loading

Sdaswani commented Feb 14, 2018 •

edited

Loading

joeyg commented Feb 18, 2018 •

edited

Loading