gasolin@mozilla.com Reporter
	Description • 7 years ago

in bug 1354707 UX have provided the final icons and illustration so we should update them before shipping v56.

	KM Lee [:rexboy] (inactivate after Sep. 22) Assignee
	Comment 1 • 7 years ago

The final assets I received is:

1) deep gray icon for tour items (on the left) for usual state.
2) blue icon for tour items (on the left) for mouse-hover state.
3) blue-and-gray illustrations on the right side of tour description.
And we'll do color change for 2) by css.

Verdi are the descriptions above correct? Do we still have any undergoing work on revising images for version 56?

	Verdi [:verdi] Best to slack me
	Comment 2 • 7 years ago

Hi Rex,
The blue version is also for the selected state. We're no longer doing some kind of white box below. Otherwise, yes, this sounds correct and there is no work happening to revise these.

	KM Lee [:rexboy] (inactivate after Sep. 22) Assignee
	Comment 3 • 7 years ago

Per Verdi's clarification, we'll need to remove the rightmost padding for the illustration such that the illustration sticks to the rightmost border of the overlay. I'll include this change in the patch.

	KM Lee [:rexboy] (inactivate after Sep. 22) Assignee
	Comment 4 • 7 years ago

And selected tour item should have blue text, without background color and border, according to Verdi's update. These polish will be included in the patch too.

	KM Lee [:rexboy] (inactivate after Sep. 22) Assignee
	Comment 5 • 7 years ago

Hi Verdi:
I found the new illustration under RTL mode may not able to stick its cut boundary to the overlay border, which I'm not sure if it's what you want. Can you confirm if this is expected?

	KM Lee [:rexboy] (inactivate after Sep. 22) Assignee
	Comment 6 • 7 years ago

From engineer's aspect mirroring the illustration by CSS is possible but that's not likely the regular thing we do. We can put another version of illustration only for RTL if necessary too.

	KM Lee [:rexboy] (inactivate after Sep. 22) Assignee
	Comment 7 • 7 years ago

Quick update from Verdi:
Mirroring the illustration is okay for RTL case. There may be RTL-specific illustrations later if the visual want to do those minor refinement, but mirroring current set works for him.

	KM Lee [:rexboy] (inactivate after Sep. 22) Assignee
	Comment 9 • 7 years ago

This patch is basically just the image assets and minor css changes mentioned above.

Hello Mossop, are you available for reviewing the patch? 
Or maybe transferring the review to Fischer or Fred if you're not available. Thanks a lot!

	gasolin@mozilla.com Reporter
	Comment 10 • 7 years ago
mozreview-review

Comment on attachment 8882043 [details]
Bug 1374573 - Update overlay tourset icons and illustrations.

https://reviewboard.mozilla.org/r/153124/#review158442

since sync tour is landed, we should put the sync icon/illustration update in as well

::: browser/extensions/onboarding/content/img/icons_customize-colored.svg:9
(Diff revision 1)
> +    <title>Glyph / Customize</title>
> +    <desc>Created with Sketch.</desc>
> +    <defs></defs>
> +    <g id="Symbols" stroke="none" stroke-width="1" fill="none" fill-rule="evenodd">
> +        <g id="Glyph-/-Customize" fill-rule="nonzero" fill="#0A84FF">
> +            <path d="M4,10 C3.11377721,10.0019983 2.33522075,10.5886657 2.089,11.44 C2.089,11.45 2.075,11.455 2.072,11.465 C1.71,12.6 1.367,13.575 0.313,14.038 L0.29,14.05 L0.266,14.062 C0.103569051,14.1480122 0.00142339961,14.3162069 0,14.5 C3.38176876e-17,14.7761424 0.223857625,15 0.5,15 C2.23928916,15.1206928 3.96070475,14.5855377 5.325,13.5 C5.331,13.494 5.332,13.487 5.338,13.481 C5.95714834,12.9293483 6.17001231,12.0519562 5.87249095,11.2779114 C5.57496958,10.5038667 4.82923974,9.9949314 4,10 L4,10 Z M15.693,0.307 C15.3277668,-0.0561298005 14.7443099,-0.0761888569 14.355,0.261 L6.324,7.261 C6.11941742,7.43978453 5.99804884,7.69533882 5.98876394,7.96687469 C5.97947905,8.23841056 6.08310771,8.50165841 6.275,8.694 L7.307,9.725 C7.49098604,9.90850285 7.74014608,10.0116903 8,10.012 L8.033,10.012 C8.30454362,10.0028095 8.56014395,9.88152608 8.739,9.677 L15.739,1.646 C16.0778022,1.25688682 16.0577071,0.671944821 15.693,0.307 L15.693,0.307 Z" id="Shape"></path>

the new svg seems not optimized and contain the editor info(Generator: Sketch 44.1), could we ask the designer to get the optimized version?

::: browser/extensions/onboarding/content/onboarding.css:186
(Diff revision 1)
>  .onboarding-tour-content > img {
>    width: 352px;
> -  margin: 0 calc(50% - 176px);
> +  margin: 0;
> +}
> +
> +/*  */

add some comment or get rid of the empty /**/

	KM Lee [:rexboy] (inactivate after Sep. 22) Assignee
	Comment 11 • 7 years ago

Thanks for finding that out. I was working on too many things at a time.. Let me correct that right away.

	KM Lee [:rexboy] (inactivate after Sep. 22) Assignee
	Comment 13 • 7 years ago
mozreview-review-reply

Comment on attachment 8882043 [details]
Bug 1374573 - Update overlay tourset icons and illustrations.

https://reviewboard.mozilla.org/r/153124/#review158442

I have rebased them into the sync tour's patch so that should be okay.

	Dave Townsend [:mossop]
	Comment 14 • 7 years ago
mozreview-review

Comment on attachment 8882043 [details]
Bug 1374573 - Update overlay tourset icons and illustrations.

https://reviewboard.mozilla.org/r/153124/#review158806

	KM Lee [:rexboy] (inactivate after Sep. 22) Assignee
	Comment 15 • 7 years ago

Thank you Mossop!

Looks good on Try server so let's check it in:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=748d4503f98e

	Pulsebot
	Comment 16 • 7 years ago

Pushed by rchien@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9831635161a8
Update overlay tourset icons and illustrations.r=mossop

	Phil Ringnalda (:philor)
	Comment 17 • 7 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/9831635161a8

	Saddam Hossain
	Comment 18 • 7 years ago

I have reproduced this Bug on Nightly 56.0a1 (2017-06-20) on Windows 10, 64 bit!

The bug's fix is now verified on latest  Nightly 56.0a1

Build ID    :	20170706060058
User Agent  : 	Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:56.0) Gecko/20100101 Firefox/56.0

	Justin [:JW_SoftvisionQA]
	Comment 19 • 7 years ago

I can confirm this issue has been fixed on today's nightly.