1392467 - [Onboarding] change speech bubble and overlay CTA style

Status: VERIFIED FIXED

(Firefox :: New Tab Page, defect, P2)

Description

[[e-mail]]

based on https://docs.google.com/document/d/1XVowXtnAzzzyLcwdFiP6cx3l9_CNBadP9_7v17XJ40s/edit

Need change
* speech bubble style
* overlay CTA button style

Comment 1

[[e-mail]]

and the speech bubble should above the activity stream's search bar

Comment 2

[[e-mail]]

Attachment: Bug 1392467 - [Onboarding] change speech bubble and overlay CTA style;

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

Comment 3

[[e-mail]]

Attachment: UI preview

Aaron here's the UI preview, (the speech bubble font-size is equal to semibold). You can see the CTA button hover & active state looks not good. I'd plan to pick the origin hover text color(white) if you do not offer another hover/active style.

Comment 4

[Aaron Benson]

I'm inclined to leave the font-weight alone for the speech bubble but would like to get some feedback from the design systems folks when they return. It certainly looks a little to strong from this preview but not terrible. 

For the CTA button, use the same button styles that we use in Preferences for those secondary/gray buttons. The hover state should not be blue.

Comment 5

[Verdi [:verdi] Best to slack me]

Attachment: Screen Shot 2017-08-24 at 4.48.41 PM.png

(In reply to Fred Lin [:gasolin] from comment #3)
> Created attachment 8900595 [details]
> UI preview

Hey Fred, the buttons in the overlay should still be blue buttons with white text. 
color: #0060df
hover: #003eaa
border-radius: 2px
semibold system font, 14px

Comment 6

[[e-mail]]

Comment on attachment 8900593 [details]
Bug 1392467 - [Onboarding] change speech bubble and overlay CTA style;

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

Comment 7

[[e-mail]]

Here's the update screencast for reference http://g.recordit.co/dTsKWzp8oA.gif

Comment 8

[[e-mail]]

The current speech bubble take normal font-weight 
http://g.recordit.co/dTsKWzp8oA.gif

You can compare the result with previous screencast and decide which font-weight is proper for the sppech bubble
http://g.recordit.co/3EQ3hRx43V.gif


As discussion result, we still need to change overlay CTA style to deep blue and apply round corner style.
Aaron, could you help update the spec to reflect the overlay CTA button style change?

Comment 9

[KM Lee [:rexboy] (inactivate after Sep. 22)]

Comment on attachment 8900593 [details]
Bug 1392467 - [Onboarding] change speech bubble and overlay CTA style;

https://reviewboard.mozilla.org/r/172006/#review178510

r=me with issues addressed. Please fix them or clearify with UX before proceeding.

::: browser/extensions/onboarding/content/onboarding.css:344
(Diff revision 2)
>    -moz-outline-radius: 2px;
>  }
>  
>  .onboarding-tour-action-button:hover:not([disabled]) ,
>  #onboarding-notification-action-btn:hover {
>    background: #0060df;

The rule `.onboarding-tour-action-button:hover:not([disabled])` is no longer perceivable to user because we got the unhovered background already `#0060df` . 
Seems we lack the background color for both hover and active (Got active only). Maybe ask UI for the color, or just separate the rule with just `cursor: pointer`.

::: browser/extensions/onboarding/content/onboarding.css:345
(Diff revision 2)
>  }
>  
>  .onboarding-tour-action-button:hover:not([disabled]) ,
>  #onboarding-notification-action-btn:hover {
>    background: #0060df;
> +  color: #fff;

Is this necessary?

::: browser/extensions/onboarding/content/onboarding.css:352
(Diff revision 2)
>  }
>  
>  .onboarding-tour-action-button:active:not([disabled]),
>  #onboarding-notification-action-btn:active  {
>    background: #003EAA;
> +  color: #fff;

Is this necessary?

Comment 10

[KM Lee [:rexboy] (inactivate after Sep. 22)]

(In reply to Verdi [:verdi] from comment #5)
> Created attachment 8900922 [details]
> Screen Shot 2017-08-24 at 4.48.41 PM.png
> 
> (In reply to Fred Lin [:gasolin] from comment #3)
> > Created attachment 8900595 [details]
> > UI preview
> 
> Hey Fred, the buttons in the overlay should still be blue buttons with white
> text. 
> color: #0060df
> hover: #003eaa
> border-radius: 2px
> semibold system font, 14px

Actually we need three states of color: normal, hover, active. Fred's current implementation has #0060df for normal and hover, and #003eaa for active. Do you want to provide different another color so these 3 states are satisfied?

Comment 11

[KM Lee [:rexboy] (inactivate after Sep. 22)]

Comment on attachment 8900593 [details]
Bug 1392467 - [Onboarding] change speech bubble and overlay CTA style;

https://reviewboard.mozilla.org/r/172006/#review178524

Sorry, clear review flag before getting answer for the background color.

Comment 12

[Verdi [:verdi] Best to slack me]

(In reply to KM Lee [:rexboy] from comment #10)
> 
> Actually we need three states of color: normal, hover, active. Fred's
> current implementation has #0060df for normal and hover, and #003eaa for
> active. Do you want to provide different another color so these 3 states are
> satisfied?

Normal should be: #0060df 
Hover should be: #003eaa (not #0060df)
Active should be: #002275

Comment 13

[[e-mail]]

Comment on attachment 8900593 [details]
Bug 1392467 - [Onboarding] change speech bubble and overlay CTA style;

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

Comment 14

[[e-mail]]

issue addressed and updated with new color, please kindly review it again

Comment 15

[KM Lee [:rexboy] (inactivate after Sep. 22)]

Comment on attachment 8900593 [details]
Bug 1392467 - [Onboarding] change speech bubble and overlay CTA style;

https://reviewboard.mozilla.org/r/172006/#review179404

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/28cfcdb5694b
[Onboarding] change speech bubble and overlay CTA style;r=rexboy

Comment 17

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

https://hg.mozilla.org/mozilla-central/rev/28cfcdb5694b

Comment 19

[Justin [:JW_SoftvisionQA]]

I have verified that this is fixed on today's nightly.

Comment 20

[Cristian Comorasu, QA [:ccomorasu], Releae Desktop QA - Please contact [e-mail]]

I can confirm the intended behavior is respected on beta. I verified using Fx 57.0b7 on Windows 10 x64, Ubuntu 14.04 LTS and macOS X 10.12.6.