Referencing.. Wireframe pack : HTML5_SMS-MMSUserStorySpecifications_20130424_V6.0 Page : 45 Banner required from Visual Design to hold the message the user sees when they reaches the Max Segments in SMS composer Referenced wireframe pack attached. also reference bug: https://bugzilla.mozilla.org/show_bug.cgi?id=843511

RFI to Steve P to provide the necessary visual design components for Juilen to close this bug

Blocking for now, since this seems to go hand in hand with the other tef+ bug 843511, if disabling the send button is enough user feedback though - let's remove the block on this.

Lukas, in bug 843511 we actually did more than merely disabling the send button: we display an alert box. This is a huge user feedback, and probably too huge, hence this bug.

(In reply to lsblakk@mozilla.com from comment #2) > Blocking for now, since this seems to go hand in hand with the other tef+ > bug 843511, if disabling the send button is enough user feedback though - > let's remove the block on this. Disabling the send button is absolutely not the right thing to do for 2 reasons: 1) its not enough feedback for the user as they have no idea why the send button is disabled. 2) **and this is the most important point** ...the user should be able to send a message that is at its maximum length... so disabling the send button at this point is inappropriate. What is appropriate is to prevent the message field from receiving any more the input at the point where the user reaches the max message length so that the user cannot continue to add text content to a message that cannot be sent. (i.e. we stop the error - which is 'message too long to send' - before it happens) When we prevent the user from inputting more text into the text field, they need to be notified why... and this is why this banner is essential. There is also slightly more to this banner. referencing the attached wireframe specification.. The pattern of banner is also required for instances: 1) page 42 Successfully saving attachment from inside a thread 2) page 47, 48, 51 Conversation of SMS to MMS and converting from MMS to SMS referencing the SMS wireframe specification HTML5_SMS_20121212_R2S1_V8.0.. The pattern of banner is also required for this instance: 3) page 17 notification when the user passes between SMS packets (this is currently being brought over into the new specifications) Visual Design for the banner already exists in the email app. It is used to inform the user (for example) when a message has been deleted. All that is required is for this banner to be moved up the screen into the specified position. I therefore do not perceive a big effort at all for Visual Design to complete this task..

(In reply to Julien Wajsberg [:julienw] from comment #3) > Lukas, in bug 843511 we actually did more than merely disabling the send > button: we display an alert box. This is a huge user feedback, and probably > too huge, hence this bug. He julien. Yes it is too big and also presents a barrier to usage... as like i say. it should be the message field that can receive no further text, not the ability to send denied.

yep we must never disable the send button, and we don't. I wanted to explain correctly the current state because I'm really not sure at all that this must be a blocker now. even if it could, I don't mind, but this is a product decision and Lukas comment implies that he didn't get the whole picture.

I mean that I think it is not more important than some Leo+ work we're doing, like MMS stuff. that's why I'm not sure it should be TEF+. but Ayman if you still think this should be then let's go !

Ayman, there is one case you didn't designed. What if the user is in a case where there are more than 10 segments. Should we keep the same banner + disable the send button ? Should we have a different banner ? My preference go to having the same banner so that we don't have another l10n change for v1.0.1. This can happen when switching from 140-character SMS to 70-character SMS because the user inserted some "wrong" character.

Hi Julien, I am attaching the design for the in APP message banner. It is a new element that is not used anywhere in the OS, but as Ayman provided specs for various cases where it's going to be used, so this shu¡ould be a Building Blocks and i am creating a bug to include it as a common element. I've created this bug: 867173, to solve it. Thanks!

Hi Julien, I am attaching the design for the in APP message banner. It is a new element that is not used anywhere in the OS, but as Ayman provided specs for various cases where it's going to be used, so this shu¡ould be a Building Blocks and i am creating a bug to include it as a common element. I've created this bug: 867173, to solve it. Thanks!

(In reply to Julien Wajsberg [:julienw] from comment #8) > Ayman, there is one case you didn't designed. What if the user is in a case > where there are more than 10 segments. Should we keep the same banner + > disable the send button ? Should we have a different banner ? My preference > go to having the same banner so that we don't have another l10n change for > v1.0.1. Hi Juilen. I am not fully understanding how the following case you outlined is triggered and how it affects the number of segments in the SMS message: > This can happen when switching from 140-character SMS to 70-character SMS > because the user inserted some "wrong" character. Could you give me more detail please. Thanks

Ayman, depending on the characters that are in an SMS, it can use different encodings. Depending on these encodings, one segment can contain either 160, 140 or 70 characters. So when you switch from one encoding to another (because eg you entered a character that does not fit the previous encoding, I like using "ê" for example, even if doesn't always trigger the 70-characters encoding), we might double the number of segments with only one more character.

* implement the notification according to UX and Visual design * display this notification if we reach the max length * disable the send button if over 10 segments, but keep it enabled if we have exactly the max length * rewrote the enableSend function to make it more readable and manageable * use the new markup loader in thread_ui_test.js, and make it global --- apps/sms/index.html | 7 + apps/sms/js/startup.js | 3 +- apps/sms/js/thread_ui.js | 51 +++++--- apps/sms/style/images/pattern.png | Bin 0 -> 6851 bytes apps/sms/style/notification.css | 23 ++++ apps/sms/style/sms.css | 2 +- apps/sms/test/unit/setup.js | 2 +- apps/sms/test/unit/sms_test.js | 1 - apps/sms/test/unit/thread_ui_test.js | 240 +++++++++++++++++++++++----------- 9 files changed, 230 insertions(+), 99 deletions(-) create mode 100644 apps/sms/style/images/pattern.png create mode 100644 apps/sms/style/notification.css (see also PR https://github.com/mozilla-b2g/gaia/pull/9515)

we can skip it by modifying the encoding (eg with the character ě that you can have on some keyboard layouts)

(note that the counters show 2 or 3 segments on the screenshots because I changed the limit while testing)

sorry, the previous patch was breaking the unit tests. So here is the new one with the green unit tests. same PR https://github.com/mozilla-b2g/gaia/pull/9515

Your patch looks great. Let me check in my device. Could you deliver the PR meanwhile? Thanks!

The PR is already there borja ;) see comment 18

borja, any chance to r+ this today ?

(In reply to Julien Wajsberg [:julienw] from comment #13) > Ayman, depending on the characters that are in an SMS, it can use different > encodings. Depending on these encodings, one segment can contain either 160, > 140 or 70 characters. So when you switch from one encoding to another > (because eg you entered a character that does not fit the previous encoding, > I like using "ê" for example, even if doesn't always trigger the > 70-characters encoding), we might double the number of segments with only > one more character. Hey Julien Thanks for bringing this to my attention. Regarding characters having a different encoding I had no idea that this happened. However my current understanding is that if a user make an action that crosses the SMS limit (so lets say adds 131th character when the limit is 130 or adds a country specific character such as ä which changes the number of segments available) we convert the SMS to MMS at which point segments are no longer in play so we would not have this issue. This is what i see happening on the android device I am referring to. However I am going to RFI Daniel to be sure of this before I specify.

Ayman> that will happen on 1.1 but remember this bug is about 1.0.1 too. My current plan was to land this quickly on master/v1-train/v1.0.1 before we change how the mms composition work that will invalidate most of this. Unfortunately I think this won't happen because I'd like to land the mms composition work today and this bug was not reviewed today. So I think I'll have to make a specific 1.0.1 patch.

RFI to Tyler to look at the copy of the banner. Tyler I would like the message delivered to be only a single line. refer to attachment 744749 [details] [diff] [review] for current text being used. thanks

ayman> maybe we can ask Arnau if we can lower the font size too.

tyler, see comment 24. I think you gave this sentence at first.

arnau, please see comment 25, there is a question for you :)

Julien, if we're needing a simple, one-line, translatable message for this, I would just put: "Character limit reached"

Julien, I think is better if you fine tune font-size with Victoria.

I think the new proposition from Tyler is indeed quite "unfriendly" because too short, I do favor a smaller font-size or even a 2-line notification... Victoria, Ayman, I'm waiting for your decision now.

Agreed. If there is an option to adjust font size or distribution to make space for a friendlier message, it would be a big improvement. Also, we can remove "for an SMS." from the message to make it shorter -- it should be more than obvious that we're talking about the length of the current SMS message. So it would just be: "You've reached the maximum length." It doesn't fit on one line in the current font size, but should be easier to adjust for.

Yeah I like that, thanks :) especially that we'll have MMS soon, so the same message works in that case too !

see also PR https://github.com/mozilla-b2g/gaia/pull/9515/files I reduced the font-size and changed the string. I made the font-size smaller by 0.1rem than what was necessary because I thought that in some languages (eg: french, spanish) the text would probably be longer. Victoria, Arnau, maybe we can also reduce the margin, and make the font-size bigger. About the string change, if UX is ok with that, we could keep the old string for 1.0.1, and the text would be displayed on 2 lines. Ayman, would it be good enough for 1.0.1 ? That would keep our l10n-drivers not mad ;-)

reduced the margin, increased the font size

Please tell me what you prefer here and I can update my patch accordingly to your preferences. Also, this won't work correctly on master in a "new thread" mode, until the patch for Bug 861227 lands. Borja, do you think I should do here some of the changes you did in Bug 861227 (mainly: moving the "to" container outside of the header + fiddling with z-indexes) ? It will probably need an ad hoc patch for v1.0.1 too.

Hi Julien, Please go with the last option, the reduced font size and smaller margin and please make sure the margin is 30px so if we change it, it is consistent with other margins through the UI. Thanks!

for font size i defer to Victoria for copy I defer to Julien. From a UX perspective my only request is that the message is one line.

(In reply to ayman maat :maat from comment #38) > for font size i defer to Victoria for copy I defer to Julien. From a UX > perspective my only request is that the message is one line. sorry typo - i mean: for font size i defer to Victoria for copy I defer to *Tyler*. From a UX perspective my only request is that the message is one line.

Ayman, is this request true for both v1.1 and v1.0.1 ? As explained previously, this will need a string change, and I'm reluctant to this in v1.0.1...

We'll reconsider this bug for landing on v1.0.1 if it doesn't hit before 5/10.

(In reply to Victoria Gerchinhoren from comment #37) > Hi Julien, > Please go with the last option, the reduced font size and smaller margin and > please make sure the margin is 30px so if we change it, it is consistent > with other margins through the UI. Victoria, in https://bugzilla.mozilla.org/attachment.cgi?id=745870 I experimented with a margin of 20px which lets me having a bigger font size. Could you please tell me what you prefer between https://bugzilla.mozilla.org/attachment.cgi?id=745870 and https://bugzilla.mozilla.org/attachment.cgi?id=745867.

(In reply to Julien Wajsberg [:julienw] from comment #40) > Ayman, is this request true for both v1.1 and v1.0.1 ? As explained > previously, this will need a string change, and I'm reluctant to this in > v1.0.1... Ok guys. Had a conversation with Julien and am updating this bug with the conclusion for documentation and clarity. We have two situations that we need to cover with the banner: 1) where the user reaches the limit of a message by 'natural' means. i.e. the maximum message length is 300 characters, the user has input 299 characters and up on inputting the 300th character reaches the limits of the message length. This is covered in tyler's #comment 31. This is the one i would like to see as a single line if possible as it will also exist in V1.1 2) where the user can exceed the limit of a message by including a diacritic character: So for example the maximum message length is 300 characters and the user has input 275 characters meaning that they have 25 characters left to input. The user then adds a diacritic character, like ü as the 276th character. The inclusion of this character switches the encoding of the message which reduces its maximum length from 300 characters to 250 characters. This results in the user now being in a state whereby they have exceeded the maximum length of the message. (Julien correct me if i am incorrect in this summary). Therefore we are now in state whereby we have to communicate to the user why they are suddenly, and unexpectedly over their limit and how to recover from this. I am attaching document 'HTML5_SMS-MMSUserStorySpecifications_length exceded_20130506_V0.2' to outline the solution to point 2 above so that UX does not block the progress of dev on this bug. This wireframe will be included in the next version of the MMS/SMS wireframe pack to be released (v9.0)

Sorry Julien, I realise that 30px was the previous one. Thanks for your try outs, very useful, let's go for this one: https://bug865411.bugzilla.mozilla.org/attachment.cgi?id=745870 as it will line up with the left divider in the header, so it's still sticking to a main layout. Thanks!

(In reply to Julien Wajsberg [:julienw] from comment #45) > Created attachment 746121 [details] > screenshot with the second message from maat could we put the 'please' on the next line like this: This character is overweight and requires more space than others. Please remove it to sent his message apart from that it is fine by me...

(In reply to Julien Wajsberg [:julienw] from comment #45) > Created attachment 746121 [details] > screenshot with the second message from maat Julien, the line height seems a bit large to me, can you make sure the line hight is 2.2 for 1.7 rem? And if the text is 1.9 rem the line height should be 2.5 rems Reference: https://docs.google.com/spreadsheet/ccc?key=0Ari8z5WN5YqndFJQOHpIMjZyQzFkejYxcW9jRUtLY2c#gid=0

* implement the notification according to UX and Visual design * display this notification if we reach the max length * disable the send button if over 10 segments, but keep it enabled if we have exactly the max length * rewrote the enableSend function to make it more readable and manageable * use the new markup loader in thread_ui_test.js, and make it global --- apps/sms/index.html | 15 +- apps/sms/js/startup.js | 3 +- apps/sms/js/thread_ui.js | 54 ++++--- apps/sms/locales/sms.en-US.properties | 3 +- apps/sms/style/common.css | 5 +- apps/sms/style/images/pattern.png | Bin 0 -> 6851 bytes apps/sms/style/notification.css | 21 +++ apps/sms/style/sms.css | 15 +- apps/sms/test/unit/setup.js | 2 +- apps/sms/test/unit/sms_test.js | 6 +- apps/sms/test/unit/thread_ui_test.js | 250 +++++++++++++++++++++++---------- 11 files changed, 266 insertions(+), 108 deletions(-) create mode 100644 apps/sms/style/images/pattern.png create mode 100644 apps/sms/style/notification.css see also PR https://github.com/mozilla-b2g/gaia/pull/9515

Two comments: 1. Every phone I've used has the same pattern with special characters reducing the character limit of a message (even old Nokia's), and I've never seen one that explains this to users. It has always seems very intuitive/obvious. Why do we feel that it is necessary to explain this? 2. If we decide it is necessary to explain, the "overweight" text should be changed. I suggest: "You've exceeded the maximum length. Remove special characters or shorten the message to send it."

(In reply to tyler.altes from comment #51) > Two comments: > > 1. Every phone I've used has the same pattern with special characters > reducing the character limit of a message (even old Nokia's), and I've never > seen one that explains this to users. It has always seems very > intuitive/obvious. > > Why do we feel that it is necessary to explain this? Well, to me, it has never seemed obvious at all. But here we're explaining specifically because we disable the send button. > > > 2. If we decide it is necessary to explain, the "overweight" text should be > changed. I suggest: > > "You've exceeded the maximum length. Remove special characters or shorten > the message to send it." ok, thanks a lot (In reply to Victoria Gerchinhoren from comment #47) > (In reply to Julien Wajsberg [:julienw] from comment #45) > > Created attachment 746121 [details] > > screenshot with the second message from maat > > Julien, the line height seems a bit large to me, can you make sure the line > hight is 2.2 for 1.7 rem? And if the text is 1.9 rem the line height should > be 2.5 rems the text is 1.5rem and the line height is 2.2rem. I'll change to 2rem as your reference doesn't mention what it should be at this size, so picked one inbetween. thanks

ok, fixed the last things from Victoria, Tyler and Ayman. Note that if that is not reviewed before 5pm CET today I won't be able to work on this before next monday and therefore it won't likely go to 1.0.1 and therefore it will likely be just lost because the MMS stuff obsoletes all this. I sincerely hope we can at last go forward and land this today. Thanks

Github PR is still in https://github.com/mozilla-b2g/gaia/pull/9515.

Comment on attachment 746320 [details] [diff] [review] patch v5 requesting review from l10n. Sorry about that guys, I tried to not change the string but UX wanted too.

Comment on attachment 746320 [details] [diff] [review] patch v5 Review of attachment 746320 [details] [diff] [review]: ----------------------------------------------------------------- I have two nits that made the patch harder to review, the non-localizable string, and that the keys that do localize it don't show up in the code. With that, f+=me. ::: apps/sms/index.html @@ +117,5 @@ > + role='notification' > + class='hide'> > + <p> > + You've reached the maximum length. > + </p> I'm wondering if you should start with this <p> being empty, or adding a data-l10n-id? ::: apps/sms/js/thread_ui.js @@ +470,5 @@ > > + if (hasMaxLength || exceededMaxLength) { > + this.input.setAttribute('maxlength', value.length); > + var key = hasMaxLength ? 'max' : 'exceeded'; > + var message = navigator.mozL10n.get('messages-' + key + '-length-text'); Can you make the keys show up verbatim in the patch here? It confused the heck out of me, and it seems like it's not adding any real value to the code flow.

fixes to pike's comments. Thanks Pike, carrying the f+ from you. Still the PR in https://github.com/mozilla-b2g/gaia/pull/9515

needinfo maat and vpg to check that the screenshots look good. Anyway if you don't check them before 5pm CET I won't be able to change anything and this won't land. So it'd better be ok.

Looks good for me Julien.

Comment on attachment 746361 [details] [diff] [review] patch v6 The code in the PR is working as expected. There are several issues that will be fixed in https://bugzilla.mozilla.org/show_bug.cgi?id=861227, so we are going to try to land all this today. Thanks Julien!

master: a64ee1c346e71d7ff64904fcfe7dfbd834f1877d thanks

v1-train: 90c3cdc4e0fbab5960ae5ea80ab32ddf3993f42a

V1.0.1: 89a9c5de7313dfd1095d0ff46988bc91a35cbe9c

This is currently not working with all of the new layout patches and #840069. Please see http://github.com/bocoup/gaia/tree/messaging for all of the in-progress code for MMS.

My apologies; this is specific to in-dev work on MMS; please see bug https://bugzilla.mozilla.org/show_bug.cgi?id=870120

Issue is Verified as Fixed. The banners are displayed as they are displayed in attachments in comments 58-61 when the user maxes out segments in SMS composer. Tested the new message option as well as when replying. Unagi, Build ID: 20130520070208 Gecko: http://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/dbecb0c45504 Gaia: ee70d7517689116622c5125ce33e56d46dd3c948 Unagi, Build ID: 20130520070207 Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/321d5b4db55a Gaia: b161f42c5647b37e35ba5a20f394ba40bf674d9c