[message] Subject. The banner informing the user that the maximum length of subject has been reached obscures the subject text field

RESOLVED FIXED in Firefox OS v1.3

Status

Firefox OS
Gaia::SMS
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: maat, Assigned: borjasalguero)

Tracking

unspecified
1.3 C2/1.4 S2(17jan)
ARM
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:1.3+, b2g-v1.3 fixed, b2g-v1.3T fixed)

Details

Attachments

(2 attachments)

(Reporter)

Description

4 years ago
Created attachment 8350065 [details]
2013-12-19-09-46-02.png

When there is multiple lines of text in the message body the permanent banner informing the user that the maximum length of subject has been reached obscures the subject text field. This means that the user cannot see and therefore refer to what they have typed. refer to screenshot

Ooops that will be a design mistake by me.

I would suggest as a quick win that we make the message temporary instead of permanent and only generate it when the user tries to add a character that takes the subject over its limit.
(Reporter)

Updated

4 years ago
blocking-b2g: --- → 1.3?
(note that this is another proof that we should not have merged the subject feature that late in the 1.3 cycle)

I don't know how easy it is, I'd need Fernando to comment, but he's away now. So we'll investigate in January.
(Reporter)

Comment 2

4 years ago
ni? to Fernando referencing comment 1
Flags: needinfo?(fernando.campo)
Triage agreed this seems bad enough to block.
blocking-b2g: 1.3? → 1.3+
Assignee: nobody → borja.bugzilla
Target Milestone: --- → 1.3 C2/1.4 S2(17jan)
(In reply to ayman maat :maat from comment #0)
> 
> I would suggest as a quick win that we make the message temporary instead of
> permanent and only generate it when the user tries to add a character that
> takes the subject over its limit.
Indeed this would be the best option, as leaving less space to the message wouldn't look nice (and means more reflows).

I think it does already shows only when the user tries to pass the limit, need to check
Flags: needinfo?(fernando.campo)
(Assignee)

Comment 5

4 years ago
Im gonna deliver the patch following Ayman & Fernando's suggestions. With the new patch, the banner is only shown when we 'type', not when we 'focus' in the subject field.
On the other hand, we will show the banner for 2/3 seconds.
(Assignee)

Updated

4 years ago
OS: Mac OS X → Gonk (Firefox OS)
Hardware: x86 → ARM
(Assignee)

Comment 6

4 years ago
Created attachment 8359282 [details] [review]
Pull request
Attachment #8359282 - Flags: review?(fernando.campo)
(Assignee)

Updated

4 years ago
Attachment #8359282 - Flags: review?(schung)
Comment on attachment 8359282 [details] [review]
Pull request

Hi Borja, I've left some comments on github. Feel free to ping me once you reply/fix the suggestions, thanks.
Attachment #8359282 - Flags: review?(schung)
Comment on attachment 8359282 [details] [review]
Pull request

De-assigning myself as rwaldron and steve are already taking a look, should be enought with them :p
Attachment #8359282 - Flags: review?(fernando.campo)
(Assignee)

Comment 9

4 years ago
Comment on attachment 8359282 [details] [review]
Pull request

Steve, changes addressed! r?
Attachment #8359282 - Flags: review?(schung)
Comment on attachment 8359282 [details] [review]
Pull request

Only some small suggestions and we're almost ready :)
Attachment #8359282 - Flags: review?(schung)
(Assignee)

Comment 11

4 years ago
Comment on attachment 8359282 [details] [review]
Pull request

Suggestion addressed! r?
Attachment #8359282 - Flags: review?(schung)
(In reply to Borja Salguero [:borjasalguero] from comment #11)
> Comment on attachment 8359282 [details] [review]
> Pull request
> 
> Suggestion addressed! r?

I still feel having another naming would be better because we have banner for 4 different messages with timeout. how about subjectLengthNotice?
(Assignee)

Comment 13

4 years ago
Changing & uploading with subjectLengthNotice!
(Assignee)

Comment 14

4 years ago
Changed! r?
Comment on attachment 8359282 [details] [review]
Pull request

I think the patch is ready but it seems we got some conflict here... :( I'll r+ once you fix the conflict!
Attachment #8359282 - Flags: review?(schung)
(Assignee)

Comment 16

4 years ago
Rebased! (No conflicts actually) :) r?
(Assignee)

Updated

4 years ago
Attachment #8359282 - Flags: review?(schung)
Comment on attachment 8359282 [details] [review]
Pull request

r=me, thanks for the fixing!
Attachment #8359282 - Flags: review?(schung) → review+
(Assignee)

Comment 18

4 years ago
https://github.com/mozilla-b2g/gaia/commit/d8f015f8ce8c70b59f939c6b03304dc39c28c809
https://github.com/borjasalguero/gaia/commit/8d20961eaa4c63a7daea9069dad69015e2905266
R+. Merged.
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
status-b2g-v1.3: --- → affected
Uplifted d8f015f8ce8c70b59f939c6b03304dc39c28c809 to:
v1.3: a3e62d005b072c4050a2ec0e0331fad647c07833
status-b2g-v1.3: affected → fixed
Blocks: 919966
Depends on: 968714
FYI I filed bug 968714 because I think the behavior implemented here is not completely right.
Depends on: 995173
status-b2g-v1.3T: --- → fixed
You need to log in before you can comment on or make changes to this bug.