874496 - [MMS] [UX] Attach image, change size indicator over the image to this one

Status: RESOLVED FIXED

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

Description

[Victoria Gerchinhoren [:vicky]]

Attachment: specs for the attachment size indicator over image

Right now when attaching an image, the size is indicated in an overlay that has a very different style from the system's shapes. Use this attachment for reference to make the styling correct.

Comment 1

[greg]

Attachment: Pull request

Comment 2

[Corey Frang [:gnarf]]

Attachment: Screenshot with patch

Comment 3

[Corey Frang [:gnarf]]

Comment on attachment 754952 [details] [diff] [review]
Pull request

Looks good to me, but borja requested I double check css based fixes with julien or himself.

Comment 4

[Victoria Gerchinhoren [:vicky]]

Hi Corey, 
The shape and text are ok visually, but I see "kB" has a lower case and upper case letter, can you fix that so it's always capitals?

Thanks!

Comment 5

[Julien Wajsberg [:julienw]]

Comment on attachment 754952 [details] [diff] [review]
Pull request

looks good r=me

Comment 6

[Julien Wajsberg [:julienw]]

(In reply to Victoria Gerchinhoren from comment #4)
> Hi Corey, 
> The shape and text are ok visually, but I see "kB" has a lower case and
> upper case letter, can you fix that so it's always capitals?

This is in the locale.

Greg, can you change it here ?

Pike, can we change the value for the "attachmentSize" key, without changing the key, on 1.1 ? I think this key appeared on 1.1 last night only, so it probably haven't hit any localizer yet.

Comment 7

[Axel Hecht [:Pike]]

I just pushed all of 1.1 to localizers earlier today, so this is out in the wild.

FTR, SI and binary units say kB, the only thing with a capital K is a KiB, i.e. a kibibyte, http://en.wikipedia.org/wiki/Kilobyte

Comment 8

[Julien Wajsberg [:julienw]]

Thanks for the information Pike !

Victoria, after Pike's comment, do you still want a all-capital "KB" ?

Comment 9

[Francesco Lodolo [:flod]]

(In reply to Axel Hecht [:Pike] from comment #7)
> FTR, SI and binary units say kB, the only thing with a capital K is a KiB,
> i.e. a kibibyte, http://en.wikipedia.org/wiki/Kilobyte

About kB vs KB, see also bug 814936

Comment 10

[Victoria Gerchinhoren [:vicky]]

Hi Axel, 

I understand that it is a convention, but for me it looks like an error and I don't think the regular user will know this is a convention, it just looks weird. But, if you have your specific reasons, then you rule!

Comment 11

[Julien Wajsberg [:julienw]]

Corey, Grey, since you're native english speakers, I think you can have the final word here.

Note that if you change the locale string, you'll need to change the key as well.

Comment 12

[greg]

kB is the accurate capitalization per http://en.wikipedia.org/wiki/Kilobyte

Comment 13

[Corey Frang [:gnarf]]

This is a design change that should land leo+ - marking leo?

Comment 14

[Corey Frang [:gnarf]]

master: https://github.com/mozilla-b2g/gaia/commit/fe41240fc905f7fc1026e87f0e36ec7c7aba5fb3

Comment 15

[Corey Frang [:gnarf]]

Comment on attachment 754952 [details] [diff] [review]
Pull request

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 869255
User impact if declined: Style
Testing completed: Yes
Risk to taking this patch (and alternatives if risky): very low, css only change
String or UUID changes made by this patch: none

This impacts visual design only, we should uplift.

Comment 16

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 754952 [details] [diff] [review]
Pull request

leo+, no need for uplift approval

Comment 17

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

I was not able to uplift this bug to v1-train.  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-train, 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-train
  git cherry-pick -x  fe41240fc905f7fc1026e87f0e36ec7c7aba5fb3
  <RESOLVE MERGE CONFLICTS>
  git commit

Comment 18

[Corey Frang [:gnarf]]

https://github.com/gnarf37/gaia/commit/c5c933268532c6d82c6dfd2c0616ab1d3f95bbc1

Comment 19

[Corey Frang [:gnarf]]

v1-train: 367db8847ab18497248b5fef53c0234edfa66fdd