Closed Bug 1168366 Opened 9 years ago Closed 9 years ago

Link-clicker: Joining a room, giving feedback, looses context information until the page is reloaded


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



(firefox41 fixed)

41.2 - Jun 8
Tracking Status
firefox41 --- fixed


(Reporter: standard8, Assigned: standard8)




(1 file)


1) Start a new conversation with context attached (works with just the room name as well).
2) Copy the link to another browser.

=> Context is displayed

3) Join the room at both ends
4) Leave the room on the link-clicker side
5) Give feedback
6) Wait for the display to revert back to "Join the conversation"

Expected Results

=> Room Name and Room Context are still displayed

Actual Results

=> Neither room name nor context are displayed
Flags: firefox-backlog+
Rank: 23
Priority: -- → P2
Blocks: 1115340
This adjusts our flow - currently we're attempting to reset too much after the user's given feedback - this misses resetting in a few of cases (like the user leaving the room without having had someone else present), and assumes too much.

I've changed it so that we now reset an appropriate bunch of items whenever the room is "left" from the sdk sense. Once feedback is finished, we just change the bits we really need to.
Attachment #8610541 - Flags: review?(mdeboer)
Comment on attachment 8610541 [details] [diff] [review]
Loop Link-clicker: Joining a room, giving feedback, looses context information until the page is reloaded.

Review of attachment 8610541 [details] [diff] [review]:

r=me with comments addressed. Thanks for tackling this!

::: browser/components/loop/content/shared/js/activeRoomStore.js
@@ +769,5 @@
>        // We probably don't need to end screen share separately, but lets be safe.
>        this._sdkDriver.disconnectSession();
> +      // Reset various states

nit: missing dot.

@@ +796,5 @@
>        this.setStoreState({roomState: nextState});
>      },
>      /**
>       * When feedback is complete, we reset the room to the initial state.

You should update this docblock comment to reflect the updated reality.

::: browser/components/loop/test/shared/activeRoomStore_test.js
@@ +1310,5 @@
>        expect(store._storeState.roomState).eql(ROOM_STATES.ENDED);
>      });
> +
> +    it("should reset various store states", function() {

I *think* you want to test all very specific store states here, right? And additionally that leaving a room doesn't reset other state properties.
Attachment #8610541 - Flags: review?(mdeboer) → review+
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
You need to log in before you can comment on or make changes to this bug.