1034633 - [Messages] subtle padding issues while manipulating the subject

Status: VERIFIED FIXED

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

Description

[Julien Wajsberg [:julienw]]

See in attachments the subject + input locations.

Comment 1

[Julien Wajsberg [:julienw]]

Attachment: without a subject line

Comment 2

[Julien Wajsberg [:julienw]]

Attachment: with an empty subject line

Comment 3

[Julien Wajsberg [:julienw]]

Attachment: with a non-empty subject line

Comment 4

[Julien Wajsberg [:julienw]]

This is especially noticeable when typing the first letter of a subject, because we very clearly see that it's moving.

NI Vicky to give input about what is the correct padding.

Comment 5

[Victoria Gerchinhoren [:vicky]]

THis is the correct hight: https://bug1034633.bugzilla.mozilla.org/attachment.cgi?id=8451010

Comment 6

[Oleg Zasypkin [:azasypkin]]

Attachment: GitHub pull request URL

Hey Julien,

I've reduced the height of the send button a bit (so now it's the same as min-height of the message input) to get rid of the issue described in the bug. But I'm in doubt whether there is enough space now, it looks OK imho.

+ I'm experimenting with 'background-blend-mode' instead of separate image for the disabled state.

So I need your feedback on both changes :)

Comment 7

[Julien Wajsberg [:julienw]]

Comment on attachment 8452300 [details] [review]
GitHub pull request URL

You should ask Victoria about this, in both DSDS and non-DSDS screen captures. If she's fine, I am too.

But I suspect she won't :)

For example I don't understand why there is a difference between a empty and non empty subject. This surely can be fixed differently...

Comment 8

[bhavana bajaj [:bajaj]]

This does not look like a release blocking issue, its really hard to spot. Although, if this is a simple css change we can consider uplift before July 21.

Comment 9

[Oleg Zasypkin [:azasypkin]]

Attachment: Single SIM (SMS)

Hey Vicky,

Could you please let me know if you're OK (or not) with the top and bottom paddings for the send button. I'm asking becuase I've reduced send button height from 4.5rem to 4rem (initial height of the message input field) to simplify the fix for the issue in this bug.

If you're not OK with it, I'll try to find other solution.

Thanks!

Comment 10

[Oleg Zasypkin [:azasypkin]]

Attachment: Single SIM (MMS)

Comment 11

[Oleg Zasypkin [:azasypkin]]

Attachment: Dual SIM (SMS)

Comment 12

[Oleg Zasypkin [:azasypkin]]

Attachment: Dual SIM (MMS)

Comment 13

[Victoria Gerchinhoren [:vicky]]

(In reply to Oleg Zasypkin [:azasypkin] from comment #9)
> Created attachment 8452532 [details]
> Single SIM (SMS)
> 
> Hey Vicky,
> 
> Could you please let me know if you're OK (or not) with the top and bottom
> paddings for the send button. I'm asking becuase I've reduced send button
> height from 4.5rem to 4rem (initial height of the message input field) to
> simplify the fix for the issue in this bug.
> 
> If you're not OK with it, I'll try to find other solution.
> 
> Thanks!

I think it's fine

Comment 14

[Oleg Zasypkin [:azasypkin]]

Comment on attachment 8452300 [details] [review]
GitHub pull request URL

(In reply to Victoria Gerchinhoren [:vicky] from comment #13)
> (In reply to Oleg Zasypkin [:azasypkin] from comment #9)
> > Created attachment 8452532 [details]
> > Single SIM (SMS)
> > 
> > Hey Vicky,
> > 
> > Could you please let me know if you're OK (or not) with the top and bottom
> > paddings for the send button. I'm asking becuase I've reduced send button
> > height from 4.5rem to 4rem (initial height of the message input field) to
> > simplify the fix for the issue in this bug.
> > 
> > If you're not OK with it, I'll try to find other solution.
> > 
> > Thanks!
> 
> I think it's fine

Thanks Vicky!

Julien, could you please review the patch then?

Thanks

Comment 15

[Julien Wajsberg [:julienw]]

(In reply to bhavana bajaj [:bajaj] [NOT reading Bugmail, needInfo please] from comment #8)
> This does not look like a release blocking issue, its really hard to spot.

Bhavana, this is really not hard to spot when you use the device (it is accordingly hard to spot with screenshots). I can't provide videos right now as I'm not in the office, but the padding change is very noticeable when you add text. Try these STR:

1. open the SMS app
2. tap the "new message" button
3. tap the "settings" button, then "add subject"
4. tap the "subject" input, wait that the keyboard appears
5. input one letter
=> see how it changed
6. remove that letter
=> it changed back

I agree this is polish, but here I consider this is a regression, and that it brings a really bad feeling.

Comment 16

[Julien Wajsberg [:julienw]]

Vicky, I'd like to be sure that you'be seen that the input's min height is changed with this patch too. Is it ok for you?

Comment 17

[Julien Wajsberg [:julienw]]

Comment on attachment 8452300 [details] [review]
GitHub pull request URL

r=me, but please wait for Vicky needinfo before merging. If she's not ok with the min-height change we'll need something else.

Comment 18

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

triage: 2.0+ for the regression.

Comment 19

[Julien Wajsberg [:julienw]]

Looks like you missed the "2.0+" item ;)

Comment 20

[Victoria Gerchinhoren [:vicky]]

looks ok.

Comment 21

[Oleg Zasypkin [:azasypkin]]

Master: https://github.com/mozilla-b2g/gaia/commit/3924f4d1d38c1359e8071316bec45eae24affee2

Comment 22

[Ryan VanderMeulen [:RyanVM]]

v2.0: https://github.com/mozilla-b2g/gaia/commit/4cf4fd8698578c12a7cf9a01c63a75eef8710f7e

Comment 23

[Shine(Leave from Mozilla)]

Attachment: screenshot

According to the STR in comment#15, the issue has been verified successfully on Flame 2.0 and 2.1.
See attachment: Verify_Flame2.1_1.png & Verify_Flame2.1_2.png
Reproducing rate: 0/5

Flame 2.0 build:
Gaia-Rev        856863962362030174bae4e03d59c3ebbc182473
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/e40fe21e37f1
Build-ID        20141208000206
Version         32.0

Flame 2.1 build:
Gaia-Rev        38e17b0219cbc50a4ad6f51101898f89e513a552
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/8b92c4b8f59a
Build-ID        20141205001201
Version         34.0

Comment 24

[Shine(Leave from Mozilla)]

Attachment: screenshot