Closed Bug 1097749 Opened 10 years ago Closed 9 years ago

Standalone rooms should display the room name

Categories

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

defect
Points:
2

Tracking

(firefox35 fixed, firefox36 fixed)

RESOLVED FIXED
mozilla36
Iteration:
36.3
Tracking Status
firefox35 --- fixed
firefox36 --- fixed
backlog Fx35+

People

(Reporter: standard8, Assigned: standard8)

References

Details

Attachments

(1 file, 1 obsolete file)

As part of the UX we should be displaying the room name. We can only get this after joining, via a GET /rooms/{token} on the server.
backlog: --- → Fx35+
Priority: -- → P1
Whiteboard: [rooms]
Assignee: nobody → standard8
Iteration: --- → 36.3
Points: --- → 2
Iteration: 36.3 → 37.1
Currently, until the server fixes and deploys bug 1103331, we can only display the room name after join. This does the necessary work to make that happen.
Attachment #8528376 - Flags: review?(nperriault)
Comment on attachment 8528376 [details] [diff] [review]
Standalone rooms should display the room name once the room has been joined.

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

Looks good, works great: ship it! (optional nits you might want to address)

::: browser/components/loop/content/shared/js/activeRoomStore.js
@@ +296,5 @@
> +
> +      // If we haven't got a room name yet, go and get one. We typically
> +      // need to do this in the case of the standalone window.
> +      // XXX When bug 1103331 lands this can be moved to earlier.
> +      if (!this.getStoreState().roomName) {

Nit: this._storeState.roomName is probably just fine here.

@@ +302,5 @@
> +          function(err, result) {
> +            if (err) {
> +              console.error("Failed to get room data:", err);
> +              return;
> +            }

Note: We'd usually want to dispatch an action here, but it's probably okay as we're only logging.

::: browser/components/loop/standalone/content/js/standaloneMozLoop.js
@@ +94,5 @@
> +          // We currently only require the roomName in the response.
> +          callback(null, validate(responseData, {
> +            roomName: String,
> +            roomOwner: String,
> +            roomUrl: String

Nit: This definition seems to contradict the comment above.
Attachment #8528376 - Flags: review?(nperriault) → review+
Updated patch for review comments.
Attachment #8528376 - Attachment is obsolete: true
Attachment #8528622 - Flags: review+
Keywords: checkin-needed
Whiteboard: [rooms] → [rooms][checkin needed, may depend on 1097742 landing first]
https://hg.mozilla.org/integration/fx-team/rev/562e47b962ed
Keywords: checkin-needed
Whiteboard: [rooms][checkin needed, may depend on 1097742 landing first]
Target Milestone: --- → mozilla36
https://hg.mozilla.org/mozilla-central/rev/562e47b962ed
Status: NEW → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Comment on attachment 8528622 [details] [diff] [review]
Standalone rooms should display the room name once the room has been joined.

Approval Request Comment
[Feature/regressing bug #]: Rooms

[User impact if declined]: Merge issues on the one shared file

[Describe test coverage new/current, TBPL]: Includes tests

[Risks and why]: Standalone changes; touches one file that's shared.  Very low risk, totally to rooms.

[String/UUID change made/needed]: none.
Attachment #8528622 - Flags: approval-mozilla-aurora?
Flags: qe-verify-
Comment on attachment 8528622 [details] [diff] [review]
Standalone rooms should display the room name once the room has been joined.

Approval Request Comment
Move request to beta
Attachment #8528622 - Flags: approval-mozilla-aurora? → approval-mozilla-beta?
Attachment #8528622 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Iteration: 37.1 → 36.3
You need to log in before you can comment on or make changes to this bug.