1170292 - Toolbars need properly scaled assets

Status: RESOLVED FIXED

(Firefox for iOS :: General, defect)

Description

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

The two toolbars, bottom and top in case of landscape mode, currently use button assets that are too large. Instead of shrinking these in code I would like to include properly sized images as assets that we can include as-is without having to scale or do tricks with contentEdgeInsets.

I *think* the version of the button images in the top bar are slightly larger than in the bottom bar. (For landscape)

If this is the case, we have two options:

1) Provide two icon sets that have the proper sizes for both bars.

2) Provide just the larger set and provide scale-down factor for when we use these images in the bottom toolbar.

Either is fine from a technical point of view. I don't know if scaling has a visual result that is noticable.

Comment 1

[Robin Andersen [:tecgirl]]

https://github.com/mozilla/firefox-ios/pull/538



Currently, the assets display at 40x40 when in portrait, 50x50 in landscape. However, the assets are 60x60.

I removed the insets (just set them to 0) in BrowserToolbar.swift/NavButtonInset and the nav icons display in portrait at 60x60. Share/Bookmark remained at 40x40 until rotation, when the entire set displays at 50x50. On rotation back to portrait the nav icons display at 40x40, send and bookmark display at 60x60.

So, by removing the inset padding and reworking and visually balancing all of the set to 50x50, it looks great!

I did notice though that when on a panel and rotating to landscape, the reload, send, and bookmark icons appear active (though they are not on tap). Then, after rotating back to portrait, all icons appear active (when they should not).

I also noticed that if you add a bookmark in landscape then rotate it to portrait the active bookmark icon does not persist (though the bookmark is still saved).

Comment 2

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Attachment: PR: https://github.com/mozilla/firefox-ios/pull/538

Comment 3

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Robin, your pull request contained some other unrelated changes so I cherry-picked your new assets commit into the PR for bug 1164646.

I will file separate bugs, if we dont have them yet, for the issues you observed.

Comment 4

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Resolving because I merged the assets from this bug into the commit of https://bugzilla.mozilla.org/show_bug.cgi?id=1164646

Comment 5

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Comment on attachment 8614022 [details] [review]
PR: https://github.com/mozilla/firefox-ios/pull/538

Closing this since this has been merged via another pr.