Closed Bug 813689 Opened 12 years ago Closed 11 years ago

[SMS] Characters counter in SMS application

Categories

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

Product:
Firefox OS Graveyard
Component:
Gaia::SMS
Type:
defect

Tracking

(blocking-basecamp:+)

Status:
VERIFIED FIXED
Milestone:
B2G C3 (12dec-1jan)
Project Flags:
blocking-basecamp +

People

(Reporter: brg, Assigned: vingtetun)

References

Details

(Keywords: feature, Whiteboard: [target:12/21], UX-P1)

Attachments

(1 file)

Patch
4.64 KB, patch
kaze
: review+
Details | Diff | Splinter Review 
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:16.0) Gecko/20100101 Firefox/16.0
Build ID: 20121024073032

Steps to reproduce:

there is no information of how huge is the SMS or how many more characters I can type before having to send two SMS. 


Actual results:

user should know when the length of the SMS is close to the max size.
Component: General → Gaia::SMS
blocking-basecamp: --- → ?
Priority: -- → P3
Whiteboard: UX_QA
Assignee: nobody → fbsc
blocking-basecamp: ? → +
Priority: P3 → P1
Mass Modify: All un-milestoned, unresolved blocking-basecamp+ bugs are being moved into the C3 milestone. Note that the target milestone does not mean that these bugs can't be resolved prior to 12/10, rather C2 bugs should be prioritized ahead of C3 bugs.
Target Milestone: --- → B2G C3 (12dec-1jan)
Whiteboard: UX_QA → UX_QA [target:12/21]
Why are we blocking in this? This is not in the original requirements, and doesn't seem like something we'd hold the release for.
blocking-basecamp: + → ?
Keywords: feature
This missed the feature complete milestone by a few months. Its a feature we can live without. We can put it on the v1.1 train. If this is a P1 requirement, and we decide to re-open the feature scope, we also would have to re-open the schedule. As a result I think this is b- in my opinion. Daniel, can you confirm as well please.
Flags: needinfo?(dcoloma)
Andreas, Dietrich: I've sent you a private e-mail about this.
Flags: needinfo?(dcoloma)
Necessary for certification requirements.  Marking as blocking.
blocking-basecamp: ? → +
Hi Casey,
please ref https://bugzilla.mozilla.org/show_bug.cgi?id=813682#c7, thanks
Flags: needinfo?(kyee)
Assignee: fbsc → schung
Flags: needinfo?(kyee) → needinfo?(aymanmaat)
working on it as part of Bug 813682
Flags: needinfo?(aymanmaat)
note to self
Flags: needinfo?(aymanmaat)
UX architecture to feedback to users on how many more characters can be typed before having to send another SMS is contained in Bug 813682

RFI to Steve Chung - should we merge these bugs?
Flags: needinfo?(aymanmaat) → needinfo?(schung)
(In reply to ayman maat :maat from comment #9)
> UX architecture to feedback to users on how many more characters can be
> typed before having to send another SMS is contained in Bug 813682
> 
> RFI to Steve Chung - should we merge these bugs?

I see your point, you will fix 2 issues at the same time but the issues are different, I do not want to see them merged... because they are two different issues/things/features.
Hi, 

I include this issue as a certification blocker for v1. 

Thx!
David
Blocks: b2g-v1-certification
(In reply to Beatriz Rodríguez [:brg] from comment #10)
> (In reply to ayman maat :maat from comment #9)
> > UX architecture to feedback to users on how many more characters can be
> > typed before having to send another SMS is contained in Bug 813682
> > 
> > RFI to Steve Chung - should we merge these bugs?
> 
> I see your point, you will fix 2 issues at the same time but the issues are
> different, I do not want to see them merged... because they are two
> different issues/things/features.

I lean more toward Beatriz. Even these fixing would be fixed in one patch, it would be better to trace them independently. These counter/indicator returned by API are separated, we will not compute these numbers in gaia layer.
Flags: needinfo?(schung)
Will this counter also include the number of SMS which will be send out when the user hits send? Means how do we support multi-part SMS?
Status: UNCONFIRMED → NEW
Ever confirmed: true
(In reply to Henrik Skupin (:whimboo) from comment #14)
> Will this counter also include the number of SMS which will be send out when
> the user hits send? Means how do we support multi-part SMS?

yes. please refer to attachment of Bug 813682
Attached patch Patch — — Splinter Review 
Kaze has already looked at operator variant for 7bit-encoding so let's ask him the review since the code does not really alter any existing sms code but just add a very isolated method.
Assignee: schung → 21
Attachment #693927 - Flags: review?(kaze)
Comment on attachment 693927 [details] [diff] [review]
Patch

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

r=me for the code.

UX-wise, I’m a bit confused that we only display the number of allowed characters once that we’re *over* the one-message limit. I would expect to be warned that I’m going to get over the one-message limit *before* reaching it…

Josh, if the current behavior is not OK for you, please open a follow-up.
Attachment #693927 - Flags: review?(kaze) → review+
Flags: needinfo?(jcarpenter)
(In reply to Fabien Cazenave [:kaze] from comment #17)
> Comment on attachment 693927 [details] [diff] [review]
> Patch
> 
> Review of attachment 693927 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r=me for the code.
> 
> UX-wise, I’m a bit confused that we only display the number of allowed
> characters once that we’re *over* the one-message limit. I would expect to
> be warned that I’m going to get over the one-message limit *before* reaching
> it…
> 
> Josh, if the current behavior is not OK for you, please open a follow-up.

specified behavior equalizes android UX. further justification can be found in comment 13 of bug 813682.
(In reply to ayman maat :maat from comment #19)
> specified behavior equalizes android UX. further justification can be found
> in comment 13 of bug 813682.

Hm, that's not fully true. Android shows the counter in the first message once the user crosses the 10 left character limit. That helps in adjusting the message so it will fit into a single one.
(In reply to Henrik Skupin (:whimboo) [away 12/21 - 01/01] from comment #20)
> (In reply to ayman maat :maat from comment #19)
> > specified behavior equalizes android UX. further justification can be found
> > in comment 13 of bug 813682.
> 
> Hm, that's not fully true. Android shows the counter in the first message
> once the user crosses the 10 left character limit. That helps in adjusting
> the message so it will fit into a single one.

My mistake. I was referring to a different OS when i wrote this comment. Spinning one too many plates at once there...
(In reply to Fabien Cazenave [:kaze] from comment #17)
> Josh, if the current behavior is not OK for you, please open a follow-up.

I think it's a valid request and went ahead. Bug 823437 has been filed which will cover that remaining issue.
I defer to Ayman here, given his ownership of SMS UX. Sounds like we've managed to add this feature, despite the requirement surfacing so late. Nice work.
Flags: needinfo?(jcarpenter)
Whiteboard: UX_QA [target:12/21] → [target:12/21], UX-P1
This one is not working properly (at least there is no indicator in today's build). Following https://bugzilla.mozilla.org/show_bug.cgi?id=813682#c16 we must reopen this one and probably backout the changes, because the implementation is not using the methods available in Gecko. This is so important because this is part of certification requirements should be treated as high priority.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(In reply to Borja Salguero [:borjasalguero] from comment #24)
> This one is not working properly (at least there is no indicator in today's
> build). Following https://bugzilla.mozilla.org/show_bug.cgi?id=813682#c16 we

It's not broken for me. But I wonder if you are seeing bug 823437. See comment 22.
Yep, but one thing is the 'visual' stuff, which probably is right, and other is the functionality. The thing is that when I was talking with Vicamo and Steve about how to implement the patch to this bug we stablished some functionalities to be used in Gaia, and I dont see any Gecko request in the code of the patch (https://bug813689.bugzilla.mozilla.org/attachment.cgi?id=693927)... so probably, despite it seems to be working fine, it's not. I need more info from Vicamo.
[:vicamo] What should we do here? Backout and create a new patch? Or the current one is valid?
Flags: needinfo?(vyang)
Hi Borja,
I think we can replace the logic inside updateCounter with the backend api. There is no need to revert the patch totally :)
Hi Steve!,
Ok, for me it's ok. Do you want to work on it? I dont know if you have some patch almost done for this issue. BTW I could work on it next week (I will come back from my Holidays following Monday!). Thanks for the info! ;)
Since https://bugzilla.mozilla.org/show_bug.cgi?id=820780 already passed all the reviews, it should be landed in this week. I'll apply the patch right after that and you can review it next Monday, so have a nice holiday!
Assignee: 21 → schung
(In reply to Steve Chung from comment #28)
> Hi Borja,
> I think we can replace the logic inside updateCounter with the backend api.
> There is no need to revert the patch totally :)

What Steve says. Also let's open a new bug explaining what to change instead of reopening old ones.
Assignee: schung → 21
Status: REOPENED → RESOLVED
Closed: 12 years ago → 11 years ago
Resolution: --- → FIXED
Flags: needinfo?(vyang)
Refer to this bug for Further information 
https://bugzilla.mozilla.org/show_bug.cgi?id=823437

Unagi Build 20130103070201
This issue is fixed for  device Unagi; build #20130113070202v.1, sms character count shows a the start of second page
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: