1155421 - [Messages] Change "Discard" string to "Delete Draft' to clarify that draft will be deleted, rather than your last changes since opening

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::SMS, defect)

Description

[Oliver Nelson [:oliverthor]]

Attachment: logcat_20150416_1549.txt

Description:
The user has a handful of UI options when observing drafts. Leaving an unsent message for the first time will present the user with:
- Save as Draft
- Discard 
Saving the draft will leave it in your messages app, which can then be opened again. Once in this draft, making changes and attempting to leave will present:
- Replace existing Draft
- Discard
Discard in both cases will DELETE the draft from the app, but there is no UX for 'discarding' last changes to preserve the draft you had initially.
Similar to this issue that was resolved for the E-Mail app: https://bugzil.la/1026967

Repro Steps:
1) Update a Flame to 20150416010206
2) Open the messages app
3) Create a new message
4) Type a few characters
5) Leave the message: Save as Draft
6) Open the draft
7) Type a few characters
8) Leave the message: Discard
9) Observe messages

Actual:
Messages draft will delete draft when 'Discarding' changes

Expected:
Message draft will preserve draft when 'Discarding' changes


Environmental Variables:
--------------------------------------------------

Device: Flame 3.0
Build ID: 20150416010206
Gaia: 629097847567e51095a454e7e63186a6e2ac0307
Gecko: a35163f83d22
Gonk: b83fc73de7b64594cd74b33e498bf08332b5d87b
Version: 40.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0

Device: Flame 2.2
BuildID: 20150416002504
Gaia: 8e24d8b7f5e7c74c3004b22710dda0dac3e04ead
Gecko: 41388836b5c6
Gonk: ebad7da532429a6f5efadc00bf6ad8a41288a429
Version: 37.0 (2.2) 
Firmware Version: v18D
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Device: Flame 2.1
BuildID: 20150416001203
Gaia: bbe983b4e8bebfec26b3726b79568a22d667223c
Gecko: c54aa1be51d6
Gonk: ebad7da532429a6f5efadc00bf6ad8a41288a429
Version: 34.0 (2.1) 
Firmware Version: v18D
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
--------------------------------------------------

Repro frequency: 5/5
See attached: 
video- https://youtu.be/hkfiWuRrKOc
logcat

Comment 1

[Julien Wajsberg [:julienw]]

I understand what you mean.

NI Jenny, what do you think?

Note that we'll likely eventually remove this dialog to just save the draft, so we won't have the possibility to delete or discard the last changes.

Comment 2

[Jenny Lee]

Hello Julien,

I think the request makes sense, let's make the change "Discard" -> "Delete draft" for now. Thanks!

Comment 3

[Julien Wajsberg [:julienw]]

This should be quite easy to fix running the application in Firefox as explained in [1] so let's make this a mentored bug !

Note to a contributor: when changing the text, we need to change the l10n key as well because this is how the localizers know something has changed.

[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/sms/README.md

Comment 4

[Julien Wajsberg [:julienw]]

Hey Matej, can you please have a quick look on the string change here? As explained in comment 0, the action will always delete the full draft and not discard only the new changes. I wonder if "discard draft" could work as well. Or if we need the article "the" ("discard the draft" or "delete the draft"). My feeling is that "Delete draft" has quite a technical feel.

Comment 5

[Aron Barreira Bordin]

Hi Julien!

I'd like to be assigned to work with this bug after you get the necessary info about the correct string. 

"when changing the text, we need to change the l10n key as well because this is how the localizers know something has changed." How can I get this new string translated to other languages?

Comment 6

[Matej Novak [:matej]]

(In reply to Julien Wajsberg [:julienw] from comment #4)
> Hey Matej, can you please have a quick look on the string change here? As
> explained in comment 0, the action will always delete the full draft and not
> discard only the new changes. I wonder if "discard draft" could work as
> well. Or if we need the article "the" ("discard the draft" or "delete the
> draft"). My feeling is that "Delete draft" has quite a technical feel.

I think in this case it's OK to sound a bit technical to keep it brief, though I think "discard" is less friendly than "delete" so I'd go with "delete draft."

Comment 7

[Julien Wajsberg [:julienw]]

Hey Aron, thanks a lot ! I added you as assignee to the bug :)

In Gaia we only use the english locale; strings get translated by our localizers later, usually when a version is getting stabilized. When we change the l10n key, localizers actually see there is a _new_ key to translate.

Hope this is clearer !

Comment 8

[GitHub Autolander Bot]

Attachment: [gaia] aron-bordin:change_string_message_draft > mozilla-b2g:master

Comment 9

[Julien Wajsberg [:julienw]]

Comment on attachment 8595650 [details] [review]
[gaia] aron-bordin:change_string_message_draft > mozilla-b2g:master

Thanks, this looks good !

Please update the unit tests as advised on github, and this should be fine.

Please add your next changes in a separate commit so that it's easier for me to review again, and flip the "review" flag again once you're ready.

Thansk again for your help on this :)

Comment 10

[Aron Barreira Bordin]

Comment on attachment 8595650 [details] [review]
[gaia] aron-bordin:change_string_message_draft > mozilla-b2g:master

Hi!

When possible, check if it's ok now.

Thx

Comment 11

[Julien Wajsberg [:julienw]]

Comment on attachment 8595650 [details] [review]
[gaia] aron-bordin:change_string_message_draft > mozilla-b2g:master

An integration test is failing. You can find more information on github.

Please flip the r flag when you're ready :) Thanks !

Comment 12

[Aron Barreira Bordin]

Comment on attachment 8595650 [details] [review]
[gaia] aron-bordin:change_string_message_draft > mozilla-b2g:master

Hi Julien!

I fixed the errors, now all tests are passing. Please, when possible, check if patch is ok. 

Thx!

Comment 13

[Julien Wajsberg [:julienw]]

Comment on attachment 8595650 [details] [review]
[gaia] aron-bordin:change_string_message_draft > mozilla-b2g:master

This now looks good, I can put a r+ on this patch, thanks a lot :)

Can you now squash the commits into one commit ?
The first line of the commit log should read something like:

  Bug 1155421 - [Messages] Change "Discard" string to "Delete Draft' to clarify that draft will be deleted r=julien

What's needed is the bug number, the title of the bug, and the name of the reviewer (that's me).

You can add some more information if you want (but in the case of this patch it's not really necessary).

Once you're ready, just add the "checkin-needed" keywords in the "keywords" section of the bug, and our Autolander bot should come and land it after running the tests once again.

I'll watch from the outside anyway so don't worry :)

Thanks again, and congrats for this first bug :) now you know everything to move on a more complicated bug !

Comment 14

[GitHub Autolander Bot]

https://github.com/mozilla-b2g/gaia/pull/29653

Autolander could not land the pull request due to not having collaborator rights. This is possibly due to a tree closure. Please check the tree status and request checkin again once the tree is open.

Comment 15

[Julien Wajsberg [:julienw]]

Gaia tree status is at https://treestatus.mozilla.org/Gaia
Let's wait that it's open again...

Comment 16

[GitHub Autolander Bot]

Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/95ba4752c9371a0fea4c5e5dc51e56f2efcc99df

Comment 17

[Julien Wajsberg [:julienw]]

Well done !

Comment 18

[Shally Li（leave from Mozilla）]

This bug has been verified as pass on latest Nightly build of Flame v3.0 and Nexus 5 v3.0 by the STR in Comment 0.

Actual results: It shows "Delete draft" after saving a draft or changing the draft.
See attachment: verified_v3.0.png
Reproduce rate: 0/6


Device: Flame v3.0 build(Pass)
Build ID               20150601160204
Gaia Revision          6d477a7884273886605049b20f60af5c1583a150
Gaia Date              2015-06-01 16:41:42
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/56241c1f8a3b
Gecko Version          41.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150601.193935
Firmware Date          Mon Jun  1 19:39:44 EDT 2015
Bootloader             L1TC000118D0

Device: Nexus 5 v3.0 build(Pass)
Build ID               20150601075320
Gaia Revision          85e6fcef45c0cb2c017739df42b68b96cf5bb9c3
Gaia Date              2015-06-01 06:40:19
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/666b584fb521
Gecko Version          41.0a1
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.cltbld.20150601.112653
Firmware Date          Mon Jun  1 11:27:09 EDT 2015
Bootloader             HHZ12f

Comment 19

[Shally Li（leave from Mozilla）]

Attachment: verified_v3.0.png