Closed Bug 1170627 Opened 5 years ago Closed 5 years ago

Save Context button doesn't exit screen

Categories

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

defect
Points:
2

Tracking

(firefox40+ verified, firefox41 verified)

VERIFIED FIXED
mozilla41
Iteration:
41.3 - Jun 29
Tracking Status
firefox40 + verified
firefox41 --- verified

People

(Reporter: rpapa, Assigned: mikedeboer)

References

Details

(Whiteboard: [context bug])

Attachments

(2 files, 1 obsolete file)

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.
Seems reasonable - but what's your take Sevaan?  we'll try to get in before released if you say so.
Flags: needinfo?(sfranks)
Whiteboard: [context]
Flags: firefox-backlog+
The button should close the edit screen.
Flags: needinfo?(sfranks)
Rank: 11
Priority: -- → P1
Whiteboard: [context] → [context bug]
Assignee: nobody → mdeboer
Iteration: --- → 41.3 - Jun 29
Flags: qe-verify+
Status: NEW → ASSIGNED
Points: --- → 2
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.
Attachment #8617267 - Flags: review?(standard8) → review-
Attachment #8617267 - Attachment is obsolete: true
Attachment #8617877 - Flags: review?(standard8)
Attachment #8617877 - Flags: review?(standard8) → review+
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
https://hg.mozilla.org/mozilla-central/rev/1ad9bfdbbba4
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
QA Contact: bogdan.maris
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).
Status: RESOLVED → VERIFIED
Flags: qe-verify+
[Tracking Requested - why for this release]:
Email from Florin says that 40 Beta is affected, in that case we should track and uplift.
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.
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.
Attachment #8634017 - Flags: review+
Attachment #8634017 - Flags: approval-mozilla-beta?
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.
Attachment #8634017 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: qe-verify+
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)
Flags: qe-verify+
Depends on: 1175833
You need to log in before you can comment on or make changes to this bug.