We'll need to do a visual polish for the revised UX spec. Hopefully this is going to be started when the visuals and assets (and maybe texts) are all locked down. specs see bug 1354707.

Please ignore `Polyfill - Add private browsing and search tour to onboarding overlay.`, which will be replaced after `Bug 1357046 - Should add the Private Browsing tour in the onBoarding overlay` is landed.

Comment on attachment 8876609 [details] Bug 1370459 - Polish the onboarding overlay UI to align with new visual spec; https://reviewboard.mozilla.org/r/147936/#review152424 ::: browser/extensions/onboarding/content/onboarding.css:65 (Diff revision 1) > background: #f5f5f7; > border: 1px solid rgba(9, 6, 13, 0.1); /* #09060D, 0.1 opacity */ > + border-radius: 3px; > position: relative; > - margin: 0 calc(50% - 600px); > - top: calc(50% - 275px); > + margin: 0 calc(50% - 480px); > + top: calc(50% - 255px); Please use ``` top: 50%; left: 50%; transform: translate(-50%, -50%); ``` This way can deouple from the width and height. ::: browser/extensions/onboarding/content/onboarding.css:146 (Diff revision 1) > } > > .onboarding-tour-description { > grid-row: tour-page-start / tour-page-end; > grid-column: tour-page-start / tour-content-start; > - padding: 40px; > + /*padding: 40px;*/ Why a comment out rule here?

Comment on attachment 8876609 [details] Bug 1370459 - Polish the onboarding overlay UI to align with new visual spec; https://reviewboard.mozilla.org/r/147936/#review153240 Per discussion I think we are still waiting for the latest visual assets to be confirmed.

Comment on attachment 8876609 [details] Bug 1370459 - Polish the onboarding overlay UI to align with new visual spec; https://reviewboard.mozilla.org/r/147936/#review153342 ::: browser/extensions/onboarding/content/onboarding.css:66 (Diff revision 2) > border: 1px solid rgba(9, 6, 13, 0.1); /* #09060D, 0.1 opacity */ > + border-radius: 3px; > position: relative; > - margin: 0 calc(50% - 600px); > - top: calc(50% - 275px); > + top: 50%; > + offset-inline-start: 50%; > + transform: translate(-50%, -50%); What's the purpose for changing from margin to transform? It puts the overlay wrong when viewport height is less than 510px. If for some reason we have to use transform please consider the rule below inside media query together. And I believe we can place the overlay right without using transform. ::: browser/extensions/onboarding/content/onboarding.css:114 (Diff revision 2) > > #onboarding-tour-list > li { > list-style: none; > height: 48px; > border-inline-start: 6px solid transparent; > + border-radius: 2px; I saw the border has been removed in the spec. ::: browser/extensions/onboarding/content/onboarding.css:115 (Diff revision 2) > #onboarding-tour-list > li { > list-style: none; > height: 48px; > border-inline-start: 6px solid transparent; > + border-radius: 2px; > padding-inline-start: 66px; and padding-inline-start being 49px ::: browser/extensions/onboarding/content/onboarding.css:116 (Diff revision 2) > list-style: none; > height: 48px; > border-inline-start: 6px solid transparent; > + border-radius: 2px; > padding-inline-start: 66px; > + margin-bottom: 9px; and margin-inline-start being 10px. ::: browser/extensions/onboarding/content/onboarding.css:155 (Diff revision 2) > + padding-inline-end: 28px; > } > > .onboarding-tour-description > h1 { > font-size: 36px; > - margin: 40px 0 20px 0; > + margin-top: 40px; Let's use a smaller value to make it vertically centered like 25px ::: browser/extensions/onboarding/content/onboarding.css:181 (Diff revision 2) > height: 100%; > border: none; > } > > .onboarding-tour-button > button { > + position: absolute; Do you still prefer using position: absolute button than just stretch page grid down to footer-end?

will rebase based on bug 1357049's button style

mossop this is an overlay CSS only change so you can delegate the review to rexboy and fischer if you like.

Here's how it looks like after the patch http://g.recordit.co/bsnLkp4mbu.gif

(In reply to Fred Lin [:gasolin] from comment #12) > Here's how it looks like after the patch > > http://g.recordit.co/bsnLkp4mbu.gif This is inaccessible for me

Comment on attachment 8876609 [details] Bug 1370459 - Polish the onboarding overlay UI to align with new visual spec; https://reviewboard.mozilla.org/r/147936/#review154174

Comment on attachment 8876609 [details] Bug 1370459 - Polish the onboarding overlay UI to align with new visual spec; https://reviewboard.mozilla.org/r/147936/#review154296 Looks good to me. Thanks.

Comment on attachment 8876609 [details] Bug 1370459 - Polish the onboarding overlay UI to align with new visual spec; https://reviewboard.mozilla.org/r/147936/#review154320

thanks!

Pushed by rchien@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/5fa6bbe236e0 Polish the onboarding overlay UI to align with new visual spec;r=Fischer,mossop,rexboy

Backed out for failing browser_parsable_css.js: icons_tour-complete.svg missing: https://hg.mozilla.org/integration/autoland/rev/7e97fe54970f987fbc4a18422cd08013e1561a40 Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=5fa6bbe236e081e36b80264f6456de04db42af99&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=107599308&repo=autoland > TEST-UNEXPECTED-FAIL | browser/base/content/test/static/browser_parsable_css.js | missing resource://onboarding/img/icons_tour-complete.svg referenced from resource://onboarding/onboarding.css

fixed Bug 1373731 and bug 1373612 as well

Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/49d4a1aa666a Polish the onboarding overlay UI to align with new visual spec;r=Fischer,mossop,rexboy

I have verified this fix on today's nightly.