The functionality of the X icon button in the onboarding overlay was done in Bub 1357005. However, the visual is still not yet done with the formal visual asset.

The close button change is based on the new UI spec https://mozilla.invisionapp.com/share/4MBDUMS5W#/screens/230316106 and reuse the photon close button in Bookmark sidebar Here's the screencast http://g.recordit.co/EIcgnXZ8sv.gif

Comment on attachment 8875144 [details] Bug 1369282 - Update the close button style in the onboarding overlay to fit the spec; https://reviewboard.mozilla.org/r/146540/#review150508 ::: browser/extensions/onboarding/content/onboarding.css:46 (Diff revision 1) > position: absolute; > top: 15px; > offset-inline-end: 15px; > + width: 16px; > + height: 16px; > + background-image: url(chrome://browser/skin/sidebar/close.svg); Not sure if this chrome protocol would work in the content process. Better to move this svg inside our folder.

Comment on attachment 8875144 [details] Bug 1369282 - Update the close button style in the onboarding overlay to fit the spec; https://reviewboard.mozilla.org/r/146540/#review150518 ::: browser/extensions/onboarding/content/onboarding.css:52 (Diff revision 2) > + background-position: center center; > + background-repeat: no-repeat; > + padding: 12px; > +} > + > +#onboarding-overlay-close-btn:hover { Does specs require the hovering state change?

Comment on attachment 8875144 [details] Bug 1369282 - Update the close button style in the onboarding overlay to fit the spec; https://reviewboard.mozilla.org/r/146540/#review150524 ::: browser/extensions/onboarding/content/onboarding.css:52 (Diff revision 2) > + background-position: center center; > + background-repeat: no-repeat; > + padding: 12px; > +} > + > +#onboarding-overlay-close-btn:hover { Yes, I show the work version to michael and he says we need that.

(but the hover state does not described in spec) In last update I also add a README file to note we copy the close.svg from `browser/themes/shared/sidebar/close.svg`. And add 2 landing rules that we have discussed before.

Comment on attachment 8875144 [details] Bug 1369282 - Update the close button style in the onboarding overlay to fit the spec; https://reviewboard.mozilla.org/r/146540/#review150578 Thanks, looks good to me. ::: browser/extensions/onboarding/README.md:14 (Diff revision 3) > +## Landing rules > + > +We would apply some rules: > + > +* Avoid `chrome://` in `onbaording.js` since onboarding is intented to be injected into a normal content process page. > +* All styles and ids should be prefixed by `onboarding-overlay-` to avoid conflict with the origin page. Thanks for this note. Could use `onboaridng-*` because we will have `onboarding-notification-*`? `onboarding-overlya-notification-*` should be fine but a little long :) ::: browser/extensions/onboarding/content/onboarding.css:46 (Diff revision 3) > position: absolute; > top: 15px; > offset-inline-end: 15px; > + width: 16px; > + height: 16px; > + background-image: url(img/overlay-close.svg); Could we use the filename of "onboarding-close-icon.svg"? 2 reasons for this: (1) the main reason; the notification bar is going to use this icon too and (2) the minor reason; denote this is a icon. Thanks. p.s I could update the filename when landing the notification bar then if you liked.

Comment on attachment 8875144 [details] Bug 1369282 - Update the close button style in the onboarding overlay to fit the spec; https://reviewboard.mozilla.org/r/146540/#review150764 Looks good to me with 2 typos need to be fixed. ::: browser/extensions/onboarding/README.md:14 (Diff revision 4) > +## Landing rules > + > +We would apply some rules: > + > +* Avoid `chrome://` in `onbaording.js` since onboarding is intented to be injected into a normal content process page. > +* All styles and ids should be format as `onboarding-*` to avoid conflict with the origin page. typo: should be formatted ::: browser/extensions/onboarding/README.md:15 (Diff revision 4) > + > +We would apply some rules: > + > +* Avoid `chrome://` in `onbaording.js` since onboarding is intented to be injected into a normal content process page. > +* All styles and ids should be format as `onboarding-*` to avoid conflict with the origin page. > +* All strings in `locales` should be format as `onboarding.*` for consistency. same above

Comment on attachment 8875144 [details] Bug 1369282 - Update the close button style in the onboarding overlay to fit the spec; https://reviewboard.mozilla.org/r/146540/#review150796 ::: browser/extensions/onboarding/README.md:3 (Diff revision 4) > +# Onboarding > + > +System addon to provide the onboarding overlay for User friendly Tours. No need to capitalise the U and T here.

thanks!

Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/5decd51af7a9 Update the close button style in the onboarding overlay to fit the spec;r=Fischer,mossop,rexboy

Backed out in https://hg.mozilla.org/integration/autoland/rev/757a3c336d6e because we apparently so very very much don't want you to copy an existing file that we break the build when you do, https://treeherder.mozilla.org/logviewer.html#?job_id=105376912&repo=autoland

Fischer, since we can still access chrome:// in script, do you think we can reuse the existing one with `chrome://`?

(In reply to Fred Lin [:gasolin] from comment #17) > Fischer, since we can still access chrome:// in script, do you think we can > reuse the existing one with `chrome://`? Yes, please.

Thanks Phil, I think its ready to land again.

Pushed by cbook@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/e92dd6dae230 Update the close button style in the onboarding overlay to fit the spec;r=Fischer,mossop,rexboy

I have confirmed this fix on today's nightly build.

Based on comment 23 I am updating the flags accordingly.