1170627 - Save Context button doesn't exit screen

Status: VERIFIED FIXED

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

Description

[Richard Pappalardo [:rpapa][:rpappalardo]]

Hello Firefox 'Save Context' button on Nightly (41.0a1) doesn't exit from that screen (though it does save the context that I enter).  To the user, it appears that they are stuck!  I have to click the little 'x' in the upper right-hand corner to exit that menu and proceed.  

------------------
STR
------------------

1. Click on hello Icon in Nightly browser
2. Click on "Start a Conversation" button
3. Click on "add some context"
4. Enter context
5. Click on "Save Context" button
6. Bug: Observe nothing happen
7. Now, in order to exit this screen, I realize that I'll have to click on small 'x' icon to the right of "Let's talk about" 
8. In the next screen, I observe in top of window that context subject has been saved.

------------------
EXPECTED
------------------

1. Click on Hello Icon in Nightly browser
2. Click on "Start a Conversation" button
3. Click on "add some context"
4. Enter context
5. Click on "Save Context" button
6. Observe that clicking "Save Context" button properly closes save context screen allowing you to proceed to next step.
7. Observe in top of window that context subject has been saved.

Comment 1

[:shell escalante]

Seems reasonable - but what's your take Sevaan?  we'll try to get in before released if you say so.

Comment 2

[Sevaan Franks [:sevaan]]

The button should close the edit screen.

Comment 3

[Mike de Boer [:mikedeboer]]

Attachment: Patch v1: close the context edit form upon a successful save

Comment 4

[Mark Banner (:standard8)]

Comment on attachment 8617267 [details] [diff] [review]
Patch v1: close the context edit form upon a successful save

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

::: browser/components/loop/content/js/roomStore.js
@@ +512,5 @@
>  
>          // When no properties have been set on the roomData object, there's nothing
>          // to save.
>          if (!Object.getOwnPropertyNames(roomData).length) {
> +          setTimeout(function() {

I think this is to ensure async actions so that we get separate setStoreState events. If so, please add a comment, if not, please explain.

::: browser/components/loop/test/desktop-local/roomViews_test.js
@@ +878,5 @@
>                newRoomThumbnail: fakeContextURL.thumbnail
>              }));
>          });
> +
> +      it("should close the edit form when context was saved successfully", function(next) {

nit: Prefer to use done rather than next, to be consistent with what we've been doing elsewhere.

::: browser/locales/en-US/chrome/browser/loop/loop.properties
@@ +350,5 @@
>  context_add_some_label=Add some context
>  context_edit_tooltip=Edit Context
>  context_hide_tooltip=Hide Context
>  context_show_tooltip=Show Context
> +context_save_label=Save

I think this needs a string id change for landing in m-c to ensure locales pick up the change.

For branch, we'll probably want to stick with the en-US change only (no id change) - i.e. not all locales will pick it up.

Comment 5

[Mike de Boer [:mikedeboer]]

Attachment: Patch v2: close the context edit form upon a successful save

Comment 6

[Pulsebot]

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

Comment 7

[Carsten Book [:Tomcat]]

sorry backed out the commit from comment 6 because i was confused by treeherder. Sorry :( corrected this now and re-checked this in as https://hg.mozilla.org/integration/fx-team/rev/1ad9bfdbbba4

Comment 8

[Pulsebot]

Backout:
https://hg.mozilla.org/integration/fx-team/rev/4776477b6357

Comment 9

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

https://hg.mozilla.org/mozilla-central/rev/1ad9bfdbbba4

Comment 10

[Bogdan Maris [:bogdan_maris], Desktop QA]

Reproduced the initial issue on old Nightly build (2015-06-09), verified that the issue is fixed on latest Nightly across platforms (Windows 7 64-bit, Mac OS X 10.9.5 and Ubuntu 14.04 32-bit).

Comment 11

[Robert Kaiser]

[Tracking Requested - why for this release]:
Email from Florin says that 40 Beta is affected, in that case we should track and uplift.

Comment 12

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Let's try to fix this for FF40 as it's not ideal for end-users to click on the 'x' to close the window when "save context" button should be doing that after saving the context.

Comment 13

[Mike de Boer [:mikedeboer]]

Attachment: Patch v2.beta: without the string changes

Approval Request Comment
[Feature/regressing bug #]: Hello context in conversations feature
[User impact if declined]: The edit context form inside the conversation window will not close automatically after a successful save.
[Describe test coverage new/current, TreeHerder]: already on m-a, covered by unit tests.
[Risks and why]: 
[String/UUID change made/needed]: I removed the string changes from the patch, they're not critical to be added in Fx40.

Carrying over r=Standard8.

Comment 14

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8634017 [details] [diff] [review]
Patch v2.beta: without the string changes

We want this feature to be as polished as possible. taking it.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/a212800c773f

Comment 16

[Bogdan Maris [:bogdan_maris], Desktop QA]

Verified that the issue is fixed on Firefox 40 beta 6 across platforms (Windows 10 32-bit, Windows 8.1 64-bit, Mac OS X 10.10 and Ubuntu 14.04 32-bit)