1509629 - Contribute button text color has wrong text color in Add-ons Manager

Status: VERIFIED FIXED

(Toolkit :: Themes, defect, P2)

Description

[Tim Nguyen :ntim]

Attachment: Screen Shot 2018-11-24 at 12.41.27.png

That button should really use the .primary class, instead of duplicating the styling...

Comment 1

[Tim Nguyen :ntim]

Attachment: Bug 1509629 - Clean up add-ons manager 'Contribute' button styling. r=dao

Comment 2

[Tim Nguyen :ntim]

The heart SVG icon is from https://design.firefox.com/icons/viewer/#default%20browser

Comment 3

[Dão Gottwald [:dao]]

Hmm, aren't "primary" / default buttons supposed to be used in dialogs, where Enter would execute the default action? I'm not sure that this styling makes sense here, and why we even have a "primary" class instead of using default="true" (where it makes sense).

Comment 4

[Tim Nguyen :ntim]

(In reply to Dão Gottwald [::dao] from comment #3)
> Hmm, aren't "primary" / default buttons supposed to be used in dialogs,
> where Enter would execute the default action? I'm not sure that this styling
> makes sense here, and why we even have a "primary" class instead of using
> default="true" (where it makes sense).

See https://design.firefox.com/photon/components/buttons.html#primary

A primary button is a button that aims to draw attention. I think this use of the primary class is valid, we want to draw attention for the user to donate to the add-on developer.

Comment 5

[Tim Nguyen :ntim]

It's also used in other places like about:sessionrestore as a call to action, rather than the default action for a dialog.

Comment 6

[Dão Gottwald [:dao]]

(In reply to Tim Nguyen :ntim (please use needinfo?) from comment #4)
> (In reply to Dão Gottwald [::dao] from comment #3)
> > Hmm, aren't "primary" / default buttons supposed to be used in dialogs,
> > where Enter would execute the default action? I'm not sure that this styling
> > makes sense here, and why we even have a "primary" class instead of using
> > default="true" (where it makes sense).
> 
> See https://design.firefox.com/photon/components/buttons.html#primary
> 
> A primary button is a button that aims to draw attention. I think this use
> of the primary class is valid, we want to draw attention for the user to
> donate to the add-on developer.

The highlighting seems similar to what we've always done for default buttons, except that the style guide seems to focus on optics rather than functionality... I don't think it really aims to describe something different.

(In reply to Tim Nguyen :ntim (please use needinfo?) from comment #5)
> It's also used in other places like about:sessionrestore as a call to
> action, rather than the default action for a dialog.

This makes sense there as we focus this button by default.

Comment 7

[Tim Nguyen :ntim]

(In reply to Dão Gottwald [::dao] from Phabricator: https://phabricator.services.mozilla.com/D12818#inline-67703)
> I don't think we should use the primary class here (and we should probably get rid of that class in favor of default="true").

In about:preferences subdialogs, the [default=true] button (the one that says "OK") isn't actually blue, so I'm not sure relying on default="true" is what we want here.

> How about we use standard colors for the button but make the heart red to draw attention to it?

Emanuela, what do you think of Dão's proposal? The button in question is attachment 9027316 [details], it can be found in the add-on manager details view for add-ons that ask for donations like "Black Menu For Google". My current patch keeps the current styling, but fixes the text color to be white.

For completeness, the button on AMO is also blue: https://addons.mozilla.org/en-US/firefox/addon/black-menu-google/

Dão's rationale for not using the primary button styling is that it should be reserved to the default action on a page/dialog, for which we don't seem to be consistent.

Comment 8

[Tim Nguyen :ntim]

Emanuela mentioned that making the button non-primary should be fine, with the icon color matching the text color.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8fed3c0c42be
Clean up add-ons manager 'Contribute' button styling. r=dao

Comment 10

[Noemi Erli[:noemi_erli]]

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

Comment 11

[Gabi Cheta [:Gabi] Desktop QA]

Reproduced the issue on a Nightly from 2018-11-24.
Verified as fixed in 65.0b12 on Windows 10x64, MacOS 10.14 and Ubuntu 16.04.