884975 - [MMS] Apply 1.5x Res Version of the Images

Status: VERIFIED FIXED

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

Description

[Peter La]

Attachment: @2x Res assets for MMS

This is an addition to the bug 878042 to add the @2x resolution version of the assets.

Comment 1

[Fabien Cazenave [:kaze]]

Follow-up of a leo+ bug.

Comment 2

[Lukas Blakk [:lsblakk] use ?needinfo]

Is this for WVGA?  If so, we want this to be hd+ not leo+

Comment 3

[Peter La]

I am not sure - asking Vicky, who requested this.

Comment 4

[Victoria Gerchinhoren [:vicky]]

No, it is for the bigger screen devices, all assets in the UI are produced in 1x and 2x.

Comment 5

[Alex Keybl [:akeybl]]

We think this is only necessary for the hd branch.

Comment 6

[Alex Keybl [:akeybl]]

sorry, only necessary for leo+ 1.1 based upon comment 4 (NOT hd)

Comment 7

[Alex Keybl [:akeybl]]

kaze any updates here?

Comment 8

[Fabien Cazenave [:kaze]]

Attachment: link to pull request

https://github.com/mozilla-b2g/gaia/pull/10710

I don’t have any HIDPI device to test, feel free to send me a Peak. :-p

Comment 9

[Fabien Cazenave [:kaze]]

Comment on attachment 769518 [details]
link to pull request

Alexandre, would you please test this patch on your Peak and confirm it works in HIDPI mode?

Comment 10

[Julien Wajsberg [:julienw]]

Comment on attachment 769518 [details]
link to pull request

I don't like this approach; using rem in background-position will break as soon as the default font-size change.

Comment 11

[Fabien Cazenave [:kaze]]

As per discussion on the pull request, un-assigning and handing over to the Taipei team.

Fred, I saw you worked on a hi-dpi patch yesterday, can you please look at this bug or assign it to someone else in your team?

Comment 12

[[e-mail]]

Sure, I'll talk with steve(he is working on 1.5x image at bug 881155) and help do the following fix.

Comment 13

[[e-mail]]

Comment on attachment 769518 [details]
link to pull request

talked with steve and we'd prefer land this and fixed some background-size issue at bug 881155

And we are curious about why 2x image issue is a leo+ bug?

Comment 14

[Julien Wajsberg [:julienw]]

I actually think it's a hd+ bug, you're right !

Comment 15

[Wayne Chang [:wchang]]

hd+ing as required.

Comment 16

[Steve Chung [:steveck]]

I have a comment on https://github.com/mozilla-b2g/gaia/pull/10710#issuecomment-20407695.
Hi Julien, Using the rem or px both works for me and I think it should not break the css sprite at least in v1.1hd. We can continue the hd work after landing this patch. What do you think?

Comment 17

[Steve Chung [:steveck]]

There is no need to apply 2X image got v1.1 leo branch, even v1.1 hd branch will only required for 1.5X image. I will change the title to 1.5X image.

In this bug, I will add all the 1.5X image assets with some css fixing and it will uplift to v1.1 HD
In bug 881155, it will be hd-only patch for the remaining missing css changes in hd branch.

Hi Peter/Victoria, Could you please also provide the 1.5X images for v1.1hd branch? We already convert existing 2X images to 1.5X, but some images is still missing in the master. except for the placeholder image, we still need:

images/paperclip-button.png
images/add_contact.png
images/icons/deliveredtick.png

Comment 18

[Amy Lee [:amylee] UX]

Hi Steve, 

I've emailed you the 3 outstanding icons @1.5x. Let me know if you need anything else.

Comment 19

[Steve Chung [:steveck]]

Attachment: Link to github

Hi Julien, I add 1.5X image assets with some css fix. 2X images is not necessary for master and hd now, thanks.

Comment 20

[Julien Wajsberg [:julienw]]

Comment on attachment 771960 [details]
Link to github

r=me

thanks !

Comment 21

[Steve Chung [:steveck]]

Land in master : 566dc43df6ebb19669471557093e12c26ccb932c
        v1.1hd : 5a83b21cb25ab012b5f468e483d323cb56e04761

Comment 22

[Wayne Chang [:wchang]]

Changing to leo+ in order to dupe bug 889805, the same patch should be applied.

jhford: can you uplift this to v1-train also? thanks.

Comment 24

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

v1-train: 2d7498b0a696fa902239fc803542dab9b8ef5d4a

Comment 25

[isabelrios]

Icon for video attachments has the correct size.
Verified with unagi v1-train 07/16 build:
Gecko-ea7cfd4
Gaia-c8f432c

Comment 26

[Rudy Lu [:rudyl] (inactive after 2015/7/3)]

Since 1.5x assets are included in this patch, I don't think it is a good idea to uplift this patch to v1-train.

For now, the build script in v1-train could not handle the 1.5x assets with GAIA_DEV_PIXELS_PER_PX (only in master and hd branch).
So it would cause these 1.5x assets wrapped up in the app package, while they are unnecessary for v1-train.

Steve,

Do you think the above concern is reasonable?
Thanks.

Comment 27

[Steve Chung [:steveck]]

(In reply to Rudy Lu [:rudyl] from comment #26)
> Since 1.5x assets are included in this patch, I don't think it is a good
> idea to uplift this patch to v1-train.
> 
> For now, the build script in v1-train could not handle the 1.5x assets with
> GAIA_DEV_PIXELS_PER_PX (only in master and hd branch).
> So it would cause these 1.5x assets wrapped up in the app package, while
> they are unnecessary for v1-train.
> 
> Steve,
> 
> Do you think the above concern is reasonable?
> Thanks.

Just check the build script and GAIA_DEV_PIXELS_PER_PX will not be considered in v1-train just like you said. Apologize for my negligence and I will fire another follow up patch only for removing the image assets. Thanks for the notice!

Comment 28

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

v1.1.0hd: 2d7498b0a696fa902239fc803542dab9b8ef5d4a