1079227 - Loop feedback form should always allow comments

Status: RESOLVED FIXED

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

Description

[Romain Testard [:RT]]

Attachment: feedback form.png

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.

Comment 1

[Maire Reavy [:mreavy]]

This looks like a dup of bug 1086512

Comment 2

[Romain Testard [:RT]]

I am re-opening this bug as bug 1086512 is about supporting the feedback form in rooms although this one is about changing the feedback form layout.
I added details in the user story field so requirements are clearer.
I also created bug 1113540 to track the link clicker UI implementation as I don't think there was a bug for this.

Comment 3

[Romain Testard [:RT]]

Comment on attachment 8501020 [details]
feedback form.png

Marking as obsolete as we agreed to keep the "Other" field in another bug.
The UI will look similar to what we have currently implemented, just the behavior will change (ability to select the free text field with any option).

Comment 4

[Romain Testard [:RT]]

Maire can you confirm this can be marked as a Fx36 bug given the meta that was supposed to address this is a Fx36 bug?

Comment 5

[Maire Reavy [:mreavy]]

I've marked for Fx36, P1, but I'll be moving it P2 later today when we normalize our priorities according to our new definitions.  See https://bugzilla.mozilla.org/show_bug.cgi?id=1113613#c1 for more details.

Comment 6

[Maire Reavy [:mreavy]]

Moving all P1->P2.  (P2 means a major bug that we very much want to fix, but we wouldn't stop ship or block the release for it.)

Comment 7

[Maire Reavy [:mreavy]]

Here's the actual change from P1->P2 (per the previous comment).  P2 indicates a major bug, but not a stop ship.

Comment 8

[Mark Banner (:standard8)]

Attachment: Improve the Loop feedback form.

This does the necessary for what is required. I'll get a needinfo up in a moment for feedback on the actual functionality.

Comment 10

[Mark Banner (:standard8)]

Romain/Sevaan, can you test the feedback form on our dev server and check you are both happy with the new form?

Things I've changed:

- Any option can now have text associated with it
- I've dropped the ':' from the end of "Other:" as it didn't seem right now that the text box applies to any category.

Example output on input dev server:

https://input.allizom.org/en-US/?date_end=2015-01-05&date_start=2015-01-05&product=Loop

Note that to see the category annotation, you need to visit the "permalink"

To test, either:

- Set up a new profile, set the loop.server pref to "https://loop-dev.stage.mozaws.net" (no trailing slash), restart FF, generate a room and visit, etc

or 

- Ask me for a room link on irc.

Comment 11

[Sevaan Franks [:sevaan]]

I'm having a hard time getting it to work. I'm on the latest build and am using your updated pref in a new profile.

Still, I can see the screens and think we need to tweak the visuals a bit to fit with the rest of our screens. I'll file a bug for that.

Comment 12

[Romain Testard [:RT]]

Thanks Mark, it works fine for me on the standalone client although it has not been implemented in the  nightly desktop client?

Comment 13

[:shell escalante]

uplift for desktop, there's minor string change to pick up in 37 - but not breaking.  stand-alone will be as soon as next update.

Comment 14

[Mark Banner (:standard8)]

Attachment: Loop feedback form should always allow comments.

This does the necessary changes to make the form always allow comments - Sevaan and RT both like the new functional changes (this version also fixes tests).

Comment 15

[Mark Banner (:standard8)]

Attachment: Loop feedback form should always allow comments.

The full patch this time, forgot the -a on amending the commit.

Comment 16

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

Comment on attachment 8544666 [details] [diff] [review]
Loop feedback form should always allow comments.

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

Works great, thanks!

Comment 17

[Mark Banner (:standard8)]

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

Will do a patch for aurora/beta later.

Comment 18

[Mark Banner (:standard8)]

Attachment: Loop feedback form should always allow comments.

Patch for aurora without the string id changes - the colon change will be picked up by locales in the next versions, but lets do the best we can for now.

Comment 19

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/91117e6349d0

Comment 20

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

Comment on attachment 8545415 [details] [diff] [review]
Loop feedback form should always allow comments.

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

LGTM. I've added comments from an IRC discussion to explain the string change.

::: browser/components/loop/standalone/content/l10n/en-US/loop.properties
@@ +71,5 @@
>  feedback_category_audio_quality=Audio quality
>  feedback_category_video_quality=Video quality
>  feedback_category_was_disconnected=Was disconnected
>  feedback_category_confusing=Confusing
> +feedback_category_other=Other

[15:34:33]  Standard8:	so basically, I see it as although its changing the context of the string, its doesn’t actually matter too much, in that it’ll just lead to some possibly broken user assumptions for non-en-US locales.
[15:35:03]  Standard8:	but the broken assumptions will only be that it relates to the “other” category, and not the rest as well
[15:35:34]  Standard8:	We are allowed to change strings for punctuation errors (basically non-context) without changing the id
[15:39:07]  Standard8:	https://developer.mozilla.org/en-US/docs/Mozilla/Localization/Localization_content_best_practices#Changing_existing_strings

Comment 22

[Maire Reavy [:mreavy]]

We decided to let this ride the Fx37 train (just updating the Fx36 blocking flag for clarity).