963050 - [SMS] Subject's line-height in composer is too high

Status: VERIFIED FIXED

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

Description

[José Vittone]

Attachment: 2014-01-23-13-09-59.png

Steps to reproduce:

1. Open Messages app
2. Tap "create new" button
3. Tap on "more options" menu and "Add subject"
4. Type something on the subject input that is more than 1 line, around 30 characters

Comment 1

[Julien Wajsberg [:julienw]]

I think you see this when you type "enter" too.

I'd like a 1.3+ bug for this, because subject is due for 1.3 and this makes a pretty bad experience. This should be quite esay to fix.

Fernando will you have the time to take a look?

Comment 2

[Fernando Campo (:fcampo)]

Attachment: Link to PR - https://github.com/mozilla-b2g/gaia/pull/15736

Comment 3

[Julien Wajsberg [:julienw]]

Comment on attachment 8366095 [details] [review]
Link to PR - https://github.com/mozilla-b2g/gaia/pull/15736

I'm generally fine with these changes. I left some questions and nits on github though so please request review once you fixed and answered them.

Thanks, we should definitely have this in 1.3!

Comment 4

[David Scravaglieri [:scravag]]

Please ask for approval once it is ready or renominate for 1.4?

Comment 5

[Julien Wajsberg [:julienw]]

Renominating.

Sorry, please try this on a real device. If we don't have this bug in, the subject feature just looks broken.

Comment 6

[Preeti Raghunath(:Preeti)]

1.3+ for bad experience and inability to read.

Comment 7

[Wilfred Mathanaraj [:WDM]]

PM reviewed : stays blocker

Comment 8

[Kai-Chih Hu [:khu]]

Can we get ETA to fix this bug? Thank you.

Comment 9

[Julien Wajsberg [:julienw]]

I think we just need a new review round, should be ready in 1 day-work.

Fernando, are you around or would you like that somebody picks this up?

Comment 10

[Julien Wajsberg [:julienw]]

Gonna put tomorrow as a tentative landing date. If Fernando can't finish this today I think somebody should steal this tomorrow.

Comment 11

[Fernando Campo (:fcampo)]

Comment on attachment 8366095 [details] [review]
Link to PR - https://github.com/mozilla-b2g/gaia/pull/15736

Updated the PR, got rid of the scrollbar (when less than 3 lines), and put some order on the CSS changes.

Comment 12

[Julien Wajsberg [:julienw]]

Comment on attachment 8366095 [details] [review]
Link to PR - https://github.com/mozilla-b2g/gaia/pull/15736

I'm gonna r+ this because this is fine enough for 1.3, but I think the height in a single line mode is not correct, or maybe it should be somewhat more centered.

I think we can revisit this safely and in a quieter time in Bug 949457, so let's land this now!

Comment 13

[Julien Wajsberg [:julienw]]

master: d876a70c6d96e8c67a26897d8d4d508407b7581d

Comment 14

[John Ford [:jhford] CET/CEST Berlin Time]

I was not able to uplift this bug to v1.3.  If this bug has dependencies which are not marked in this bug, please comment on this bug.  If this bug depends on patches that aren't approved for v1.3, we need to re-evaluate the approval.  Otherwise, if this is just a merge conflict, you might be able to resolve it with:

  git checkout v1.3
  git cherry-pick -x -m1 d876a70c6d96e8c67a26897d8d4d508407b7581d
  <RESOLVE MERGE CONFLICTS>
  git commit

Comment 15

[Steve Chung [:steveck]]

Some css class naming chages doesn't uplift to v1.3. Conflict fixed and uplifted to v1.3:
bf79fe1fbc59838c13899dd39748875ee4086ec3

Comment 16

[Loli (:lolimartinezcr)]

Tested (02/10/2014) and working
1.3
Gecko b71e4b8
Gaia bf79fe1