Closed
Bug 1408331
Opened 7 years ago
Closed 7 years ago
[dt-onboarding] Add footer to download Firefox Dev Edition
Categories
(DevTools :: General, enhancement, P3)
DevTools
General
Tracking
(firefox58 fixed)
RESOLVED
FIXED
Firefox 58
Tracking | Status | |
---|---|---|
firefox58 | --- | fixed |
People
(Reporter: jdescottes, Assigned: jdescottes)
References
(Blocks 1 open bug)
Details
Attachments
(3 files)
We are landing a first version of the devtools onboarding page in bug 1361080. The mockups specify a footer to encourage users to download Firefox DevEdition. This bug is about adding the footer. The current mockups are at https://docs.google.com/document/d/1ku6Jn12XdTFNno0phhLjlBD2H43k5NYswALYnA_UVBA/edit#heading=h.vihrfx1f6nm2 but the footer design might be slightly updated.
Assignee | ||
Updated•7 years ago
|
Blocks: dt-onboarding
Assignee | ||
Updated•7 years ago
|
No longer blocks: dt-addon-uishim
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → jdescottes
Status: NEW → ASSIGNED
Assignee | ||
Comment 1•7 years ago
|
||
Victoria: I added the footer (see screenshot) as follows: - text color is white - background is a linear gradient between blue-60 (#0060df) and blue-80 (#002275) - dev-edition logo is 160px wide - "Learn more" link points to https://www.mozilla.org/en-US/firefox/developer/ Let me know if that sounds ok!
Attachment #8922021 -
Flags: feedback?(victoria)
Comment hidden (mozreview-request) |
Comment 3•7 years ago
|
||
This looks great! I think my only comment is the dev-edition logo seems (visually) a tiny bit too high compared to the text - I think just 3px too high :) Thanks for determining all these things like the exact colors and sizes btw - I hope to have a better process for mockup hand-offs in the future. (Like using https://app.zeplin.io/ or something)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 6•7 years ago
|
||
mozreview-review |
Comment on attachment 8922022 [details] Bug 1408331 - add footer to about:devtools page; https://reviewboard.mozilla.org/r/193006/#review199946 I have a few comment/questions before r+ this patch. Also, how can I test it ? ::: devtools/shim/aboutdevtools/aboutdevtools.css:73 (Diff revision 2) > -.installpage-link::after { > +.installpage-link::after, > +.footer-link::after { could we add a specific css class for that (`.external` ) ? Or a smarter rule (`a:not([href^="chrome"])`), or even plain `a` since we only have 2 of them, all externals. ::: devtools/shim/aboutdevtools/aboutdevtools.css:139 (Diff revision 2) > +.footer { > + display: flex; > + align-items: center; > + justify-content: center; > + width: 100%; > + height: 300px; is this a min-height we want ? I'm reluctant having fixed size ::: devtools/shim/aboutdevtools/aboutdevtools.xhtml:109 (Diff revision 2) > <p class="feature-desc">Find memory leaks and make your application zippy.</p> > </li> > </ul> > </div> > + > + <div class="footer"> what about using `<footer>` ? ::: devtools/shim/aboutdevtools/aboutdevtools.xhtml:112 (Diff revision 2) > </div> > + > + <div class="footer"> > + <img class="dev-edition-logo" > + src="chrome://devtools-shim/content/aboutdevtools/images/dev-edition-logo.svg" > + alt=""/> Nice to have put an alt attribute, maybe we could fill it (Firefox DevEdition logo) ? ::: devtools/shim/aboutdevtools/aboutdevtools.xhtml:115 (Diff revision 2) > + <img class="dev-edition-logo" > + src="chrome://devtools-shim/content/aboutdevtools/images/dev-edition-logo.svg" > + alt=""/> > + <div class="footer-message"> > + <h1 class="footer-message-title">Firefox Developer Edition</h1> > + <div> Maybe `<p>` tags would be more meaningful ? ::: devtools/shim/aboutdevtools/aboutdevtools.xhtml:119 (Diff revision 2) > + <a class="footer-link" href="https://www.mozilla.org/en-US/firefox/developer/" target="_blank"> > + Learn more > + </a> could this be put inside the previous container element with a `<br>` tag before it ? That way we can have consistent styling between the text and the link. Or maybe we don't want that, and in that case, keep it as is :) . ::: devtools/shim/aboutdevtools/images/dev-edition-logo.svg:1 (Diff revision 2) > +<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" viewBox="0 0 1024 1024"> I'm not sure, but shouldn't we add the license comment block at the top ? ::: devtools/shim/aboutdevtools/images/external-link.svg:1 (Diff revision 2) > <svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 16 16"> same question about license header
Attachment #8922022 -
Flags: review?(nchevobbe) → review-
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 9•7 years ago
|
||
Side-note: In this page, we say to the user to open the devtools with the menu/kbd shortcuts, which is nice, but I wonder if we could get a step further and have a button for each "feature" in this page that would open the correct panel ? This is something that looks natural to me, the user parsing this page, seeing "debugger" and clicking on it to open the debugger. I am really not sure how feasible this is though :)
Assignee | ||
Comment 10•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8922022 [details] Bug 1408331 - add footer to about:devtools page; https://reviewboard.mozilla.org/r/193006/#review199946 Thanks for the review. To test it, simply go to about:devtools . If you want to check that the footer is hidden if devtools are disabled, go to about:config and flip devtools.enabled to false. The page should update dynamically when you do so. > could we add a specific css class for that (`.external` ) ? > Or a smarter rule (`a:not([href^="chrome"])`), or even plain `a` since we only have 2 of them, all externals. Done with .external, I prefer to keep rules explicit for now. > is this a min-height we want ? I'm reluctant having fixed size I think you are right. Otherwise if the user has a very big screen, they might see some blank space below the footer. > could this be put inside the previous container element with a `<br>` tag before it ? That way we can have consistent styling between the text and the link. > > Or maybe we don't want that, and in that case, keep it as is :) . Not sure what this would achieve, but maybe I don't have the whole picture? Would this allow to simplify the CSS? > I'm not sure, but shouldn't we add the license comment block at the top ? done thanks! > same question about license header Done for this file + all existing SVGs in a second changeset.
Comment 11•7 years ago
|
||
So, this is looking fine, and I'm willing to r+ it. I have the feeling we can make it better, though. The nice thing is that we have a grid already defined in features-list, and I wonder if we couldn't have it author the whole page. So everything would go into the `<ul>` tag, like: ``` <ul xmlns="http://www.w3.org/1999/xhtml" class="features-list"> <img src="chrome://devtools-shim/content/aboutdevtools/images/otter.png" style="max-width: 100%;align-self: end;" /> <div class="right-pane"> <h1>Welcome to Firefox Developer Tools!</h1> <p class="message" id="welcome-message">You’ve successfully enabled DevTools! To get started, explore the Web Developer menu or open the tools with Cmd+Opt+I.</p> </div> <hr style="grid-column-start: 1;grid-column-end: -1;background-color: var(--grey-30);height: 1px;display: block;width: 100%;" class="" /> <li class="feature"> <img class="feature-icon" src="chrome://devtools-shim/content/aboutdevtools/images/feature-inspector.svg" alt="" /> <h3 class="feature-name">Inspector</h3> <p class="feature-desc">Inspect and refine code to build pixel-perfect layouts.</p> </li> […] </ul> ``` The logo would be one column large and the title would span across 2 columns. That way, everything is neatly aligned. Plus, it would make things easier in the bug where we plan to make this page work on narrow viewports. Tell me what you think about this (you can also tell me that it's bad :) )
Comment 12•7 years ago
|
||
mozreview-review |
Comment on attachment 8923770 [details] Bug 1408331 - add license header to about:devtools svg files; https://reviewboard.mozilla.org/r/194918/#review199988 Thanks !
Attachment #8923770 -
Flags: review?(nchevobbe) → review+
Assignee | ||
Comment 13•7 years ago
|
||
(In reply to Nicolas Chevobbe [:nchevobbe] from comment #11) > So, this is looking fine, and I'm willing to r+ it. > > I have the feeling we can make it better, though. The nice thing is that we > have a grid already defined in features-list, and I wonder if we couldn't > have it author the whole page. > So everything would go into the `<ul>` tag, like: > > > ``` > <ul xmlns="http://www.w3.org/1999/xhtml" class="features-list"> > <img src="chrome://devtools-shim/content/aboutdevtools/images/otter.png" > style="max-width: 100%;align-self: end;" /> > <div class="right-pane"> > <h1>Welcome to Firefox Developer Tools!</h1> > <p class="message" id="welcome-message">You’ve successfully enabled > DevTools! To get started, explore the Web Developer menu or open the tools > with Cmd+Opt+I.</p> > </div> > <hr style="grid-column-start: 1;grid-column-end: -1;background-color: > var(--grey-30);height: 1px;display: block;width: 100%;" class="" /> > > <li class="feature"> > <img class="feature-icon" > src="chrome://devtools-shim/content/aboutdevtools/images/feature-inspector. > svg" alt="" /> > <h3 class="feature-name">Inspector</h3> > <p class="feature-desc">Inspect and refine code to build > pixel-perfect layouts.</p> > </li> > > […] > </ul> > ``` > > The logo would be one column large and the title would span across 2 columns. > That way, everything is neatly aligned. > Plus, it would make things easier in the bug where we plan to make this page > work on narrow viewports. > > Tell me what you think about this (you can also tell me that it's bad :) ) Not sure about this. This means using non-list elements inside our <UL>? If we want to keep the list semantics we would have to define two grids here I guess. Could this be handled in a follow up?
Comment 14•7 years ago
|
||
> Not sure about this. This means using non-list elements inside our <UL>? If we want to keep the list semantics we would have to define two grids here I guess. Yes, you're right, until sub-grid is a thing. > Could this be handled in a follow up? Sure
Comment 15•7 years ago
|
||
mozreview-review |
Comment on attachment 8922022 [details] Bug 1408331 - add footer to about:devtools page; https://reviewboard.mozilla.org/r/193006/#review200054
Attachment #8922022 -
Flags: review?(nchevobbe) → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 18•7 years ago
|
||
Pushed by jdescottes@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/e25cb414c2f2 add footer to about:devtools page;r=nchevobbe https://hg.mozilla.org/integration/autoland/rev/21e6718ad041 add license header to about:devtools svg files;r=nchevobbe
![]() |
||
Comment 19•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/e25cb414c2f2 https://hg.mozilla.org/mozilla-central/rev/21e6718ad041
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox58:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
Comment 20•7 years ago
|
||
Comment on attachment 8922021 [details]
footer_screenshot.png
Clearing the feedback flag
Attachment #8922021 -
Flags: feedback?(victoria) → feedback+
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•