1408330 - [dt-onboarding] Devtools onboarding page UI & UX polish

Status: RESOLVED FIXED

(DevTools :: General, enhancement, P3)

Description

[Julian Descottes [:jdescottes]]

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

Comment 1

[Julian Descottes [:jdescottes]]

Attachment: Bug 1408330 - UI polish on DevTools onboarding page;

Review commit: https://reviewboard.mozilla.org/r/189784/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/189784/

Comment 2

[Julian Descottes [:jdescottes]]

Comment on attachment 8918897 [details]
Bug 1408330 - UI polish on DevTools onboarding page;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/189784/diff/1-2/

Comment 3

[Julian Descottes [:jdescottes]]

Attachment: Bug 1408330 - remove mentions about styleeditor, webaudio and scratchpad;

Review commit: https://reviewboard.mozilla.org/r/190584/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/190584/

Comment 4

[Julian Descottes [:jdescottes]]

Comment on attachment 8918897 [details]
Bug 1408330 - UI polish on DevTools onboarding page;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/189784/diff/2-3/

Comment 5

[Julian Descottes [:jdescottes]]

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

Comment 6

[Julian Descottes [:jdescottes]]

Comment on attachment 8918897 [details]
Bug 1408330 - UI polish on DevTools onboarding page;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/189784/diff/3-4/

Comment 7

[Julian Descottes [:jdescottes]]

Comment on attachment 8919675 [details]
Bug 1408330 - remove mentions about styleeditor, webaudio and scratchpad;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/190584/diff/1-2/

Comment 8

[Julian Descottes [:jdescottes]]

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

Comment 9

[Nicolas Chevobbe [:nchevobbe]]

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 10

[Nicolas Chevobbe [:nchevobbe]]

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 !

Comment 11

[Julian Descottes [:jdescottes]]

Comment on attachment 8918897 [details]
Bug 1408330 - UI polish on DevTools onboarding page;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/189784/diff/4-5/

Comment 12

[Julian Descottes [:jdescottes]]

Comment on attachment 8919675 [details]
Bug 1408330 - remove mentions about styleeditor, webaudio and scratchpad;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/190584/diff/2-3/

Comment 13

[Julian Descottes [:jdescottes]]

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.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
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

Comment 15

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

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

Comment 16

[Victoria Wang [:victoria]]

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!

Comment 17

[Victoria Wang [:victoria]]

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;
}

Comment 18

[Victoria Wang [:victoria]]

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!

Comment 19

[Julian Descottes [:jdescottes]]

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 `[e-mail]` 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).

Comment 20

[Julian Descottes [:jdescottes]]

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

Comment 21

[Victoria Wang [:victoria]]

(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 `[e-mail]` 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 ^_^;;

Comment 22

[Victoria Wang [:victoria]]

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);

Comment 23

[Julian Descottes [:jdescottes]]

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.

Comment 24

[Julian Descottes [:jdescottes]]

Attachment: border_2px_3px.png

Comment 25

[Victoria Wang [:victoria]]

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.