PopupNotification buttons have a slight extra outline

VERIFIED FIXED in Firefox 53

Status

()

Firefox
Site Identity and Permission Panels
P1
normal
VERIFIED FIXED
5 months ago
5 months ago

People

(Reporter: johannh, Assigned: johannh)

Tracking

Trunk
Firefox 55
Points:
---

Firefox Tracking Flags

(firefox52 unaffected, firefox53 verified, firefox54 verified, firefox55 verified)

Details

(Whiteboard: [fxprivacy])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Assignee)

Description

5 months ago
Created attachment 8850019 [details]
Screen Shot 2017-03-22 at 14.39.48.png

See screenshot. This might have regressed with bug 1333469.
(Assignee)

Updated

5 months ago
Whiteboard: [fxprivacy]
Comment hidden (mozreview-request)
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 hidden (mozreview-request)
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
Flags: needinfo?(jhofmann)
(Assignee)

Comment 5

5 months ago
Sounds good to me :)
Flags: needinfo?(jhofmann)
Comment hidden (mozreview-request)

Comment 7

5 months ago
mozreview-review
Comment on attachment 8850023 [details]
Bug 1349562 - Fix slight outline on popup notification buttons.

https://reviewboard.mozilla.org/r/122772/#review125406
Attachment #8850023 - Flags: review?(jkt) → review+

Comment 8

5 months ago
Pushed by jhofmann@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8f23d68f4d10
Fix slight outline on popup notification buttons. r=jkt

Comment 9

5 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/8f23d68f4d10
Status: ASSIGNED → RESOLVED
Last Resolved: 5 months ago
status-firefox55: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
(Assignee)

Comment 10

5 months ago
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
Attachment #8850023 - Flags: approval-mozilla-release?
Attachment #8850023 - Flags: approval-mozilla-beta?
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+.
Attachment #8850023 - Flags: approval-mozilla-release?
Attachment #8850023 - Flags: approval-mozilla-release-
Attachment #8850023 - Flags: approval-mozilla-beta?
Attachment #8850023 - Flags: approval-mozilla-beta+
Attachment #8850023 - Flags: approval-mozilla-aurora+
(Assignee)

Comment 12

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

Thanks!
status-firefox52: --- → unaffected

Comment 13

5 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/d2c1ecc0f820
status-firefox54: affected → fixed

Comment 14

5 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/7f4bb26d4303
status-firefox53: affected → fixed
Flagging this for manual testing, instructions in Comment 10.
Flags: qe-verify+
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.
Status: RESOLVED → VERIFIED
status-firefox53: fixed → verified
status-firefox54: fixed → verified
status-firefox55: fixed → verified
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.