|Submitter||Diff||Changes||Open Issues||Last Updated|
|Error loading review requests:|
15.58 KB, image/png
59 bytes, text/x-review-board-request
|Details | Review|
Created attachment 8850019 [details] Screen Shot 2017-03-22 at 14.39.48.png See screenshot. This might have regressed with bug 1333469.
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.
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
Sounds good to me :)
Comment on attachment 8850023 [details] Bug 1349562 - Fix slight outline on popup notification buttons. https://reviewboard.mozilla.org/r/122772/#review125406
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/8f23d68f4d10 Fix slight outline on popup notification buttons. r=jkt
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 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+.
Oh, yes, sorry for that. Hit the wrong select. It's not even affecting release. Thanks!
Flagging this for manual testing, instructions in Comment 10.
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.