1349562 - PopupNotification buttons have a slight extra outline

Status: VERIFIED FIXED

(Firefox :: Site Identity, defect, P1)

Description

[Johann Hofmann [:johannh]]

Attachment: Screen Shot 2017-03-22 at 14.39.48.png

See screenshot. This might have regressed with bug 1333469.

Comment 1

[Johann Hofmann [:johannh]]

Attachment: Bug 1349562 - Fix slight outline on popup notification buttons.

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

Comment 2

[Jonathan Kingston [:jkt] he/him]

So we don't lose the understanding here, we think somewhere got lost along the way of implementing this.

There are several related issues here:

1. On the secondary select element with a arrow picker there is a 1px white dot missing.
2. The inconsistent height issue solved in bug 1333469.
3. Top border of the button should be a different colour on hover

Outline was used to solve some of these issues with a border on the containing element however this was likely the cause of bug 1333469. Moving the border to the child element causes issue 1.

Most of the styling can be viewed here: https://people-mozilla.org/~shorlander/styleguide/ui-elements/panel.html

As discussed on irc the closest solution might be just to change the outline to a border however likely will still have the 1. issue.

Comment 3

[Johann Hofmann [:johannh]]

Comment on attachment 8850023 [details]
Bug 1349562 - Fix slight outline on popup notification buttons.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/122772/diff/1-2/

Comment 4

[Jonathan Kingston [:jkt] he/him]

TL;DR: Remove the borders reverting the reviewed patch back to the first push.

1. The hover of secondary buttons looks too pale and doesn't match
2. The white dot issue as mentioned above on buttons with a picker (ie: notification permissions)
3. The unhovered border colour of a primary button is the same as secondary buttons when the mockup shows a darker blue than the button, the hover of the primary button colour looks much closer to the mockup.
   border-top: 1px solid var(--panel-separator-color); is used for both buttons
4. The hover of secondary buttons on the download panel doesn't seem to change at all
   rgba(204, 204, 204, 0.3) seems to be the outline colour that --arrowpanel-dimmed resolves to which potentially has changed since it's first implementation.

As the outline is using transparency on the background colour I don't think it is going to be an accurate reference for the border here as it will sit outside of the .popup-notification-button-container which the transparency was using as it's base mask colour.

The mockup hover border colour change is very subtle difference: #BEBEBE; and hovered: #CACACA; (the mockup uses a separator element and layers a alpha colour on top of it similar to the solution used for the downloads section)

Due all of these I think the best thing to do is remove the border for the time being, we could follow up with a bug for the outstanding issues caused by this:
1. The 1px of white
2. The border colours of the primary button being wrong
3. The hover state border colour
4. That Downloads pane and permissions now use different solutions

Comment 5

[Johann Hofmann [:johannh]]

Sounds good to me :)

Comment 6

[Johann Hofmann [:johannh]]

Comment on attachment 8850023 [details]
Bug 1349562 - Fix slight outline on popup notification buttons.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/122772/diff/2-3/

Comment 7

[Jonathan Kingston [:jkt] he/him]

Comment on attachment 8850023 [details]
Bug 1349562 - Fix slight outline on popup notification buttons.

https://reviewboard.mozilla.org/r/122772/#review125406

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8f23d68f4d10
Fix slight outline on popup notification buttons. r=jkt

Comment 9

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

https://hg.mozilla.org/mozilla-central/rev/8f23d68f4d10

Comment 10

[Johann Hofmann [:johannh]]

Comment on attachment 8850023 [details]
Bug 1349562 - Fix slight outline on popup notification buttons.

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1282768 and bug 1333469
[User impact if declined]: Ugly outline around popup notification buttons 
[Is this code covered by automated tests?]: No, it's just CSS.
[Has the fix been verified in Nightly?]: Not yet
[Needs manual test from QE? If yes, steps to reproduce]: Go to permission.site and trigger a permission prompt. Confirm that on hover it does not match the screenshot that is attached to this bug.
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: Extremely simple CSS change
[String changes made/needed]: None

Comment 11

[Gerry Chang [:gchang]]

Comment on attachment 8850023 [details]
Bug 1349562 - Fix slight outline on popup notification buttons.

Polish an UI issue related to an outline around popup notification buttons. 
Assuming that you are asking the aurora request not release request, I change the request from release to aurora. Let me know if I'm wrong. Aurora54+ & Beta53+.

Comment 12

[Johann Hofmann [:johannh]]

Oh, yes, sorry for that. Hit the wrong select. It's not even affecting release.

Thanks!

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/d2c1ecc0f820

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/7f4bb26d4303

Comment 15

[Andrei Vaida [:avaida]]

Flagging this for manual testing, instructions in Comment 10.

Comment 16

[Paul Silaghi, QA [:pauly]]

Reproduced on nightly 2017-03-22.
Verified fixed 53b7, 54.0a2 (2017-03-28), 55.0a1 (2017-03-28) on Win 10, Ubuntu 14.04, OS X 10.12.