Closed Bug 1101754 Opened 10 years ago Closed 9 years ago

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

Categories

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

defect
Points:
5

Tracking

(firefox35 verified, firefox36 verified)

VERIFIED FIXED
mozilla36
Iteration:
36.3
Tracking Status
firefox35 --- verified
firefox36 --- verified
backlog Fx35+

People

(Reporter: jaws, Assigned: jaws)

References

Details

(Whiteboard: [FTE])

Attachments

(1 file, 1 obsolete file)

(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?
Flags: needinfo?(mmaslaney)
backlog: --- → Fx35+
Priority: -- → P1
Whiteboard: [FTE]
Assignee: nobody → jaws
Status: NEW → ASSIGNED
Iteration: --- → 36.3
Points: --- → 5
Flags: qe-verify+
Flags: firefox-backlog+
Marco, can you please add this to 36.3 iteration?
Flags: needinfo?(mmucci)
Added to IT 36.3
Flags: needinfo?(mmucci)
Attached patch Patch (obsolete) — Splinter Review
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).
Attachment #8526417 - Flags: review?(mdeboer)
Attached patch Patch v1.1Splinter Review
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.
Attachment #8526417 - Attachment is obsolete: true
Attachment #8526417 - Flags: review?(mdeboer)
Attachment #8526419 - Flags: review?(mdeboer)
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?
Attachment #8526419 - Flags: review?(mdeboer) → review+
(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
Whiteboard: [FTE] → [FTE][fixed-in-fx-team]
See bug 1100565 comment 10 for a follow-up patch to address the vertical alignment question.
https://hg.mozilla.org/mozilla-central/rev/5c654f266efb
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [FTE][fixed-in-fx-team] → [FTE]
Target Milestone: --- → mozilla36
(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?
Flags: in-testsuite+
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
Attachment #8526419 - Flags: approval-mozilla-aurora?
Attachment #8526419 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Verified fixed FF 35b3, 36.0a2 (2014-12-16) Win 7
Status: RESOLVED → VERIFIED
I filed bug 1118380 to get the close button icon tracked.
Flags: needinfo?(mmaslaney)
You need to log in before you can comment on or make changes to this bug.