1443631 - Devices in Send Tab needs latest icons

Status: RESOLVED FIXED

(Firefox for iOS :: Sync, defect, P2)

Description

[Ryan Feeley [:rfeeley]]

Attachment: IMG_5560.PNG

I noticed that the Devices list in Send Tab is showing the previous generation of icons. They should be using the Photon device icons like inside the app.

https://design.firefox.com/icons/viewer/#device

Comment 1

[Ryan Feeley [:rfeeley]]

Attachment: device-tablet.svg.zip

If the tablet is not yet on the icon site, here it is.

Comment 2

[:garvan]

Send To in new share sheet has new icons, but Send To through the page menu has old ones. I am in this code ATM, will fix

Comment 3

[:garvan]

Can I get these as 24, 48, and 72 px high pngs? All iOS icons are PNGs at 1x, 2x, and 3x sizes. 
The width must be the same, whatever common px width fits all the images.

Comment 4

[Ryan Feeley [:rfeeley]]

Attachment: tablets.zip

I think these are the sizes you are looking for. 72 is too big.

Comment 5

[:garvan]

For reference these are the icons:
https://github.com/mozilla-mobile/firefox-ios/tree/master/Client/Assets/Images.xcassets/deviceTypeMobile.imageset
https://github.com/mozilla-mobile/firefox-ios/tree/master/Client/Assets/Images.xcassets/deviceTypeDesktop.imageset

Comment 6

[:garvan]

The current code expects 24, 48, and 72 px high PNGs. All the device images need to be correctly proportional, so if one image has 5 px of transparent border on top and bottom, another image can't be cropped content appears larger. Viewing the images side-by-side at the same zoom level is a way to check this.

Comment 7

[:garvan]

Robin, can you locate/generate some icons for this? Thanks.

Comment 8

[Farhan Patel [:farhan]]

Attachment: Pull Request

Comment 9

[:garvan]

It is unclear if this bug is asking only to update the 2 device icons we use, or to ensure we show the mobile device icon type, phone vs tablet, which requires adding a 3rd icon type.

Comment 10

[Farhan Patel [:farhan]]

It looks like we just need to update the icons. We updated these for Photon in the main app bundle but never did for the share extension.

Comment 11

[:garvan]

Taking the bug back :), I hadn't read your comment on the PR until now, I'll build with the icons you provided and see if the UIImageViews need adjustig

Comment 12

[:garvan]

I am adding a build step to copy shared icons between projects.

The alternative is to have a SharedImages.xcassets (included in Client and ShareTo build targets), however this will break our current asset organization which is spread across Client Images.xcassets, Home xcassets, and Menu xcassets. I think moving individual images from each one of these into SharedImages.xcassest will be confusing to those looking for images in those asset groups.

Comment 13

[:garvan]

Attachment: Pull request

Comment 14

[:garvan]

We should show both types of mobile device icons, bug added here: https://bugzilla.mozilla.org/show_bug.cgi?id=1462113

Comment 15

[:garvan]

Attachment: Pull request

This is a simpler PR that shares only the Send To Device assets, and this time use Xcode's build-in way of sharing assets.