Per 1077257 we need a feedback form allowing users to provide details around any issues they may experience. We should just leave the text box available all the time and submit its contents when submitting feedback even if the user doesn't select any category.

find the bug like this to dupe...t he one with strings and details in 35+

Keeping this one open since I think we want a separate bug for Desktop feedback and Standalone feedback for Rooms.

This patch implements the feedback form for standalone users, though also introduces a change in the flow, including for Desktop; basically everytime a user had an actual video conversation with at least one participant (which is our current maximum), the room state transitions to ENDED and renders the feedback form, otherwise switches back to READY. Notes: - Styling still needs to be done. - Tests aren't written just yet, as I wanted to discuss the changes first.

Comment on attachment 8528468 [details] [diff] [review] Feedback form displayed for Loop standalone rooms, patch v1. This patch isn't good and has too many side effects. I'll take an easier route.

Here's a simpler approach, which is enough to ship imho.

Comment on attachment 8529134 [details] [diff] [review] Feedback form displayed for Loop standalone rooms. Review of attachment 8529134 [details] [diff] [review]: ----------------------------------------------------------------- Looks good, one minor comment. As you're not about, I'll fix it and get it landed. ::: browser/components/loop/content/shared/js/activeRoomStore.js @@ +375,5 @@ > */ > remotePeerDisconnected: function() { > // As we only support two users at the moment, we just set this > // back to joined. > + this.setStoreState({roomState: ROOM_STATES.JOINED}); I guess you were intending to make this ENDED. However, I had a chat to Maire, and we agreed that at the moment we should just do feedback when the user leaves the room, not a remote peer. So I'll put this back to session connected - even though the remote peer is disconnected, the sdk is still connected.

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

This fixes a couple of things I just realised were wrong in the previous patch: - The waiting for media message was formatted in the wrong location in the display (needed wrapping with a div) - If you completed one call (or room in a conversation), then gave feedback, and then had another call, the second time, you wouldn't be able to give feedback (the store wasn't resetting). This fixes both of those and updates tests. I've not included a test for checking that we dispatch the action for the Feedback view for calls, as we don't seem to have any coverage for that, and due to the way the code is, testing it would be quite difficult. Also, given that the code could either be rewritten or go away soon, I'm not sure its worth that much effort.

Comment on attachment 8529240 [details] [diff] [review] Follow-up to bug 1079225 - Fix formatting of the waiting for media message in Loop rooms, and ensure feedback can be given for multiple conversations in a row. Review of attachment 8529240 [details] [diff] [review]: ----------------------------------------------------------------- r+ -- I'd like to see a test for the feedbackStore#feedbackComplete method, and I have a naming nit, but this is otherwise good to go. ::: browser/components/loop/content/shared/js/feedbackStore.js @@ +97,5 @@ > + /** > + * Resets the store to its initial state as feedback has been completed, > + * i.e. ready for the next round of feedback. > + */ > + feedbackComplete: function() { I don't see a test for this method. ::: browser/components/loop/standalone/content/js/webapp.jsx @@ +690,5 @@ > return nextState.callStatus !== this.state.callStatus; > }, > > callStatusSwitcher: function(status) { > + this.props.feedbackStore.dispatchAction(new sharedActions.FeedbackComplete()); This seems like an awkward place to put this, given the name of the function. Rename the function to make it clearer that it has this side effect, or refactor to do this dispatch elsewhere.

Updated patch to fix review comments.

Follow-up patch pushed: https://hg.mozilla.org/integration/fx-team/rev/ce3eeca4f793

https://hg.mozilla.org/mozilla-central/rev/c6dc9a2f152c https://hg.mozilla.org/mozilla-central/rev/ce3eeca4f793

Comment on attachment 8529134 [details] [diff] [review] Feedback form displayed for Loop standalone rooms. Approval Request Comment [Feature/regressing bug #]: Rooms [User impact if declined]: This is almost all standalone code (small changes to a few shared files; several of which are just reformatting). As such, this runs on the server, so mostly this just makes merges easier (especially for the few shared files). [Describe test coverage new/current, TBPL]: Includes tests [Risks and why]: See above. Very low risk. [String/UUID change made/needed]: none. Note: applies to the followup as well

Comment on attachment 8529134 [details] [diff] [review] Feedback form displayed for Loop standalone rooms. Approval Request Comment Transfer request to beta

https://hg.mozilla.org/releases/mozilla-beta/rev/b0f3e2aeb66d https://hg.mozilla.org/releases/mozilla-beta/rev/8689e7804a1f