1651472 - Polish contacts sidebar AB selector focus ring (double/duplicate border), rectify AB context menu button layout

Status: RESOLVED FIXED

(Thunderbird :: Theme, defect)

Description

[Thomas D. (:thomas8)]

Attachment: Screenshot1: AB selector with double focus border, AB context menu button with unwanted border and wrong dimensions

+++ This bug was initially created as a clone of Bug #1643159 +++

Seen on Win10 / Daily 80.0a1 (2020-07-07) (64-bit)
Haven't checked if Beta is affected.

Contacts side bar seems to be a moving target. Some odd layout details need polishing again (see attached screenshot).

• AB selector focus ring is bigger than the text box, resulting in double border when focused. Please shrink the focus ring to match the non-focused size of the dropdown selector. Grow-on-focus is quite odd when clearly not intentional.
• AB context menu button (≡) has an unwanted border when not focused, and the entire button seems to have changed in dimensions and now looks disproportionate (too slim, too high).

Comment 1

[Thomas D. (:thomas8)]

Also (probably as a result of the increase in context menu button height), 'Address Book' selector label is now too far away from its dropdown box.

Comment 2

[Richard Marti (:Paenglab)]

The abContextMenuButton issues are fixed in bug 1650791.

Comment 3

[Wayne Mery (:wsmwk)]

I'm assuming 78 is affected. If that is not the case, please remove tb78=affected

Comment 4

[Thomas D. (:thomas8)]

(In reply to Wayne Mery (:wsmwk) from comment #3)

I'm assuming 78 is affected. If that is not the case, please remove tb78=affected

I have just checked 78.0b4 (32-bit) and it is minimally affected:

• Upon focusing, the height of the AB selector increases, i.e. the focus ring makes the input grow bigger (which is odd).
• Double border not seen, but one px white inside offset which looks wrong.
• The AB context menu button looks regular.

Comment 5

[Richard Marti (:Paenglab)]

Attachment: 1651472-AbPickerHeader-spacing.patch

This should fix both issues: the spacing and on Windows the focus on menulists. I changed from outline to border. Somehow the outline wasn't painted over the border on menulists under Windows.

Comment 6

[Alessandro Castellani [:aleca]]

Comment on attachment 9163823 [details] [diff] [review]
1651472-AbPickerHeader-spacing.patch

redirecting this review to Richard as I'm slightly overwhelmed at the moment, sorry 😅️

Comment 7

[Richard Marti (:Paenglab)]

Comment on attachment 9163823 [details] [diff] [review]
1651472-AbPickerHeader-spacing.patch

I can't review my own patch.

Asking Thomas for feedback on Windows.

Comment 8

[Alessandro Castellani [:aleca]]

Oh my God, so sorry, I don't know how but I saw the patch coming from Thomas.
I'll see if I can review it later.

Comment 9

[Alessandro Castellani [:aleca]]

Comment on attachment 9163823 [details] [diff] [review]
1651472-AbPickerHeader-spacing.patch

Review of attachment 9163823 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good.
One addition I'd like to make is to add the highlight colour for the outline on `#abContextMenuButton:-moz-focusring`, and the related macos variation.
Right now the dashed outline is super black when the focus moves on top of that button, which is inconsistent with the outline colour we're using for the close buttons of the addressing rows.

Comment 10

[Richard Marti (:Paenglab)]

Attachment: 1651472-AbPickerHeader-spacing.patch

Removing the outline on normal buttons also removed the correct outline colour for the abContextMenuButton.

Thomas, this will land soon on c-c and you can check it there.

Comment 11

[Richard Marti (:Paenglab)]

Comment on attachment 9164017 [details] [diff] [review]
1651472-AbPickerHeader-spacing.patch

[Approval Request Comment]
Regression caused by (bug #): One of my themeable dialog bugs
User impact if declined: too narrow button under Windows
Testing completed (on c-c, etc.): soon on c-c
Risk to taking this patch (and alternatives if risky): low

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/5862e0f05332
Fix the spacing in AbPickerHeader. r=aleca

Comment 13

[Wayne Mery (:wsmwk)]

Comment on attachment 9164017 [details] [diff] [review]
1651472-AbPickerHeader-spacing.patch

Approved for beta

Comment 14

[Rob Lemley [:rjl]]

Thunderbird 79.0b2:
https://hg.mozilla.org/releases/comm-beta/rev/4a492a7883e9

Comment 15

[Wayne Mery (:wsmwk)]

Comment on attachment 9164017 [details] [diff] [review]
1651472-AbPickerHeader-spacing.patch

Approved for esr78

Comment 16

[Rob Lemley [:rjl]]

Thunderbird 78.0.1:
https://hg.mozilla.org/releases/comm-esr78/rev/c143113ac3f0