Closed Bug 1374573 Opened 3 years ago Closed 3 years ago

update overlay tourset icons and illustrations


(Firefox :: New Tab Page, enhancement, P1)




Firefox 56
Tracking Status
firefox56 --- fixed


(Reporter: gasolin, Assigned: rexboy)



(Whiteboard: [photon-onboarding])


(2 files)

in bug 1354707 UX have provided the final icons and illustration so we should update them before shipping v56.
Priority: -- → P2
Whiteboard: [photon-onboarding]
Target Milestone: --- → Firefox 56
Assignee: nobody → rexboy
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?
Flags: needinfo?(mverdi)
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.
Flags: needinfo?(mverdi)
Flags: qe-verify+
Priority: P2 → P1
QA Contact: jwilliams
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.
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.
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?
Flags: needinfo?(mverdi)
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.
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.
Flags: needinfo?(mverdi)
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!
Comment on attachment 8882043 [details]
Bug 1374573 - Update overlay tourset icons and illustrations.

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 /**/
Thanks for finding that out. I was working on too many things at a time.. Let me correct that right away.
Attachment #8882043 - Flags: review?(dtownsend)
Comment on attachment 8882043 [details]
Bug 1374573 - Update overlay tourset icons and illustrations.

I have rebased them into the sync tour's patch so that should be okay.
Blocks: 1375793
Comment on attachment 8882043 [details]
Bug 1374573 - Update overlay tourset icons and illustrations.
Attachment #8882043 - Flags: review?(dtownsend) → review+
Thank you Mossop!

Looks good on Try server so let's check it in:
Keywords: checkin-needed
Pushed by
Update overlay tourset icons and illustrations.r=mossop
Keywords: checkin-needed
Closed: 3 years ago
Resolution: --- → FIXED
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
QA Whiteboard: [bugday-20170705]
I can confirm this issue has been fixed on today's nightly.
You need to log in before you can comment on or make changes to this bug.