1213486 - Resize shareplane in top level menu

Status: RESOLVED INCOMPLETE

(Firefox for Android Graveyard :: General, defect)

Description

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

It should be 24dp max height (bug 1140048 comment 25).

Comment 1

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: Screenshot v1 – 4dp all around

Anthony, I was unable to use exact height values because the code is awful so I eyeballed it – what do you think?

Does it look off-center to you too?

Comment 2

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: MozReview Request: Bug 1213486 - Resize shareplane in top level menu. r=liuche

Bug 1213486 - Resize shareplane in top level menu. r=liuche

The menu code is hard to change (I filed bug xxx) so I opted to
save time by adjusting the padding in the image with an inset
drawable. It'd be better to adjust it statically in the menu code.

Comment 3

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Comment on attachment 8675909 [details]
Screenshot v1 – 4dp all around

Looks a bit small to me. 

Try again? Not sure how to be more specific... :\

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

Is playing around with padding going to mean trouble for varying device DPIs?

Comment 5

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to Mark Finkle (:mfinkle) from comment #4)
> Is playing around with padding going to mean trouble for varying device DPIs?

No, I'm using an inset drawable with dip values.

Comment 6

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: Screenshot v2 – 2dp all around

Comment 7

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Comment on attachment 8676202 [details]
Screenshot v2 – 2dp all around

That looks better. Still a _tiny_ bit too tall it seems. Very close.

But to be sure (since we can't confirm the actual dimensions) can I see a screen of it just a bit smaller?

Sorry bout the repetition but it's hard to know for sure without numbers :)

Thanks!

Comment 8

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: Screenshot v3 – 3dp all around

Comment 9

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Comment on attachment 8676791 [details]
Screenshot v3 – 3dp all around

Lets go back to 2dp all around.

Comment 10

[Chenxia Liu [:liuche]]

Comment on attachment 8675913 [details]
MozReview Request: Bug 1213486 - Resize shareplane in top level menu. r=liuche

https://reviewboard.mozilla.org/r/22501/#review20793

I think that is fine, to create an inset drawable and add padding.

Is the reason you didn't update the shareplane in menu/browser_app_menu.xml because this change shouldn't be present on 2.3 devices?

Comment 11

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

https://reviewboard.mozilla.org/r/22501/#review20793

> Is the reason you didn't update the shareplane in menu/browser_app_menu.xml because this change shouldn't be present on 2.3 devices?

Yes, there is no icon for the entries in the "More" menu on 2.3 devices.

Comment 12

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

https://hg.mozilla.org/integration/fx-team/rev/66d1252f9864a025273f4b1fdc41022f8137eea7
Bug 1213486 - Resize shareplane in top level menu. r=liuche

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/66d1252f9864

Comment 14

[Teodora Vermesan (:TeoVermesan)]

Attachment: Screenshot_2015-10-28-15-26-18.png

Verified as fixed using:
Build: Firefox 44.0a1 (2015-10-28)
Device: Samsung S5 (Android 4.4.2)

Comment 15

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Backing out to target 45 – see bug 1140048 comment 74.

Comment 16

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Backout of 44, as per comment 15: https://hg.mozilla.org/releases/mozilla-aurora/rev/6e4d156192a1

Comment 17

[Teodora Vermesan (:TeoVermesan)]

Verified as fixed
Device: Nexus 9 (Android 6.0) and Xiaomi Mi i4 (Android 5.0.2)

Confirm that adding the "send to device" icon to top level menu was backed out from 44.0a2 Aurora (2015-11-11) and that shareplane icon was rezised on 45.0a1 Nightly (2015-11-11)

Comment 18

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

https://hg.mozilla.org/integration/fx-team/rev/e3462f6085ecead4d6664189bbcbb20a7055665e
Bug 1213486 - Backout 66d1252f9864 to backout top level shareplane (bug 1140048).

Comment 19

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

re backout: see bug 1140048 comment 83. Since we backed out the bug 1140048 that added this bug, this does not affect any versions until bug 1140048 lands again.

Comment 20

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Since bug 1140048 is backed out, not actively working on this.

Comment 21

[BMO Automation]

We have completed our launch of our new Firefox on Android. The development of the new versions use GitHub for issue tracking. If the bug report still reproduces in a current version of [Firefox on Android nightly](https://play.google.com/store/apps/details?id=org.mozilla.fenix) an issue can be reported at the [Fenix GitHub project](https://github.com/mozilla-mobile/fenix/). If you want to discuss your report please use [Mozilla's chat](https://wiki.mozilla.org/Matrix#Connect_to_Matrix) server https://chat.mozilla.org and join the [#fenix](https://chat.mozilla.org/#/room/#fenix:mozilla.org) channel.