Now that bug 646500 has landed, we need to integrate it into the SDK docs.

So I've had a go at integrating the results of the design work from bug 646500 into the SDK docs, and pushed the results to: https://github.com/wbamberg/jetpack-sdk/tree/gdesign. At the moment the CSS itself is pretty horrendous, so

(In reply to comment #1) > So I've had a go at integrating the results of the design work from bug > 646500 into the SDK docs, and pushed the results to: > https://github.com/wbamberg/jetpack-sdk/tree/gdesign. At the moment the CSS > itself is pretty horrendous, so ... I'm just asking for an opinion about whether the visual design and behaviour looks good enough for Jetpack and for AMO, then once we're OK with that I will go through and clean up the CSS moderately. As we discussed, I didn't follow the graphic design absolutely slavishly; for example, changes that we made in bug 641863 were made for good reasons that still apply, and I didn't want to regress that. Two questions: - I understand that it's important to have a link back to the main AMO site. I just did that by adding an "AMO Home" link in the global header, I couldn't think of anything better. - Also, I did keep the "Register or log in" link, which was in bug 646500, but I'm not sure whether it's right, here. I think I'd vote to remove it, but am easy either way.

> - I understand that it's important to have a link back to the main AMO site. > I just did that by adding an "AMO Home" link in the global header, I > couldn't think of anything better. That's fine. We have a "Back to add-ons" link on https://addons.mozilla.org/en-US/developers in the same location. I'd suggest using that wording for consistency. > - Also, I did keep the "Register or log in" link, which was in bug 646500, > but I'm not sure whether it's right, here. I think I'd vote to remove it, > but am easy either way. Those links are only shown if the user is logged in. Since this is a disconnected app which can't reflect that, I'd suggest you remove them also.

Will: any ETA on a patch for this?

Myk: as soon as you and Wil and Dave are happy with this look, I will clean up the CSS so it's less nasty. That will take me less that half a day, I'd estimate.

You can browse the docs here: http://members.shaw.ca/vill/sdk/ to save the trouble of building them using the SDK.

My only comments are above. I thought it was fine.

Comment on attachment 535495 [details] [diff] [review] AMO-style for SDK docs Close! A few issues... There's almost a pageful of space (i.e. about 700 pixels) between the end of the content and the footer on each page. A little padding seems reasonable (other Developer Hub pages I surveyed seem to have 2-20 pixels), but a pageful seems too much. At the default zoom level, every page is several pixels wider than the width of my screen when my screen is 1280 or 1366 pixels wide (although not when it's 1024 pixels wide), causing a horizontal scrollbar to appear. Zooming in three times makes this go away, but pages shouldn't do this regardless of the zoom level (except maybe once you zoom in very closely, since it's hard to avoid horizontal scroll at that point), and especially not at the default zoom level. diff --git a/packages/addon-kit/tests/test-page-mod.js b/packages/addon-kit/tests/test-page-mod.js +<header id="global-header"> + <div class="funnel"> + <a id="mozilla-tab" href="http://www.mozilla.org/?ref=logo">Mozilla</a> + <div class="menu"> + <p> + <a href="https://addons.mozilla.org/">Back to Add-ons</a> + </p> + </div> +</header> This link seems wonky to me. The text is fine--as Wil noted, it's consistent with the text on Developer Hub pages--but it suggests that these docs are fully integrated into the Developer Hub, whereas they seem more like a site-within-a-site, since their own navigation (i.e. the sidebar) is internal-facing, and they don't include most of the Hub-wide navigation like breadcrumbs and the menu/search bar. If we expect readers to get to these docs from the Developer Hub, and we want them to have a way to get back to where they came from after browsing around a while without having to press the Back button a bunch of times, then it seems like it would make more sense to have a Back to Developer Hub link. That too is a bit wonky, since the Developer Hub is itself a site-within-a-site (being embedded within the overall Add-ons site), which makes the SDK docs a site-within-a-site-within-a-site. Nevertheless, it still seems like the better option (in that it is more likely to be useful to readers of these docs) until we really integrate the SDK docs into the overall Hub navigation structure. Nevertheless, it's Wil's site, so he has final say here. (It's also wonky that this link exists at all on the locally-hosted docs, but I don't see an easy solution to that; long-term we need to be able to generate different content for the locally-hosted docs, but this is ok for now.) +<header id="site-header"> + <div class="funnel"> + <h1> + <a href="dev-guide/welcome.html">Add-on SDK<span></span></a> + </h1> + </div> +</header> Is it necessary to have an empty <span></span> in this link? <div class="sidebar-subsection"> + <div class="divider"></div> <h3 class="sidebar-subsection-header"><a href="dev-guide/addon-development/experimental.html">Experimental</a></h3> <div class="sidebar-subsection-contents"> <ul> Note: the patch didn't apply because of this chunk, which modifies the Experimental section, which doesn't exist anymore, although the problem was trivial to work around. diff --git a/static-files/css/base.css b/static-files/css/base.css +pre,code,kbd,tt,samp,tt { + font-family: "andale mono",monospace; } h1 { @@ -69,38 +121,14 @@ h4 { font-size: 1em; } -h5, h6 { +h5,h6 { Nit: here and elsewhere, keep a space after a comma in a comma-delimited list of selectors. diff --git a/static-files/css/footer.css b/static-files/css/footer.css diff --git a/static-files/css/header.css b/static-files/css/header.css diff --git a/static-files/css/sdk-docs.css b/static-files/css/sdk-docs.css +html, body { + min-width: 570px; /* 2 x sidebar fullwidth + main-content padding */ + height:100%; Nit: here and elsewhere, keep a space after a colon in a rule. - margin-top: 40px; /* to line up with sidebar: title-height + title padding */ + margin-top: 0; /* to line up with sidebar: title-height + title padding */ Nit: since you're making changes here, reduce the width of the line to 80 columns in the process. + background: #E0EFFD; Nit: it would probably improve readability to specify colors using lowercase consistently, i.e. #e0effd.

(In reply to comment #9) > There's almost a pageful of space (i.e. about 700 pixels) between the end of > the content and the footer on each page. A little padding seems reasonable > (other Developer Hub pages I surveyed seem to have 2-20 pixels), but a > pageful seems too much. > > > At the default zoom level, every page is several pixels wider than the width > of my screen when my screen is 1280 or 1366 pixels wide (although not when > it's 1024 pixels wide), causing a horizontal scrollbar to appear. Zooming > in three times makes this go away, but pages shouldn't do this regardless of > the zoom level (except maybe once you zoom in very closely, since it's hard > to avoid horizontal scroll at that point), and especially not at the default > zoom level. Note: I see these problems on the test site referenced in comment 6 <http://members.shaw.ca/vill/sdk/>, not on my local version of the site, although my local version is affected by Windows-specific bug 660695, so it's hard to say for sure whether the problems exist there.

(In reply to comment #10) > (In reply to comment #9) > > There's almost a pageful of space (i.e. about 700 pixels) between the end of > > the content and the footer on each page. A little padding seems reasonable > > (other Developer Hub pages I surveyed seem to have 2-20 pixels), but a > > pageful seems too much. > > > > > > At the default zoom level, every page is several pixels wider than the width > > of my screen when my screen is 1280 or 1366 pixels wide (although not when > > it's 1024 pixels wide), causing a horizontal scrollbar to appear. Zooming > > in three times makes this go away, but pages shouldn't do this regardless of > > the zoom level (except maybe once you zoom in very closely, since it's hard > > to avoid horizontal scroll at that point), and especially not at the default > > zoom level. > > > Note: I see these problems on the test site referenced in comment 6 > <http://members.shaw.ca/vill/sdk/>, not on my local version of the site, > although my local version is affected by Windows-specific bug 660695, so > it's hard to say for sure whether the problems exist there. Yeah, I would expect that. The version in the patch is later than the version at shaw.ca, and I had managed to fix those problems.

> At the default zoom level, every page is several pixels wider than the width > of my screen when my screen is 1280 or 1366 pixels wide (although not when > it's 1024 pixels wide), causing a horizontal scrollbar to appear. Zooming > in three times makes this go away, but pages shouldn't do this regardless of > the zoom level (except maybe once you zoom in very closely, since it's hard > to avoid horizontal scroll at that point), and especially not at the default > zoom level. I think this is due to the footer's min-width. I adjusted it here to be equal to the min-width in AMO (978px). > > diff --git a/packages/addon-kit/tests/test-page-mod.js > b/packages/addon-kit/tests/test-page-mod.js > > +<header id="global-header"> > + <div class="funnel"> > + <a id="mozilla-tab" > href="http://www.mozilla.org/?ref=logo">Mozilla</a> > + <div class="menu"> > + <p> > + <a href="https://addons.mozilla.org/">Back to Add-ons</a> > + </p> > + </div> > +</header> > > This link seems wonky to me. The text is fine--as Wil noted, it's > consistent with the text on Developer Hub pages--but it suggests that these > docs are fully integrated into the Developer Hub, whereas they seem more > like a site-within-a-site, since their own navigation (i.e. the sidebar) is > internal-facing, and they don't include most of the Hub-wide navigation like > breadcrumbs and the menu/search bar. > > If we expect readers to get to these docs from the Developer Hub, and we > want them to have a way to get back to where they came from after browsing > around a while without having to press the Back button a bunch of times, > then it seems like it would make more sense to have a Back to Developer Hub > link. > > That too is a bit wonky, since the Developer Hub is itself a > site-within-a-site (being embedded within the overall Add-ons site), which > makes the SDK docs a site-within-a-site-within-a-site. Nevertheless, it > still seems like the better option (in that it is more likely to be useful > to readers of these docs) until we really integrate the SDK docs into the > overall Hub navigation structure. > > Nevertheless, it's Wil's site, so he has final say here. This all makes sense to me. I've made this change here, but can revert it in the final patch if Wil prefers the original. > +<header id="site-header"> > + <div class="funnel"> > + <h1> > + <a href="dev-guide/welcome.html">Add-on SDK<span></span></a> > + </h1> > + </div> > +</header> > > Is it necessary to have an empty <span></span> in this link? It is! In the AMO header the logo is displayed using CSS like this: #site-header h1 a span { background: url("../media/firefox-logo.png") no-repeat scroll 0 0 transparent; height: 62px; left: 0; position: absolute; top: 0; width: 54px; }

Comment on attachment 536202 [details] [diff] [review] New patch (In reply to comment #12) > I think this is due to the footer's min-width. I adjusted it here to be > equal to the min-width in AMO (978px). That seems to have fixed the problem! Everything else looks good, too. r+a=myk

Landed in: https://github.com/mozilla/addon-sdk/commit/3183b36e151797a058ee7cdfe03526c8e2e2bb9e