1387618 - Increase "Close tab" icon size with Touch density

Status: VERIFIED FIXED

(Firefox :: Theme, defect, P1)

Description

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

"Close tab" button icon size should respect "Density" preference in "Customize..." option, because for now it doesn't change like all other icons and it looks kinda huge in "Compact" density mode.



"Speedy" Regression window (mozilla-central)
Good:
https://ftp.mozilla.org/pub/firefox/nightly/2017/08/2017-08-03-10-03-52-mozilla-central/

Bad:
https://ftp.mozilla.org/pub/firefox/nightly/2017/08/2017-08-04-10-03-54-mozilla-central/

Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=63e261ce8cb04c913d2e6b19ea451b7078d24dc1&tochange=32083f24a1bb2c33050b4c972783f066432194eb

Probably caused by:
Bug 1385702 - Update close icon style for photon

Comment 1

[Tim Nguyen :ntim]

The spec says the size should only be adjusted for Touch mode. Compact mode has the same size as Normal mode.

http://design.firefox.com/people/shorlander/photon/Mockups/windows-10.html

Comment 2

[Tim Nguyen :ntim]

Perhaps fixing bug 1387609 will make the icon feel lighter and smaller.

Comment 3

[Tim Nguyen :ntim]

...which will help for Compact mode.

Comment 4

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

(In reply to Tim Nguyen :ntim from comment #1)
> The spec says the size should only be adjusted for Touch mode. Compact mode
> has the same size as Normal mode.
> 
> http://design.firefox.com/people/shorlander/photon/Mockups/windows-10.html

Maybe specification forgot to include also the Compact mode, so "Close tab" button icon size should be smaller, like in Touch mode betting bigger.

Comment 5

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

Also the size of the button should change.

Comment 6

[Tim Nguyen :ntim]

Stephen, should the close button be smaller in Compact mode ?

Comment 7

[Johann Hofmann [:johannh]]

Why should it be smaller? That won't change the tab height and thus doesn't help the UI get more compact. There's no user benefit.

Comment 8

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

If the close button will be smaller, it will be consistent with other buttons also getting smaller in Compact mode or getting bigger in Touch mode, so in end Compact mode user could see more characters in tab titles (or at least I think it will be, as close button size get bigger, after landing patch from bug #1385702).

Comment 9

[Dão Gottwald [:dao]]

Attachment: Bug 1387618 - Increase tab close button size in touch mode.

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

Comment 10

[Dão Gottwald [:dao]]

I didn't extend the close button's hover background. That background is part of the icon file so fixing this would be more work.

We're not going to make the button smaller in compact mode.

Comment 11

[Dale Harvey (:daleharvey)]

Comment on attachment 8897482 [details]
Bug 1387618 - Increase tab close button size in touch mode.

https://reviewboard.mozilla.org/r/168774/#review174504

Apologies for the delay, wanted to make sure to test in touch mode and my windows machine had build issues, got it working and this is looking good cheers

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/760113529a6c
Increase tab close button size in touch mode. r=daleharvey

Comment 13

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

https://hg.mozilla.org/mozilla-central/rev/760113529a6c

Comment 14

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

Is this patch only intended for making larger hover active close tab button area in only Touch Density mode,
not making also visible close button size larger in Touch Density mode
and not touching Compact Density mode?
I'm asking to doube-check.

Comment 15

[Dão Gottwald [:dao]]

(In reply to Virtual_ManPL [:Virtual] - (please needinfo? me - so I will see your comment/reply/question/etc.) from comment #14)
> Is this patch only intended for making larger hover active close tab button
> area in only Touch Density mode,
> not making also visible close button size larger in Touch Density mode
> and not touching Compact Density mode?
> I'm asking to doube-check.

Yes, see comment 10.

Comment 16

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

I saw it, but I wanted to make sure that I understand it correctly by doube-checking.
I'm confirming that bug is fixed, starting in Mozilla Firefox Nightly 57.0a1 (2017-08-17), so I'm marking this bug as VERIFIED. Thanks.