1462166 - Style Bookmark doorhanger buttons like panel footer buttons

Status: RESOLVED FIXED

(Firefox :: Theme, enhancement, P2)

Comment 1

[Dão Gottwald [:dao]]

Attachment: Bug 1462166 - Style Bookmark doorhanger buttons like panel footer buttons.

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

Comment 2

[Dão Gottwald [:dao]]

I copied the blue values from here so we're internally consistent, and because the mockup didn't have the hover and active values:

https://searchfox.org/mozilla-central/rev/5a744713370ec47969595e369fd5125f123e6d24/toolkit/themes/shared/popupnotification.inc.css#85-96

Comment 3

[Tim Nguyen :ntim]

Attachment: macOS Screenshot

The patch doesn't look quite right on macOS.

Probably because of these bits: https://searchfox.org/mozilla-central/source/browser/themes/osx/browser.css#414-435

Comment 4

[Dão Gottwald [:dao]]

(In reply to Tim Nguyen :ntim from comment #3)
> Created attachment 8981756 [details]
> macOS Screenshot
> 
> The patch doesn't look quite right on macOS.
> 
> Probably because of these bits:
> https://searchfox.org/mozilla-central/source/browser/themes/osx/browser.
> css#414-435

Ah yes, I forgot about that stuff. Thanks.

Comment 5

[Dão Gottwald [:dao]]

Comment on attachment 8981399 [details]
Bug 1462166 - Style Bookmark doorhanger buttons like panel footer buttons.

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

Comment 6

[Marco Bonardo [:mak]]

Comment on attachment 8981399 [details]
Bug 1462166 - Style Bookmark doorhanger buttons like panel footer buttons.

https://reviewboard.mozilla.org/r/247520/#review254450

::: browser/themes/shared/places/editBookmarkPanel.inc.css:28
(Diff revision 2)
> +
> +.editBookmarkPanelBottomButton {
> +  flex: 1;
> +  -moz-appearance: none;
> +  margin: 0;
> +  padding: 1em;

Hm, the buttons in the mock-up are less tall, indeed they appear to be more or less as tall as the header.

Applying the patch, on Windows 10, they appear too tall and unbalanced. Could you please reduce the vertical padding a little bit, so the buttons are shorter and about the same height as the header?

::: browser/themes/shared/places/editBookmarkPanel.inc.css:48
(Diff revision 2)
> +  border-inline-end: 1px solid var(--panel-separator-color);
> +}
> +
> +.editBookmarkPanelBottomButton[default] {
> +  color: white;
> +  background-color: #0996f8;

nit: If these colors start being used across all the notification panels, it may be worth to evaluate moving them to vars

Comment 7

[Dão Gottwald [:dao]]

Comment on attachment 8981399 [details]
Bug 1462166 - Style Bookmark doorhanger buttons like panel footer buttons.

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

Comment 8

[Dão Gottwald [:dao]]

Comment on attachment 8981399 [details]
Bug 1462166 - Style Bookmark doorhanger buttons like panel footer buttons.

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

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ea2e4c24e373
Style Bookmark doorhanger buttons like panel footer buttons. r=mak

Comment 10

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/ea2e4c24e373