1113613 - Telefonica logo should only appear on FTU with the "Get started" button in the panel

Status: RESOLVED FIXED

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

Description

[Romain Testard [:RT]]

Per https://bugzilla.mozilla.org/show_bug.cgi?id=1083466#c1 the Telefonica logo should be only displayed on FTU. 
It currently follows the same rules as the ToS and privacy policy links.

Comment 1

[Maire Reavy [:mreavy]]

Later today, I'll be moving Fx36 bugs to follow the new priority definitions later today:

P1 - We would block shipment for this bug or turn off the feature if we can't fix it
P2 - We need/very much want this bug, but we wouldn't block shipment or turn off the feature.
P3 - Normal bug
P4, P5 are even lower priority

Right now I'm marking this a P1 (to align with the definitions of other Fx36 bugs), but later today this will move to P2.  This doesn't mean it has truly fallen in priorities.  We just want to save the "P1" category for a real "stop ship".

Comment 2

[Maire Reavy [:mreavy]]

Moving all P1->P2.  (P2 means a major bug that we very much want to fix, but we wouldn't stop ship or block the release for it.)

Comment 3

[Maire Reavy [:mreavy]]

Here's the actual change from P1->P2 (per the previous comment).  P2 indicates a major bug, but not a stop ship.

Comment 4

[Romain Gauthier [:tOkeshu]]

Attachment: Show the telefonica logo only on first use

Comment 5

[Nicolas Perriault (:NiKo`) — needinfo me if you need my attention]

Comment on attachment 8544056 [details] [diff] [review]
Show the telefonica logo only on first use

Review of attachment 8544056 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, r=me with comments addressed.

::: browser/app/profile/firefox.js
@@ +1669,5 @@
>  
>  pref("loop.enabled", true);
>  pref("loop.server", "https://loop.services.mozilla.com/v0");
>  pref("loop.seenToS", "unseen");
> +pref("loop.showTelefonica", true);

How about something more generic/brand agnostic, like showPartnerLogo?

::: browser/components/loop/content/js/panel.jsx
@@ +206,4 @@
>        };
>      },
>  
> +    renderTelefonica: function() {

How about renderPartnerLogo?

@@ +211,5 @@
> +        return null;
> +      }
> +
> +      var locale = mozL10n.getLanguage();
> +      navigator.mozLoop.setLoopPref('showTelefonica', false);

Note: I wish we had a Flux store for prefs, so we could dispatch an action from here; though creating a store for this bug seems overkill, so it's probably fine that way for now.

Maybe we should just file a new bug about this idea, and reference it here in a comment (if you agree it's a good idea).

::: browser/components/loop/test/desktop-local/panel_test.js
@@ +1038,5 @@
> +
> +         try {
> +           TestUtils.findRenderedDOMComponentWithClass(view, "powered-by");
> +         } catch (err) {
> +           done();

I'm confused, why do we need to use done() at all here? I can't see what's async. If it's just matter of ensuring the element isn't present, just use expect(view.getDOMNode().querySelector(".powered-by")).eql(null) as we've been doing a lot in similar situations.

Comment 6

[:shell escalante]

OK letting ride train in 37 - so not uplifting to 36.  Great to land before aurora though for 37

Comment 7

[Romain Gauthier [:tOkeshu]]

Attachment: Show the telefonica logo only on first use

Concerning Nico's proposal to have a store to set prefs, I opened Bug 1118303

Comment 8

[Nicolas Perriault (:NiKo`) — needinfo me if you need my attention]

(In reply to Romain Gauthier [:tOkeshu] from comment #7)
> Created attachment 8544646 [details] [diff] [review]
> Show the telefonica logo only on first use

This patch bitrots, could you please rebase it against latest fx-team? Thanks!

Comment 9

[Romain Gauthier [:tOkeshu]]

Attachment: Show the telefonica logo only on first use

Comment 10

[Nicolas Perriault (:NiKo`) — needinfo me if you need my attention]

Comment on attachment 8545238 [details] [diff] [review]
Show the telefonica logo only on first use

Review of attachment 8545238 [details] [diff] [review]:
-----------------------------------------------------------------

Please add the compiled version of panel.jsx (ensure you've upgraded the react-tools npm package to 0.12.2)

Comment 11

[Romain Gauthier [:tOkeshu]]

Attachment: Show the telefonica logo only on first use

Comment 12

[Romain Gauthier [:tOkeshu]]

Attachment: Show the telefonica logo only on first use

Comment 13

[Nicolas Perriault (:NiKo`) — needinfo me if you need my attention]

Comment on attachment 8545268 [details] [diff] [review]
Show the telefonica logo only on first use

Review of attachment 8545268 [details] [diff] [review]:
-----------------------------------------------------------------

Works great.

Comment 14

[Nicolas Perriault (:NiKo`) — needinfo me if you need my attention]

https://hg.mozilla.org/integration/fx-team/rev/e8a928acfdab

Comment 15

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/e8a928acfdab