We are landing a first version of the devtools onboarding page in bug 1361080. Already received some feedback from Victoria. Listing here: - remove the left margin on the button - change .left-pane background-size to 85%;? (The otter looks a bit big :)) - could .box have less width -- e.g. width 850px;? - h1 font size: change to 33px - Paragraphs: add line-height: 22px; - for "Learn more about DevTools" links, we should use this external link icon http://design.firefox.com/photon/components/link.html - about debugging message - remove "the" from "the Firefox DevTools." - install complete page - add a comma after "To get started" All blue buttons should follow new guidelines: - 2px instead of 3px radius - On press, should be this dark blue color: #002275 - normal font weight

Victoria: I normally fixed the issues you raised by email. Also removed the three last features in the installation complete page (style editor, web audio, scratchpad). Let me know if I can update anything else! I have a try push ongoing at https://treeherder.mozilla.org/#/jobs?repo=try&revision=17a0ff4ae08998cc5792a0ddd90ff33296666e2d . It should have OSX builds a bit later, in case you want to try this. Screenshots: https://docs.google.com/a/mozilla.com/document/d/1HY7XP0XiFzG_wPYm9qqVXMxsD-Vls8lYPHdlEmP27oc/edit?usp=sharing

Nicolas: assigned you the review a bit randomly, so let me know if you don't have the bandwidth for this!

Comment on attachment 8918897 [details] Bug 1408330 - UI polish on DevTools onboarding page; https://reviewboard.mozilla.org/r/189784/#review196666 Looks good, I only have a couple of comments. Also, I'm curious, do we need the file to be xhtml ? couldn't we have plain html ? ::: devtools/shim/aboutdevtools/aboutdevtools.css:2 (Diff revision 4) > .box { > - width: 980px; > + width: 850px; could it be possible that this pages appear on a narrower viewport ? I would feel better if we use vw and max-width: 850px ::: devtools/shim/aboutdevtools/aboutdevtools.css:57 (Diff revision 4) > .feature-desc { > margin: 1em 20px > } > > -a { > - color: #0A84FF; > +.installpage-link { > + color: #0060df; This looks like it is `--blue-60` I guess we don't have access to the theme variables here, but it would be nice to re-declare them at the top and use them in the document (so we can do future style refactor easily)

Comment on attachment 8919675 [details] Bug 1408330 - remove mentions about styleeditor, webaudio and scratchpad; https://reviewboard.mozilla.org/r/190584/#review196668 Seems good to me, thanks !

Thanks for the reviews! (In reply to Nicolas Chevobbe [:nchevobbe] from comment #9) > Comment on attachment 8918897 [details] > Bug 1408330 - UI polish on DevTools onboarding page; > > https://reviewboard.mozilla.org/r/189784/#review196666 > > Looks good, I only have a couple of comments. > Also, I'm curious, do we need the file to be xhtml ? couldn't we have plain > html ? The main reason for using XHTML here is to use DTDs for localized strings. > > ::: devtools/shim/aboutdevtools/aboutdevtools.css:2 > (Diff revision 4) > > .box { > > - width: 980px; > > + width: 850px; > > could it be possible that this pages appear on a narrower viewport ? > I would feel better if we use vw and max-width: 850px > Good point. I assumed by vw you meant width: 100vw. I think we need a follow up to make sure the page displays nicely on narrow viewports. > ::: devtools/shim/aboutdevtools/aboutdevtools.css:57 > (Diff revision 4) > > .feature-desc { > > margin: 1em 20px > > } > > > > -a { > > - color: #0A84FF; > > +.installpage-link { > > + color: #0060df; > > This looks like it is `--blue-60` > I guess we don't have access to the theme variables here, but it would be > nice to re-declare them at the top and use them in the document (so we can > do future style refactor easily) Done! I added photon variables for all the colors and they are defined in the beginning of the file now.

Pushed by jdescottes@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/5d4bbcac2b7b UI polish on DevTools onboarding page;r=nchevobbe https://hg.mozilla.org/integration/autoland/rev/661dc33e6491 remove mentions about styleeditor, webaudio and scratchpad;r=nchevobbe

https://hg.mozilla.org/mozilla-central/rev/5d4bbcac2b7b https://hg.mozilla.org/mozilla-central/rev/661dc33e6491

Hi Julian, this is looking great! 1. We have a new polished otter graphic from Sean! The SVG is here: https://drive.google.com/open?id=0ByWhculJS-ivWDNSNE5CZjRSWHM I'm not totally sure if I exported this file correctly, but it should have a mostly transparent background, with white areas in the clouds, otter belly etc. There's an PNG version (https://drive.google.com/open?id=0ByWhculJS-ivRUp4SmdYUk1HZVE) that shows what I expect it to look like. If you could get the sizing to visually match your current implementation, that would be great! 2. It looks like the h1s are a bit too big - could we make them the equivalent of 33px? I think that should keep the "welcome" title all on one line as well. 3. Couple styling tweaks for the Install Complete page: - Could we make the icons smaller, e.g. width: 55% on .feature-icon? - Other CSS changes that would be great: .feature-name font-size: 28px; font-weight: 300; margin: 10px 0; .feature margin: 10px 0; .feature-list margin: 60px 20px; line-height: 1.5em; for all paragraphs including feature descriptions, .welcome-message, footer p Let me know if you need clarification for any of this. Thanks!

One last thing - the "Learn more about DevTools" link looks like it's a slightly different color from the open-in-new icon. Could you make them both Blue 60? I think the icon is a tiny bit too low as well. We could borrow this styling from the design system (whichever parts are applicable): main section a:not([class])[href*="//"]::after { background-image: url(/photon/static/665e8c7728f8db0529845d445cd9d485.svg); background-repeat: no-repeat; background-size: 16px 16px; content: ""; display: inline-block; height: 16px; margin: -.3rem .15rem 0 .25rem; vertical-align: middle; width: 16px; }

Yay, good to see this is in Nighty now! I'm requesting a few more style tweaks: For the initial pages... .box padding: 17% 0 50px 0; .buttons-container margin-top: 5px; For the install complete page... .box padding: 50px 0 50px 0; .newsletter-title font-size: 17px; font-weight: 500; margin-top: 26px; margin-bottom: -4px; The guidelines on input field focus rings has changed from teal to Blue 50 http://design.firefox.com/photon/components/input-fields.html #email padding: 8px; button padding: 8px 20px; .feature-link margin-top: 10px; footer padding-bottom: 15px; Let me know if you have any questions!

Thanks for having a look Victoria! Few questions: > The guidelines on input field focus rings has changed from teal to Blue 50 http://design.firefox.com/photon/components/input-fields.html Is the thick box-shadow also part of the specs? I did it with blue-50-alpha15, 3px but want to confirm. > button > padding: 8px 20px; To make sure: only for the subscribe button, not for the ones in the initial page? Some additional questions from Nicolas during the review of Bug 1408334. - the email input has no label, just a placeholder. Should we add one? > I know this is by design, but placeholders do not replace label > (https://www.nngroup.com/articles/form-design-placeholders/) - what about changing the placeholder from Email? > I'd rather display a `email@example.com` placeholder and have a > proper label. but this can be discussed later. Also I will be using Bug 1412504 for this follow-up work (since this bug is already closed).

Forgot to ni? Victoria: see my comment & questions above.

(In reply to Julian Descottes [:jdescottes][:julian] from comment #19) > Is the thick box-shadow also part of the specs? I did it with > blue-50-alpha15, 3px but want to confirm. Yep! Sounds good! > > button > > padding: 8px 20px; > > To make sure: only for the subscribe button, not for the ones in the initial > page? Yes, only the subscribe button needs to be smaller, to match the smaller input field. The standalone button in the initial pages is a lot more important. > Some additional questions from Nicolas during the review of Bug 1408334. > > - the email input has no label, just a placeholder. Should we add one? > > I know this is by design, but placeholders do not replace label > > (https://www.nngroup.com/articles/form-design-placeholders/) > > - what about changing the placeholder from Email? > > I'd rather display a `email@example.com` placeholder and have a > > proper label. but this can be discussed later. I think for this one-input form (with explanatory preceding text) it's fine to just use the "Email" placeholder. > Also I will be using Bug 1412504 for this follow-up work (since this bug is > already closed). Ah ok! I meant to post my comments in that bug but it was taking me too long to find it ^_^;;

Btw, for the focus ring box shadow, it should have a hard edge rather than soft. This is what Firefox Sync uses: box-shadow: 0 0 0 2px hsla(214.5, 100%, 87.1%, 0.8);

Thanks for the feedback! (In reply to Victoria Wang [:victoria] from comment #22) > Btw, for the focus ring box shadow, it should have a hard edge rather than > soft. This is what Firefox Sync uses: > box-shadow: 0 0 0 2px hsla(214.5, 100%, 87.1%, 0.8); For now I am using box-shadow: 0 0 0 3px rgba(10, 132, 255, 0.15); (--blue-50 at 15% alpha) On the mockups it looked like 3px. Will upload a screenshot if you want to settle between the two. On our background, the actual colors are: - current patch: #D5E8FB - firefox sync colors: #C9DFFE If I use --blue-50 at 20% alpha that's almost the same color as Firefox sync. I can switch to that.

Thanks for the attention to detail on this! The sync page must be a bit outdated. It does looks like 3px in the DSG, so sounds good to continue with that, and Blue 50 at 15% alpha sounds like a good solution for the color.