1074678 - As a user, I should be able to click on a room and have the room open

Status: VERIFIED FIXED

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

Comment 1

[Mark Banner (:standard8)]

Attachment: Open a room window when the user selects the room in the Loop panel.

Depends on bug 1090209. This completes the flow and gets the right data to the right places.

NiKo: How do you think we should handle the openRoom - with an action to the store (which has mozLoop available for mocking), or similar to what I've got in this patch, called direct.

Comment 2

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

Comment on attachment 8514366 [details] [diff] [review]
Open a room window when the user selects the room in the Loop panel.

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

Looks good.

(In reply to Mark Banner (:standard8) from comment #1)

> NiKo: How do you think we should handle the openRoom - with an action to the
> store (which has mozLoop available for mocking), or similar to what I've got
> in this patch, called direct.

I'd find it more consistent to dispatch an action, so anything mozLoop.rooms calls would be performed from stores; having everything using some particular API localized in a single place is usually helpful when debugging, and improves maintainability over time imoe.

::: browser/components/loop/MozLoopAPI.jsm
@@ +111,5 @@
> +        });
> +      }
> +      else {
> +        api[func](...params, lastParam);
> +      }

Nit: This dense section would benefit from a bit more commenting I think.

Comment 3

[Mark Banner (:standard8)]

Attachment: Open a room window when the user selects the room in the Loop panel.

Updated patch, for bug 1090209 changes. This depends on that bug - though I've also not had chance to update for bitrot since NiKo's create room patch landed.

Comment 4

[Mike de Boer [:mikedeboer]]

Comment on attachment 8515091 [details] [diff] [review]
Open a room window when the user selects the room in the Loop panel.

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

r=me with one comment addressed. <3 the cleanup.

::: browser/components/loop/test/xpcshell/test_looprooms.js
@@ +178,5 @@
> +  };
> +
> +  LoopRooms.open("fakeToken");
> +
> +  do_check_true(!!openedUrl, "should open a chat window");

Assert.ok(openedUrl, "...");

Comment 5

[Mark Banner (:standard8)]

Attachment: Open a room window when the user selects the room in the Loop panel.

Updated for review comment and bitrot.

Comment 6

[Mark Banner (:standard8)]

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

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/24d86d8ed318

Comment 8

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

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

Comment 9

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

Comment on attachment 8515958 [details] [diff] [review]
Open a room window when the user selects the room in the Loop panel.

Approval Request Comment
Landed on aurora per IRC with lsblakk with a=loop-only

Comment 10

[Paul Silaghi, QA [:pauly]]

> UX:
> * Clicking anywhere on the room list item view, apart from the copy and delete buttons should open the > conversation view
Actually, clicking only on the call URL opens the conversation window.
Verified fixed FF 35b4, 36.0a2 (2014-12-17) Ubuntu 13.04