1239965 - Door hanger prompts the user to share on Hello as he copies the URL content

Status: RESOLVED FIXED

(Hello (Loop) :: Client, defect, P1)

Comment 1

[Romain Testard [:RT]]

Attachment: Door hanger UX

Comment 2

[Romain Testard [:RT]]

Updated the US field with acceptance criteria, please note the copy is not final yet.

Comment 3

[Romain Testard [:RT]]

Sevaan, can you please confirm that the copy specified in the user story field is the best assumption to go with for now?

Comment 4

[Sevaan Franks [:sevaan]]

WFM

Comment 5

[Ed Lee :Mardak]

> - The user can select the action button (label to be confirmed) to create a Hello conversation using the active tab URL (creates a conversation and opens it automatically as if the user had pressed the Hello button and then selected "Browse this page with a friend")

Does this mean it'll bypass the FTU/ToS panel?

Similarly, what should happen in e10s (where currently it shows "doesn't work" panel).

Comment 6

[Romain Testard [:RT]]

(In reply to Ed Lee :Mardak from comment #5)
> > - The user can select the action button (label to be confirmed) to create a Hello conversation using the active tab URL (creates a conversation and opens it automatically as if the user had pressed the Hello button and then selected "Browse this page with a friend")
> 
> Does this mean it'll bypass the FTU/ToS panel?
> 
> Similarly, what should happen in e10s (where currently it shows "doesn't
> work" panel).

Good point
Just talked with Sevaan and Ian about this.
Let's open the panel in whichever state it is (if the user has never used Hello then we show the FTU panel with ToS links, if the user is on e10s then open the e10s version of the panel offering the user to open a non e10s window. I'll update the US accordingly.

Comment 7

[Ed Lee :Mardak]

Attachment: wip video

Not entirely sure yet why e10s-mode's panel is not resizing..

Comment 8

[Ed Lee :Mardak]

Ha, duh. It was copying over the styling of the e10s panel height.

Comment 9

[Ed Lee :Mardak]

There was a desire to limit how many users see this feature by channel. What percentage of users should see this on nightly vs aurora vs beta vs release (vs esr)?

Currently, I have it with nightly/aurora: 10%, beta: 1%, release/other: 0.1%.

Comment 10

[Romain Testard [:RT]]

Is this coded client side or an external control? An external control (similar to how Hello was released initially) would help get more flexibility in the way we expose the feature.
Feels like we need to start with 0% on aurora, beta and release and 10% is probably right on Nightly.
Is there a way to force it for testing purposes?

Comment 11

[Ed Lee :Mardak]

(In reply to Romain Testard [:RT] from comment #10)
> Is this coded client side or an external control?
My patch has it hardcoded right now. I suppose a related question is how difficult would it be to get an add-on update that changes these hardcoded values.

> An external control (similar to how Hello was released initially) would help get more flexibility in the way we expose the feature.
Standard8, any ideas how this was controlled for Hello release?

Comment 12

[Ed Lee :Mardak]

I see bug 1060809 with DNS A entries to throttle the release with bug 1055319 implementing the Firefox code to use the entry.

Standard8, any suggestions on what approach to take? Will it take long to configure the DNS side? The client side will need to be ported, but that's pretty straightforward.

To support different rates for different channels, we could split the A record into three 8-bit values for a 1/256 resolution, E.g., A record = 127.[nightly/aurora].[beta].[release/other] and the client uses a random number from 0-255 to compare.

Alternatively, we can just have multiple hostnames each with their own A record.

Comment 13

[Ed Lee :Mardak]

I went ahead and implemented this as 127.[release/beta/other].[aurora].[nightly] supporting 255 values (0-254) so a given channel can be 0% (0/255) or 100% (255/255) or ~0.4% (1/255), etc. We'll need a dummy hostname.. probably copy.loop.services.mozilla.com? With an A record of 127.0.0.25 to match RT's 10% for nightly.

Comment 14

[GitHub Autolander Bot]

Attachment: [loop] Mardak:bug-1239965-copy > mozilla:master

Comment 15

[Ed Lee :Mardak]

Attachment: v1 video

This video is slightly edited to have the copy panel show up again (in non-e10s window) even though it was shown in e10s window. Normally the code only allows it to be shown once.

Comment 16

[Ed Lee :Mardak]

Comment on attachment 8729508 [details] [review]
[loop] Mardak:bug-1239965-copy > mozilla:master

Adds mochitests but no ui showcase or tests for copy.html/js as it's a chrome page that doesn't work in Chrome.

Also, no telemetry in this current PR as it'll need a separate patch for m-c to modify Histograms.json. Although I could update the PR to just call TelemetryAddValue of the expected probes.

Comment 17

[Ed Lee :Mardak]

Attachment: v1 histograms.json

Updated PR to use 2 (of 5) telemetry probes given that the UI doesn't have "no" button or checkbox for never again.

Comment 18

[Mark Banner (:standard8)]

Comment on attachment 8730096 [details] [diff] [review]
v1 histograms.json

This will need review/approval from one of the privacy peers (e.g. bsmedberg), with an explanation as to why we're doing this.

Comment 19

[Ed Lee :Mardak]

Comment on attachment 8730096 [details] [diff] [review]
v1 histograms.json

See attachment 8729510 [details] for a video of the feature. The SHOWN value would be incremented (at most once for now) when the user copies from the location bar. YES_AGAIN incremented when the user clicks on the "yes, show me" button.

For now, NO_AGAIN/NO_NEVER/YES_NEVER aren't used for the simplified panel, but the plan is to have a "no" button as well as a checkbox (again/never) as in attachment 8714777 [details]. As there are no additional controls, the copy panel is only triggered once on copy, so the most SHOWN would be is 1 (unless a pref is reset).

Comment 20

[Jeff Griffiths (:canuckistani) (:⚡︎)]

Before we test this I would prefer if there was a clear statement of acceptance / rejection criteria for moving the feature forward. What does success look like?

Comment 21

[Sevaan Franks [:sevaan]]

Comment on attachment 8729510 [details]
v1 video

Functionality looks great, but the margins of the doorhanger need looking at. Please use 15px all around. Same goes for above and below the text in the button.

Comment 22

[Ed Lee :Mardak]

Attachment: v1 screenshot

Updated padding around message/logo and button text to be 15px.

Comment 23

[Sevaan Franks [:sevaan]]

Comment on attachment 8730953 [details]
v1 screenshot

Thx!

Comment 24

[Mark Banner (:standard8)]

Comment on attachment 8729508 [details] [review]
[loop] Mardak:bug-1239965-copy > mozilla:master

I don't think I'm going to get to this soon, and Dan seems to be setting up for it. Please discuss if that isn't the case, and we'll sort something out.

Comment 25

[Romain Testard [:RT]]

(In reply to Jeff Griffiths (:canuckistani) (:⚡︎) from comment #20)
> Before we test this I would prefer if there was a clear statement of
> acceptance / rejection criteria for moving the feature forward. What does
> success look like?

FYI this is progressing on a separate thread with Winston, Sevaan and Matt Grimes to get a resonable benchmarked ratio and value.

Current assumptions are as follows although I'll have to confirm hopefully later this week:
Actions undertaken based on KPI results after 10 days of launch:
- If Ratio “Don’t show this again clicked” / “Copy message displayed” > 50% AND NPS > 20
   Increase the share of the Firefox user base exposed to the feature in 10% increments weekly up to 100% so long as the ratio remains above 50%
- If Ratio “Don’t show this again clicked” / “Copy message displayed” < 50% AND NPS > 20
   Revisit UX for copy action door hanger

Comment 26

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 8729508 [details] [review]
[loop] Mardak:bug-1239965-copy > mozilla:master

There's a bunch of good cleaniless and testing here, but I think one thing needs some shifting.  Feedback+; comments in the PR.

Another relevant question: this appears to be a partial implementation of the user story described at the top.  Which is fine by me, but it'd be good to document here what the complete landing plan is, if splitting it out into multiple bugs is not worth the overhead.

Comment 27

[Romain Testard [:RT]]

Please refer to the user story field for full implementation details.
We would not ship this partial implementation since we need all features implemented to correctly monitor success.

Comment 28

[Ed Lee :Mardak]

Attachment: v1.1 screenshot indented

Updated to have both buttons and checkbox.

Comment 29

[Ed Lee :Mardak]

Attachment: v1.1 screenshot aligned

Comment 30

[Sevaan Franks [:sevaan]]

Comment on attachment 8736224 [details]
v1.1 screenshot aligned

The left and top borders of the blue button should be #0097C5, not gray. 

Example: http://i.sevaan.com/3H3k1W3l0Z3i

Comment 31

[Ed Lee :Mardak]

Attachment: v1.2 screenshot

Just making sure, the checkbox should be left aligned and not indented?

Comment 32

[Sevaan Franks [:sevaan]]

Comment on attachment 8736345 [details]
v1.2 screenshot

No need to indent. Looks good, thanks!

Comment 33

[Romain Testard [:RT]]

I updated the user story field with the "Rules to change the throttle value" that we agreed on through a separate e-mail thread. This is the best we can think of currently with the limited experience that we have of user reaction to such features but at least that will be a starting reference for us to shoot for:

Rules to change the throttle value:
- If Ratio “Don’t show this again clicked” / “Copy message displayed” > 50% AND NPS > 20
   Increase the share of the Firefox user base exposed to the feature in 10% increments weekly up to 100% so long as the ratio remains above 50%
- If Ratio “Don’t show this again clicked” / “Copy message displayed” < 50% AND NPS > 20
   Revisit UX for copy action door hanger

Comment 34

[Ed Lee :Mardak]

Comment on attachment 8729508 [details] [review]
[loop] Mardak:bug-1239965-copy > mozilla:master

ianb noted on IRC that we should probably have a separate rollout number for release vs beta, so instead of grouping numbers by release/beta/other, aurora, nightly; it should probably be release/other, beta, aurora/nightly.

Comment 35

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 8729508 [details] [review]
[loop] Mardak:bug-1239965-copy > mozilla:master

r=dmose. That said, if you have any code you'd like me to review before landing, I'm happy to do that.

Comment 36

[Dan Mosedale (:dmosedale, :dmose)]

(With the comments in the PR addressed, of course).  :-)

Comment 37

[Ed Lee :Mardak]

https://github.com/mozilla/loop/commit/d34526b30f0566b4c490a47f94da94d05dd65330