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

RESOLVED FIXED in Firefox 41

Status

Hello (Loop)
Client
P2
normal
Rank:
23
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: standard8, Assigned: standard8)

Tracking

unspecified
mozilla41
Points:
2
Bug Flags:
firefox-backlog +

Firefox Tracking Flags

(firefox41 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
STR:

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+
(Assignee)

Updated

3 years ago
Rank: 23
Priority: -- → P2
(Assignee)

Updated

3 years ago
Blocks: 1115340
(Assignee)

Comment 1

3 years ago
Created attachment 8610541 [details] [diff] [review]
Loop Link-clicker: Joining a room, giving feedback, looses context information until the page is reloaded.

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+

Comment 3

3 years ago
https://hg.mozilla.org/integration/fx-team/rev/efa467fe589f
https://hg.mozilla.org/mozilla-central/rev/efa467fe589f
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox41: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
You need to log in before you can comment on or make changes to this bug.