Closed Bug 836690 Opened 10 years ago Closed 8 years ago

[SMS] Implement temporary in App message informing the user that they have started another SMS packet

Categories

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

x86
macOS
defect
Not set
normal

Tracking

(tracking-b2g:backlog)

RESOLVED FIXED
2.1 S5 (26sep)
tracking-b2g backlog

People

(Reporter: maat, Assigned: jorgep, Mentored)

References

Details

(Whiteboard: [lang=js][interaction][UX-P2][priority])

Attachments

(5 files)

**DESCRIPTION**
Referencing:
Wireframe pack: HTML5_SMS_20121212_R2S1_V8.0
Page: 17
Annotation: 01

The temporary in App message informing the user that they have started another SMS packet still needs to be implemented. I am open to alternative suggestion on how it is articulated, but it needs to be implemented.
Whiteboard: interaction [UX-P2]
We're using a counter which is displayed on top of the input field. Is that the message you mean or should we use something more prominent like a banner?
Would like to have Ayman's confirmation, but so far I can see the counter in all the branches and builds I tested.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → WORKSFORME
hey fernando

am going to reopen as the banner is still missing when i review the messaging app. ping me to discuss..
Status: RESOLVED → REOPENED
Flags: needinfo?(fernando.campo)
Resolution: WORKSFORME → ---
Oh, I didn't understand this was needed to be implemented as a banner, I thought it was enough with the counter, now I see why it was still open.

Any flags needed for this?
Flags: needinfo?(fernando.campo)
Apparently this is needed asap, so nominating
blocking-b2g: --- → tef?
Definitely not a 1.0.1 blocker :)
blocking-b2g: tef? → ---
hey Ayman,

do you still want the "new segment" banner in the SMS app?
Flags: needinfo?(aymanmaat)
(In reply to Julien Wajsberg [:julienw] from comment #7)
> hey Ayman,
> 
> do you still want the "new segment" banner in the SMS app?

yes please. tbh the numbering above the send button is ambiguous on its own to less technically knowledgable users so the temp segment banner acts as disambiguation to what is going on
Flags: needinfo?(aymanmaat)
Ayman, do we need it for each segment?

Asking 1.4? so that it's get moved to the backlog.
blocking-b2g: --- → 1.4?
Flags: needinfo?(aymanmaat)
Whiteboard: interaction [UX-P2] → [mentor=:julienw][interaction][UX-P2]
(In reply to Julien Wajsberg [:julienw] from comment #9)
> Ayman, do we need it for each segment?
> 
> Asking 1.4? so that it's get moved to the backlog.

Hey Julien, 

yes please. we should display the banner as the user crosses the boundary backwards and forwards over each segment. 

As an FYI for backlog this was last specified over a year ago in HTML5_SMS_20121212_R2S1_V8.0, page 17...it wss in earlier versions of the pack.
Flags: needinfo?(aymanmaat)
Ayman (I should ask all my questions in one go :p), is the text "SMS 2" final or should we refine it with José or Tyler? And what about the "centered" style?
Flags: needinfo?(aymanmaat)
(In reply to Julien Wajsberg [:julienw] from comment #12)
> Ayman (I should ask all my questions in one go :p), is the text "SMS 2"
> final or should we refine it with José or Tyler? And what about the
> "centered" style?

Feel free to ask those questions as and when they come my friend :)

..Tyler is your man as he is overseeing copy. (jose is currently on holiday for the next month in any case)
ni? to Tyler

regarding the centred alignment of text: the wireframe is just indicative so follow whatever alignment convention we are using for these temporary in app banners (i think it is left justified).
Flags: needinfo?(aymanmaat) → needinfo?(tyler.altes)
This is the first I've seen of this, and I don't quickly find the proposed text or screens, but I believe we're talking about a banner that tells the user when the SMS exceeds the limit for a single message and is going to be split into more than one, right?

If so, I would put: "This will be sent as 2 separate SMS messages."  (changing the 2 for the appropriate number where necessary)
Flags: needinfo?(tyler.altes)
(In reply to tyler.altes from comment #14)
> This is the first I've seen of this, and I don't quickly find the proposed
> text or screens, but I believe we're talking about a banner that tells the
> user when the SMS exceeds the limit for a single message and is going to be
> split into more than one, right?
> 
> If so, I would put: "This will be sent as 2 separate SMS messages." 
> (changing the 2 for the appropriate number where necessary)

hey tyler

Thanks for your quick response. The gist of what your prescribing is correct however i am a little concerned that it is a bit too long. I would like to keep this notification tight and to a single line if possible. Could we have another iteration?

for your reference you can find the wireframe proposition on page 17 of the document Julien attached : attachment 8348776 [details]
Flags: needinfo?(tyler.altes)
Note that the banner will be transient too.
Ah ok, now I see it. With that short space and quick/nondisruptive message, maybe something as simple as: "2 messages will be sent"

How well does that fit the banner?
Flags: needinfo?(tyler.altes)
Sounds good to me.
This is a basic feature for informing user that more money is engaged in sending this message. Shall we take it as a target or committed ?
Flags: needinfo?(wmathanaraj)
Flags: needinfo?(jmunuer)
I consider this as a feature change and our focus for v1.4 is going to be DSDS. This is not a bug and the implementation follows what the requirements were.

This change should be done as part of the UX refresh work - suggest doing this in v1.5
Flags: needinfo?(wmathanaraj)
add this to backlog
blocking-b2g: 1.4? → backlog
Flags: needinfo?(jmunuer)
Flags: needinfo?(ofeng)
Whiteboard: [mentor=:julienw][interaction][UX-P2] → [mentor=:julienw][interaction][UX-P2][Top25]
Whiteboard: [mentor=:julienw][interaction][UX-P2][Top25] → [mentor=:julienw][interaction][UX-P2][priority]
Whiteboard: [mentor=:julienw][interaction][UX-P2][priority] → [mentor=:julienw][lang=javascript][interaction][UX-P2][priority]
Whiteboard: [mentor=:julienw][lang=javascript][interaction][UX-P2][priority] → [mentor=:julienw][lang=js][interaction][UX-P2][priority]
Flags: needinfo?(ofeng)
Mentor: felash
Whiteboard: [mentor=:julienw][lang=js][interaction][UX-P2][priority] → [lang=js][interaction][UX-P2][priority]
Assignee: nobody → jpruden92
Status: REOPENED → ASSIGNED
Hello Carrie, I have done one patch for this, can you review the result?

https://www.youtube.com/watch?v=2snppjtG9nU
Flags: needinfo?(cawang)
Flags: needinfo?(cawang) → needinfo?(jelee)
Attached image in app message.png
I think it looks fine except for the styling and placement, all in app message should look alike (see attached). Can you make the change accordingly? Thanks!
Flags: needinfo?(jelee)
Hello Jenny, I have changed the message's design. Can you review it?

https://www.youtube.com/watch?v=9-TcI-JAYLw

Thanks a lot!
Flags: needinfo?(jelee)
looks great! Thank you Jorge!
Flags: needinfo?(jelee)
Attached file 23564.html
Attachment #8482196 - Flags: review?(schung)
Comment on attachment 8482196 [details]
23564.html

Hi Jorge, sorry for the decline, but see my comments on github for more information. Please follow the existing convention and reuse styling instead of inventing new wheels. The most important reason I decline the patch is it might break the current recipient list behavior. And please remember to add unit test for your changes. Thanks!
Attachment #8482196 - Flags: review?(schung) → review-
Hi ,
In the old message UX spec https://bug836690.bugzilla.mozilla.org/attachment.cgi?id=8348776 page 17, We will need to display a toaster to let user understand that they are editing another message segment, currently we simply use 'SMS' + segment number for display, but Jenny and I thought we should apply more meaningful string like 'Editing sms segment '+ segment number. Could you please give any suggestion about the string? Thanks.
Flags: needinfo?(Mnovak)
I have updated the PR with your comments. I think that now is a cleaner solution. Can you revise it?

Thanks for your help :-)
Flags: needinfo?(schung)
(In reply to Steve Chung [:steveck] from comment #28)
> Hi ,
> In the old message UX spec
> https://bug836690.bugzilla.mozilla.org/attachment.cgi?id=8348776 page 17, We
> will need to display a toaster to let user understand that they are editing
> another message segment, currently we simply use 'SMS' + segment number for
> display, but Jenny and I thought we should apply more meaningful string like
> 'Editing sms segment '+ segment number. Could you please give any suggestion
> about the string? Thanks.

Hi Steve,

I'm not sure I fully understand what an "SMS segment" is. Could you give me a little more information about this functionality? I want to make sure it's clear to the user who encounters it.

Thanks.
Flags: needinfo?(Mnovak)
Ah, sorry for the unclear information. For the short story, A text message actually could be concatenated by multiple segments, and each segment has a limited length of content. For example, if this segment limitation is 160 chars, and user typed 200 chars in sms. This message is actually concatenated by 2 segment, and user would be billed for additional segment. There is why UX want to show a toaster to let user know that they are editing another sms segment. For the complete story, please refer the wiki page[1] about concatenated SMS, thanks!

[1] http://en.wikipedia.org/wiki/Concatenated_SMS
Flags: needinfo?(schung) → needinfo?(Mnovak)
(In reply to Jorge Prudencio [:jorgep] from comment #29)
> I have updated the PR with your comments. I think that now is a cleaner
> solution. Can you revise it?
> 
> Thanks for your help :-)

I replied on the github, and it's much better now, thanks!
I've already done the changes that you proposed. I have added the unit tests too.
Flags: needinfo?(schung)
(In reply to Steve Chung [:steveck] from comment #31)
> Ah, sorry for the unclear information. For the short story, A text message
> actually could be concatenated by multiple segments, and each segment has a
> limited length of content. For example, if this segment limitation is 160
> chars, and user typed 200 chars in sms. This message is actually
> concatenated by 2 segment, and user would be billed for additional segment.
> There is why UX want to show a toaster to let user know that they are
> editing another sms segment. For the complete story, please refer the wiki
> page[1] about concatenated SMS, thanks!
> 
> [1] http://en.wikipedia.org/wiki/Concatenated_SMS

Thanks for the added info. That makes sense.

Instead of saying "SMS segment," could we say "Editing SMS" + segment number? Calling it a segment makes me worry that the extra charges won't be clear. Even though it's a segment of one long SMS, it's technically a separate message.

Do you think that would work? Or would it just make it more confusing?
Flags: needinfo?(Mnovak)
Thanks Matej, I agree that using word 'segment' might not able to provide clear information, so current solution sounds fine to me.

Hi Jorge, still left some comments on the github for your test part, and please see comment 34 and adjust the toaster string part, thanks!
Flags: needinfo?(schung)
Comment on attachment 8482196 [details]
23564.html

Hello Steve, I have already done the changes form Github and I have modified the text. Can you review it?

Thanks!
Attachment #8482196 - Flags: review- → review?(schung)
Comment on attachment 8482196 [details]
23564.html

Still left some comments on github, and please fix the conflicts as well, thanks!
Attachment #8482196 - Flags: review?(schung)
Comment on attachment 8482196 [details]
23564.html

Changes done! Thanks!
Attachment #8482196 - Flags: review?(schung)
Comment on attachment 8482196 [details]
23564.html

Hope it's the last comments here, but you have to rebase your branch to make you patch able to merge first ;) Please fetch the upstream and rebase upstream/master again, thanks!
Attachment #8482196 - Flags: review?(schung)
Attachment #8482196 - Flags: review?(schung)
Changes done :-)
You could see the gaia try failed in Gaia unit test, and I left some comments for it and other suggestion.
Attachment #8482196 - Flags: review?(schung)
Comment on attachment 8482196 [details]
23564.html

Hello Steve,

I have applied the changes and now all tests work fine.

Thanks!
Attachment #8482196 - Flags: review?(schung)
Comment on attachment 8482196 [details]
23564.html

Look great now, thanks for the great help here!
Attachment #8482196 - Flags: review?(schung) → review+
In master : c1327184fbdb62113a25df1198e89e9547a65969
Status: ASSIGNED → RESOLVED
Closed: 9 years ago8 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.1 S5 (26sep)
Steve, can we add a localization comment to this string? Currently is definitely unclear. I'd expect people to consider number as a phone number. For example:

# LOCALIZATION NOTE (sms-counter-notice-label): if typed message exceeds the
# character limit, more segments (messages) will be sent. {{number}} is the
# number of segments. Example: 200 characters typed, limit 160 characters,
# string will be 'Editing SMS 2' (editing 2nd segment).

Let me know if you can do it directly or if you need a new bug.
Flags: needinfo?(schung)
(In reply to Francesco Lodolo [:flod] from comment #45)
> Steve, can we add a localization comment to this string? Currently is
> definitely unclear. I'd expect people to consider number as a phone number.
> For example:
> 
> # LOCALIZATION NOTE (sms-counter-notice-label): if typed message exceeds the
> # character limit, more segments (messages) will be sent. {{number}} is the
> # number of segments. Example: 200 characters typed, limit 160 characters,
> # string will be 'Editing SMS 2' (editing 2nd segment).
> 
> Let me know if you can do it directly or if you need a new bug.

Ok, I could do it if this make localization process more clear.
Flags: needinfo?(schung)
Attached file Link to github
Comments only, hope this is clear information for you.
Attachment #8493482 - Flags: review?(francesco.lodolo)
Comment on attachment 8493482 [details] [review]
Link to github

One small variation after reading the change you made to the second part, but it looks great, thanks.
Attachment #8493482 - Flags: review?(francesco.lodolo) → review+
Additional localization comments in master : 47ad8489f3fd5de5e5b1c293b62aade41bb70fd1
blocking-b2g: backlog → ---
You need to log in before you can comment on or make changes to this bug.