1101754 - Hide the rooms/contacts view until the Getting Started tour has been accessed or dismissed

Status: VERIFIED FIXED

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

Description

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Mark Banner (:standard8) from bug 1083466 comment #25)
> I thought the planned UX, according to
> http://people.mozilla.org/~dhenein/loop/rooms/loop-rooms-spec-4.jpg was that
> we'd show just the "Get Started" button, and not the rooms list - and then
> when the button was clicked, we'd show the UX tour and the rooms list with
> start conversation button.

> And I just realised - the UX also shows that the Tabs are hidden until after
> Get Started has been pressed.

Discussed at today's standup meeting, we're going to make this hide the rooms/contacts/tabs views until either the Tour has been seen or it has been dismissed.

We can use a (X) icon in the corner of the <div> to dismiss. The spec showed an (X) button, but didn't provide the graphic.

Michael, do you have the (X) graphic for dismissing the offer?

Comment 1

[Jared Wein [:jaws] (please needinfo? me)]

Marco, can you please add this to 36.3 iteration?

Comment 2

[Marco Mucci [:MarcoM]]

Added to IT 36.3

Comment 3

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch

I had to use an event to bubble up the pref change since the encapsulating component needs to be the one that changes what gets rendered.

This patch doesn't have a close button, and currently requires the user to click on the Getting Started button to start using Loop (no skip).

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch v1.1

This patch removes the footer until Getting Started has been clicked, since it doesn't make much sense to me to allow people to sign-in as well change their availability until they can use the rest of the product.

Comment 5

[Mike de Boer [:mikedeboer]]

Comment on attachment 8526419 [details] [diff] [review]
Patch v1.1

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

How creative! I like it, sir.

Now there _is_ the question of aesthetics... I think it'd be nice to vertically center the Getting Started content in the panel. Do you want to discuss/ do that here?
Regardless, this patch works as intended, so r=me with comments addressed.

::: browser/components/loop/content/js/panel.jsx
@@ +741,5 @@
>        this.updateServiceErrors();
>      },
>  
> +    _gettingStartedSeen: function() {
> +      this.setState({gettingStartedSeen: navigator.mozLoop.getLoopPref("gettingStarted.seen")});

can you format this Object literal nicely?

@@ +807,5 @@
> +      if (!this.state.gettingStartedSeen) {
> +        return (
> +          <div>
> +            <NotificationListView notifications={this.props.notifications}
> +                                clearOnDocumentHidden={true} />

is the indention OK here, or Splinter deceiving me?

Comment 6

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Mike de Boer [:mikedeboer] from comment #5)
> Now there _is_ the question of aesthetics... I think it'd be nice to
> vertically center the Getting Started content in the panel. Do you want to
> discuss/ do that here?

I'd prefer to do that in another bug that is built on top of this patch as well as the one in bug 1100565.

Landed on fx-team as https://hg.mozilla.org/integration/fx-team/rev/5c654f266efb

Comment 7

[Jared Wein [:jaws] (please needinfo? me)]

See bug 1100565 comment 10 for a follow-up patch to address the vertical alignment question.

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/5c654f266efb

Comment 9

[u279076]

(In reply to Carsten Book [:Tomcat] from comment #8)
> https://hg.mozilla.org/mozilla-central/rev/5c654f266efb

Is the landed test coverage sufficient or do we need something more in a QA testsuite?

Comment 10

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8526419 [details] [diff] [review]
Patch v1.1

Approval Request Comment
[Risks and why]: Loop rooms code needed for 35 pref-on
[String/UUID change made/needed]: none

Comment 11

[Randell Jesup [:jesup] (needinfo me)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/fe00ed502c9f

Comment 12

[Paul Silaghi, QA [:pauly]]

Verified fixed FF 35b3, 36.0a2 (2014-12-16) Win 7

Comment 13

[Jared Wein [:jaws] (please needinfo? me)]

I filed bug 1118380 to get the close button icon tracked.