1389921 - New Tab button should move 2px to the left

Status: VERIFIED FIXED

(Firefox :: Theme, defect, P1)

Description

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

STR:
1. Hover over "Open a new tab" button in tab bar
and see that it need some tuning to be like on Photon specification - http://design.firefox.com/people/shorlander/photon/Mockups/windows-7.html


Some data

"Open a new tab" button size:
36 x 26 px - before landing patch from bug #1377184
28 x 28 px - after landing patch from bug #1377184
28 x 28 px - in Photon specification

"Open a new tab" button active hover area:
36 x 26 px - before landing patch from bug #1377184
36 x 26 px - after landing patch from bug #1377184
28 x 28 px - in Photon specification

Space between Tab and "Open a new tab" button:
4 px - after landing patch from bug #1377184
2 px - in Photon specification


So in tl;dr "Open a new tab" button:
- should be moved 2 px to the left
- active hover area should be only on the button area


Regression range pushlog:
https://hg.mozilla.org/integration/autoland/json-pushes?changeset=500a01cf896dbb9883cb85f162552e20b8facfc0&full=1

Caused by:
Bug 1377184 - Use new Photon hover effect also on "Show more bookmarks" and extensions buttons icons in Bookmarks Toolbar

Comment 1

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

Attachment: good.png

Comment 2

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

Attachment: good (500%).png

Comment 3

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

Attachment: bad.png

Comment 4

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

Attachment: bad (500%).png

Comment 5

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

Attachment: specification.png

Comment 6

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

Attachment: specification (500%).png

Comment 7

[Eddward]

One more thing. Looks like the "Open a new tab" button has slightly different parameters for every UI density option. For example there is only 1 px gap between Tab and Open a new tab button in Compact mode.

Comment 8

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

I forgot to mention, that my screenshots are with Compact density mode.

Comment 9

[Eddward]

As far as I can see all your screenshots are from Normal density mode. Check the density options on the Mockup page. There is a difference in Back button design.

Comment 10

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

Ops, yes, you're right.

Comment 11

[Dão Gottwald [::dao]]

(In reply to Virtual_ManPL [:Virtual] - (please needinfo? me - so I will see your comment/reply/question/etc.) from comment #0)
> So in tl;dr "Open a new tab" button:
> - should be moved 2 px to the left
> - active hover area should be only on the button area

We don't have plans to do the latter. Morphing bug.

Comment 13

[Dão Gottwald [::dao]]

Attachment: Bug 1389921 - Stop setting a width on the new tab button to remove excess padding around it.

As of bug 1377184, this button uses the correct inner padding depending on the UI density setting, so we don't need to explicitly set a width anymore.

Review commit: https://reviewboard.mozilla.org/r/170042/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/170042/

Comment 14

[Dale Harvey (:daleharvey)]

Comment on attachment 8898661 [details]
Bug 1389921 - Stop setting a width on the new tab button to remove excess padding around it.

https://reviewboard.mozilla.org/r/170042/#review175932

Looks good cheers

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cc018eee995b
Stop setting a width on the new tab button to remove excess padding around it. r=daleharvey

Comment 16

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/cc018eee995b

Comment 17

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Just for the record, this caused some conflicts when I merged bug 1352119 to m-c from inbound. I think I resolved it correctly, though.

Comment 18

[Dão Gottwald [::dao]]

(In reply to Wes Kocher (:KWierso) from comment #17)
> Just for the record, this caused some conflicts when I merged bug 1352119 to
> m-c from inbound. I think I resolved it correctly, though.

Looks like it.

Comment 19

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

I'm confirming that bug is fixed, starting in Mozilla Firefox Nightly 57.0a1 (2017-08-22), so I'm marking this bug as VERIFIED. Thanks.

Comment 20

[Johann Hofmann [:johannh]]

Screenshots:

https://screenshots.mattn.ca/compare/?oldProject=mozilla-central&oldRev=a9d372645a32b8d23d44244f351639af9d73b96a&newProject=mozilla-central&newRev=c7570eb46382ee56d081e549b484341f400c864b