Closed Bug 1392467 Opened 3 years ago Closed 3 years ago

[Onboarding] change speech bubble and overlay CTA style

Categories

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

57 Branch
defect

Tracking

()

VERIFIED FIXED
Firefox 57
Tracking Status
firefox57 --- verified

People

(Reporter: gasolin, Assigned: gasolin)

References

Details

(Whiteboard: [photon-onboarding][photon-onboarding-newui])

Attachments

(3 files)

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

Need change
* speech bubble style
* overlay CTA button style
Priority: P1 → --
Whiteboard: [photon-onboarding] → [photon-onboarding][triage]
and the speech bubble should above the activity stream's search bar
Blocks: 1390222
Blocks: 1392468
Flags: qe-verify+
Priority: -- → P2
QA Contact: jwilliams
Whiteboard: [photon-onboarding][triage] → [photon-onboarding]
Target Milestone: --- → Firefox 57
Attached file 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.
Flags: needinfo?(abenson)
Assignee: nobody → gasolin
Status: NEW → ASSIGNED
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.
Flags: needinfo?(abenson) → needinfo?(emanuela)
(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
Here's the update screencast for reference http://g.recordit.co/dTsKWzp8oA.gif
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?
Flags: needinfo?(abenson)
Blocks: 1392472
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?
Attachment #8900593 - Flags: review?(rexboy) → review+
(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?
Flags: needinfo?(mverdi)
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.
Attachment #8900593 - Flags: review+
Flags: needinfo?(abenson)
(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
Flags: needinfo?(mverdi)
issue addressed and updated with new color, please kindly review it again
Comment on attachment 8900593 [details]
Bug 1392467 - [Onboarding] change speech bubble and overlay CTA style;

https://reviewboard.mozilla.org/r/172006/#review179404
Attachment #8900593 - Flags: review?(rexboy) → review+
Pushed by flin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/28cfcdb5694b
[Onboarding] change speech bubble and overlay CTA style;r=rexboy
Whiteboard: [photon-onboarding] → [photon-onboarding][photon-onboarding-newui]
Duplicate of this bug: 1390222
I have verified that this is fixed on today's nightly.
Status: RESOLVED → VERIFIED
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.
Flags: qe-verify+
Flags: needinfo?(emanuela)
You need to log in before you can comment on or make changes to this bug.