The SDK's navigation bar shouldn't hide addon-kit packages under Internals docs

RESOLVED FIXED

Status

Add-on SDK
Documentation
RESOLVED FIXED
8 years ago
7 years ago

People

(Reporter: wbamberg, Assigned: wbamberg)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

8 years ago
Currently the left column navigation bar in the SDK lists the tutorial, then experimental/internals/reference stuff, which is less likely to be of interest to most people, and then list all package reference docs, including the addon-kit, which everyone needs all the time. 

The addon-kit package should precede all the more specialized stuff.
(Assignee)

Updated

8 years ago
Assignee: nobody → wbamberg
(Assignee)

Comment 1

8 years ago
Created attachment 498262 [details] [diff] [review]
Patch for bug 619707

Here's an attempt at reorganizing the SDK docs.

I think it matters that we get this structure right, because changing it repeatedly will confuse people. So I'd rather iterate a bit on this and get it more or less right than just patch something in now and have to change it radically again soon.

In this patch:

- at the top level there are 2 guides:
	- 'add-on developers guide' including:
		- tutorial
		- module reference with just addon-kit
		- 'programming with the sdk' with self-contained guides
		- reference
		- experimental
	- 'module developers guide' including:
		- internals guides
		- module reference with the other packages

The module developer guide still needs sorting out and that's OK I think, but the addon dev guide needs to be something we are happy with. I kept 'reference' because I do think there is a valid category of things that aren't modules but want reference docs rather than guide docs, like cfx.

One thing that might help navigation would be to be able to hide sections: then once people have internalized the tutorial they could hide it, and people who don't use reference much can hide that too. But I thought I'd look at that as part of the general changes to the look and feel of the whole thing, which is coming up next.
Attachment #498262 - Flags: review?(dietrich)
Attachment #498262 - Flags: feedback?(myk)

Comment 2

8 years ago
Comment on attachment 498262 [details] [diff] [review]
Patch for bug 619707

Will asked me for feedback on this patch, so here goes.

My first response was, whoa, that's a lot of "Add-on"s.  The top of the sidebar says "Add-on SDK", which is repeated as the title of the page just a few inches to the right.  Next in the sidebar is "Add-on Development", and next in the page is "Welcome to the Add-on SDK" followed by "The Add-on SDK...".

"Add-on SDK" as the sidebar title is appropriate, but it shouldn't be repeated as the "home" page title.  The home page's purpose is to welcome or introduce the reader to what he's about to read, and its title should reflect that.

If I'm someone new to Jetpack and I see "Add-on Development" in the sidebar, I would think, "Yes, that's the idea."  It's kind of a vacuous heading.  It only makes sense in comparison to module/library development, which as a new person I don't immediately understand or have an interest in.  I'm not sure if it's better to remove the heading or rename it.  ("Add-on Developer's Guide" might better imply that there are other kinds of guides.)

It's also not clear which headings in the sidebar are links.  It's not just a question of styling, but: why are some headings links and others aren't?

All that said, I agree with the approach of separating add-on development docs from module development docs, since the two audiences are different.  And even if I'm someone who's interested in both things, they're different tasks with different contexts.  So feedback+!

(In reply to comment #1)
> One thing that might help navigation would be to be able to hide sections: then
> once people have internalized the tutorial they could hide it, and people who
> don't use reference much can hide that too. But I thought I'd look at that as
> part of the general changes to the look and feel of the whole thing, which is
> coming up next.

I'm starting to think we have too much content to list it all in the sidebar.  (A good problem!)  Look at the home page.  A small amount of text followed by lots of whitespace, all because of the sidebar, which is now as tall as two of my MacBook's screens.  Why are we stuffing everything in there?  With this patch you've started to break the docs into two separate tracks.  Let's go further: tracks for add-on dev, module dev, API reference, SDK reference (cfx, Package Specification, Program ID, etc.), internals/experimentals, appendices, etc.  Each track is a collection of related pages focusing on a specific task and audience.  Each has its own sidebar or other place to quickly access its docs (and only its docs).  The Add-on SDK home page and sidebar would then simply list the different tracks, after pointing to the most important, the add-on dev track.
Attachment #498262 - Flags: feedback+
(Assignee)

Updated

7 years ago
Attachment #498262 - Flags: review?(dietrich)
Attachment #498262 - Flags: feedback?(myk)
(Assignee)

Comment 3

7 years ago
Thanks for your comments on this. I think they're very reasonable, and had a go at simplifying the sidebar, pushed to: https://github.com/wbamberg/jetpack-sdk/tree/reorganize - I've also made a start at using the same styles as addons.mozilla.org.

There is still lots more to be done in here - search, breadcrumbs, perhaps contents panels - but it's the basic structure I want to sort out.

I'm still not sure whether there should be an "Add-ons Development" heading to make the first sidebar section have the same structure as the next two: I think that looks more consistent but I take your point that it's obvious we are talking about add-on development, so the heading is kind of pointless. 

Having the same structure for "Module development" is potentially confusing ("Which 'Reference' am I looking at?") but I think it's probably clearer overall to have the same structure. Should probably have some indication in the sidebar showing where I am.
(Assignee)

Comment 4

7 years ago
Created attachment 499319 [details]
Reorganization of SDK docs

This is another attempt at reorganizing the SDK docs based on Drew's feedback. It incorporates the first set of style changes, and I'm not sure if that's a good thing, or just distracting. Feel free to give feedback on the style as well, or ignore it, or disable it by removing the last 3 stylesheets from index.html. I can remove it from this patch easily enough if you'd prefer reorganization and styling to land separately. 

I guess the big drawback about this organization is that reaching most content is now 2 clicks rather than one: but I agree that putting everything on the front page is not the best way to present stuff.
Attachment #498262 - Attachment is obsolete: true
Attachment #499319 - Flags: feedback?(myk)
Attachment #499319 - Attachment is patch: false
Attachment #499319 - Attachment mime type: text/plain → text/html
Comment on attachment 499319 [details]
Reorganization of SDK docs

I mentioned this to Will in conversation already, but in general I think this is a good idea, except that:

1. It seems like we're going to far to require two clicks to access all content.  I recommend an intermediate solution in which we require a click to move from section to section, but then we show the full table of contents for each section while you're in it, so it may be two clicks to get to content in a different section, but it's one click to get to content in the same section.

2. I'm agnostic regarding whether or not to bundle the style changes with these organization changes, but one issue I have with the style changes here is that they make the text and line spacing really large, so it's hard to fit a whole lot of content in the sidebar.  I would just tone that down a bit so we can fit a section's worth of table of contents (which should be about a page tall on a typical user's screen) in the sidebar.
Attachment #499319 - Flags: feedback?(myk) → feedback+
(Assignee)

Comment 6

7 years ago
Created attachment 504057 [details] [diff] [review]
Reorg and style changes for SDK docs

Thanks for your feedback. Here's an attempt at a patch. 

You get either all the 'add-on developer' track (aka Developer Guide) or all the 'module developer' track (aka Internals Guide) in the sidebar at one time. I think actually eventually we will need the ability to hide/collapse subsections in there, but I haven't added that yet cos we don't need it yet.

It's also here: https://github.com/wbamberg/jetpack-sdk/tree/reorganize if you prefer that thing.

Thanks!
Attachment #499319 - Attachment is obsolete: true
Attachment #504057 - Flags: review?(myk)
(Assignee)

Comment 7

7 years ago
Created attachment 504573 [details] [diff] [review]
Updated patch

Tiny style changes: a border for the sidebar and a darker link colour.
Attachment #504057 - Attachment is obsolete: true
Attachment #504573 - Flags: review?(myk)
Attachment #504057 - Flags: review?(myk)
Comment on attachment 504573 [details] [diff] [review]
Updated patch

>+++ b/static-files/css/api-reference.css

(Some nits follow; I know these are in the original file, so it's ok to leave them there when moving it in this patch, but this is something we should clean up at some point and make sure to check when taking new patches.)


>@@ -0,0 +1,138 @@
>+/*
>+Styles that are specific to the API reference doc structure
>+*/
>+
>+/*
>+Some global styles for api_refernece

Nit: refernece -> reference


>+*/
>+
>+.api_reference p
>+  {
>+  margin-top: 0px;
>+  margin-bottom: 5px;
>+  }

Nit: we should use the Mozilla convention for braces in CSS, which is either:

.api_reference p
{
  margin-top: 0px;
  margin-bottom: 5px;
}

or (preferably):

.api_reference p {
  margin-top: 0px;
  margin-bottom: 5px;
}


>+/* top level heading: something like "API Reference" */
>+h2.api_header
>+  {
>+  font-size: 200%;
>+  margin-top:30px;
>+  margin-bottom:10px;
>+  }

Nit: consistently put a space after the colon in rules.


>+/* next-level heading: something like "Methods", "Object properties" */
>+.api_reference>.api_component_group>.api_component>.api_component_group>.api_header

Nit: put whitespace around ">" in child selectors, i.e.:

.api_reference > .api_component_group > .api_component > .api_component_group > .api_header


>diff --git a/static-files/css/base.css b/static-files/css/base.css

>+html,body,div,span,applet,object,iframe,h1,h2,h3,h4,h5,h6,p,blockquote,pre,a,abbr,acronym,address,big,cite,code,del,dfn,em,font,img,ins,kbd,q,s,samp,small,strike,strong,sub,sup,tt,var,b,u,i,center,dl,dt,dd,ol,ul,li,fieldset,form,label,legend,table,caption,tbody,tfoot,thead,tr,th,td

Nit: these would be more readable if there was a space after each comma and the line was wrapped at 80 columns.  CSS selector sets can span multiple lines without any special line continuation character, so this can be simply:

html, body, div, span, applet, object, iframe, h1, h2, h3, h4, h5, h6, p,
blockquote, pre, a, abbr, acronym, address, big, cite, code, del, dfn, em, font,
img, ins, kbd, q, s, samp, small, strike, strong, sub, sup, tt, var, b, u, i,
center, dl, dt, dd, ol, ul, li, fieldset, form, label, legend, table, caption,
tbody, tfoot, thead, tr, th, td

However, do we really need to call out each individual HTML tag?  That list looks pretty comprehensive; can we not just use the universal selector (*)?


>diff --git a/static-files/css/sdk-docs.css b/static-files/css/sdk-docs.css

>+.sidebar-section
>+{
>+  background-color: #E0EFFD;
>+  margin-bottom: 1.0em;
>+  border-style:solid;
>+  border-color:#E0EFFD;
>+border-width:2px;
>+  -moz-border-radius: 5px;
>+}

Nit: indent border-width rule two spaces as well.


>diff --git a/static-files/index.html b/static-files/index.html

>+        <img alt="Firefox" src="https://addons.mozilla.org/media//img/zamboni/app_icons/firefox.png"> Add-on SDK</a>

Nit: media//img -> media/img


>diff --git a/static-files/js/main.js b/static-files/js/main.js

>+        if (pkg.readme)
>+          entry.find(".docs").html(markdownToHtml(pkg.readme));
>+        if (filter(pkg)) {
>+          pkgList.append(entry);
>+        }

Nit: omit braces around one-line conditional blocks consistently.


>diff --git a/static-files/md/dev-guide/addon-development/api-modules.md b/static-files/md/dev-guide/addon-development/api-modules.md

>+### [request](#module/addon-kit/request) ###
>+
>+This module enables you to make XMLHttpRequests from your add-on.

Nit: this is better described as enabling you to make "network requests", as the API is different from XMLHttpRequest's.


Otherwise, this looks great, r=myk.

I'm a bit worried about taking it at the end of the cycle, however, given how large it is (although I know a bunch of this is just moving files and the contents of files around).  How sure are you that it doesn't cause any regressions?


Also, I was a bit confused by the table of contents when I first opened the docs, as it wasn't clear to me that Getting Started and Introducing the APIs are tutorials in the Tutorials section.  I suspect that's because they aren't indented, while their contents are indented, so my brain assumed that indentation determines encapsulation.

Perhaps the fix is as simple as indenting the contents of the Tutorials section (and the other sections).  But we can address that in another patch.
Attachment #504573 - Flags: review?(myk) → review+
(Assignee)

Comment 9

7 years ago
> 
> I'm a bit worried about taking it at the end of the cycle, however, given how
> large it is (although I know a bunch of this is just moving files and the
> contents of files around).  How sure are you that it doesn't cause any
> regressions?
> 

I'm pretty sure: I've played around with it a bit and I'm reasonably sure that all the links still work, and that it looks all right. However, I agree that it's quite disruptive and it's the kind of change I'd really want to land at the beginning, not the end, of the development cycle.
(In reply to comment #9)
> I'm pretty sure: I've played around with it a bit and I'm reasonably sure that
> all the links still work, and that it looks all right. However, I agree that
> it's quite disruptive and it's the kind of change I'd really want to land at
> the beginning, not the end, of the development cycle.

Ok, let's land as soon as the tree reopens for 1.0b3 then!
(Assignee)

Comment 11

7 years ago
Submitted as: https://github.com/mozilla/addon-sdk/commit/708f4c7bb2c6282c26fcd35f98d3d1ae31ef38c2

I fixed the nits, but not this:

> I was a bit confused by the table of contents when I first opened
> the docs, as it wasn't clear to me that Getting Started and
> Introducing the APIs are tutorials in the Tutorials section. 
> I suspect that's because they aren't indented, while their contents
> are indented, so my brain assumed that indentation determines
> encapsulation.
>
> Perhaps the fix is as simple as indenting the contents of the
> Tutorials section (and the other sections).  But we can address
> that in another patch.

I did try this initially, but didn't like it. Still, I've spent too much time fooling around with it to be a very good judge of what works, and if your reaction is a common one it's trivial to change, just let me know...
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED

Comment 12

7 years ago
What's the reason for this change?

--- a/static-files/css/base.css
+++ b/static-files/css/base.css
[...]
+ul
+{
+  list-style: none;

In my opinion, the list without bullets (e.g. on the first page) looks very broken. I even started to look for Firefox bugs in bugzilla...
(Assignee)

Comment 13

7 years ago
(In reply to comment #12)
> What's the reason for this change?
> 
> --- a/static-files/css/base.css
> +++ b/static-files/css/base.css
> [...]
> +ul
> +{
> +  list-style: none;
> 
> In my opinion, the list without bullets (e.g. on the first page) looks very
> broken. I even started to look for Firefox bugs in bugzilla...

Yes, I agree. It came in because I used the 'standard' style used by AMO as a base for this, but it looks terrible. It'll be fixed tomorrow.

Comment 14

7 years ago
Excellent, thank you!
You need to log in before you can comment on or make changes to this bug.