595245 - Docs of high-level UI-related APIs should include screenshots

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, defect)

Description

[Drew Willcoxon :adw]

Docs of high-level UI-related APIs should include screenshots of what those APIs produce.  The tutorial too.

Comment 1

[Drew Willcoxon :adw]

And where the platforms have different UI (context-menu, notifications, panel?), we should include screenshots for each platform.  I like how SUMO does this: they sniff your browser to see which platform you're running on and show you that content by default, but there are toggle buttons for other platforms so you can check them too.  e.g.: http://support.mozilla.com/en-US/kb/Private+Browsing

Comment 2

[Drew Willcoxon :adw]

People have been asking about widgets and the add-on bar on the mailing list recently, which brought this bug to my mind.

CC'ing Will in case he has any thoughts.

Comment 3

[Myk Melez [:myk] [@mykmelez]]

The Add-on SDK is no longer a Mozilla Labs experiment and has become a big enough project to warrant its own Bugzilla product, so the "Add-on SDK" product has been created for it, and I am moving its bugs to that product.

To filter bugmail related to this change, filter on the word "looptid".

Comment 4

[Drew Willcoxon :adw]

(In reply to comment #1)
> And where the platforms have different UI (context-menu, notifications,
> panel?), we should include screenshots for each platform.

This would be nice, but as a first pass, using shots from only one platform is OK.  Having shots at all is much better than not, and having them for all platforms is only a little better than that.  I'd also add "significantly different UI" -- context menus look pretty much the same everywhere, but Growl notifications on OS X appear significantly different from toaster notifications on Windows and Linux.  Maybe notifications is the only case, actually.

> I like how SUMO does
> this: they sniff your browser to see which platform you're running on and show
> you that content by default, but there are toggle buttons for other platforms
> so you can check them too.

On second thought, SUMO's use case is different from ours.  A visitor to SUMO only cares about the platform he's using, but Jetpack developers potentially have users on all platforms.  So if there are shots from multiple platforms, it doesn't really make sense to show only one platform at a time.

Comment 5

[Will Bamberg [:wbamberg]]

> On second thought, SUMO's use case is different from ours.  A visitor to SUMO
> only cares about the platform he's using, but Jetpack developers potentially
> have users on all platforms.  So if there are shots from multiple platforms, it
> doesn't really make sense to show only one platform at a time.

Yeah, that sounds reasonable. So for UI components that are close enough across platforms, just show one platform, and for those that are different enough, show several and make it explicit.

The bit I find difficult about what Sumo does is, how to present that "show me what it looks like for platform X" thing without the UI looking very busy, or without it being terribly obscure. I don't think I would have understood "Help With: *Mac OS X* " to mean that at all, if I hadn't been looking for it.

Comment 6

[Drew Willcoxon :adw]

Yeah, they changed their UI after I posted comment 1 -- not for the better I think.  For us, where shots per platform would be helpful, it's perfectly good to simply list them without a UI and DOM manipulation stuff like they do.

Comment 7

[Will Bamberg [:wbamberg]]

Attachment: Screenshots for UI modules

(In reply to comment #4)
> (In reply to comment #1)
> > And where the platforms have different UI (context-menu, notifications,
> > panel?), we should include screenshots for each platform.
> 
> This would be nice, but as a first pass, using shots from only one platform is
> OK.  Having shots at all is much better than not, and having them for all
> platforms is only a little better than that.  I'd also add "significantly
> different UI" -- context menus look pretty much the same everywhere, but Growl
> notifications on OS X appear significantly different from toaster notifications
> on Windows and Linux.  Maybe notifications is the only case, actually.

In this pass at any rate I've only included screenshots for OS X. Most of the UI modules look pretty similar on OS X and Linux, at least (even notifications look pretty similar on Ubuntu) and unfortunately it's difficult for me to get access to a Windows machine at the moment. So I decided it's much better to submit something now even if it's only for one platform.

Also, I've only included them in the 'module overview' page - including them in the API docs as well looked like too much repetition. But do let me know if you'd like them in there too, or if there are any other useful screenshots I didn't think of.

Comment 8

[Drew Willcoxon :adw]

Comment on attachment 509242 [details] [diff] [review]
Screenshots for UI modules

Looks good, r+ with the non-nit changes below.  (But I think the nits are worthwhile too!)

(In reply to comment #7)
> access to a Windows machine at the moment. So I decided it's much better to
> submit something now even if it's only for one platform.

Defintely!

> Also, I've only included them in the 'module overview' page - including them in
> the API docs as well looked like too much repetition.

Hmm.  I feel like the API Overview page should be as brief and to-the-point as possible, while each API reference page should be the canonical place to learn about a particular API -- introduction, example code, screenshots, and API ref.  If you see a link to an API ref on the mailing list or IRC or elsewhere, it's not obvious how to get to the screenshots from there, or even that there are screenshots.  OTOH, screenshots on the overview page are useful when you're trying to quickly decide which UI API meets your needs.

So I'm conflicted, but I think it's fine for now.

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

>+A panel is a dialog. Its content is specified as HTML and you can execute
>+scripts in it: so the appearance and behaviour of the panel is limited only
>+by what you can do using HTML, CSS and JavaScript.

Nit: I think a comma is more appropriate here than a colon, but up to you.

>+You can build the content locally or load it from a remote server.

Nit: To my mind, "build" implies DOM modification, but it's also possible to supply a simple, static HTML file.  (The "built" in the sentence that follows this one is right on, for instance.)

>+<div align='center'>

Nit: Please use double quotes for HTML attribute values.

>+<img src='media/screenshots/modules/panel-tabs-osx.jpg' alt='List open tabs panel'>

Whoa, the background of this image is really distracting.  Backgrounds are fine, but let's use a more subtle one.

Also, please use PNGs.  jpeg compression is especially bad for screenshots.

>+<img src='media/screenshots/modules/widget-content-osx.jpg' alt='Mozilla widget content'>

Nit: A sans-serif font would look better.

>+<img src='media/screenshots/modules/widget-icon-osx.jpg' alt='Mozilla widget icon'>

Yeurgh, bug 628113 bites.  Could you add some right-padding so that the widget doesn't overlap the window resizer?

>+<img src='media/screenshots/modules/reddit-panel-osx.jpg' alt='Reddit panel'>

Same here?

>+<img src='media/screenshots/modules/context-menu-image-osx.jpg' alt='Context-menu'>

Background here too.

>+<img src='media/screenshots/modules/notification-growl-osx.jpg' alt='Growl notification'>

Nice text!  The notification isn't centered within the shot though.  Actually, it would be good to provide more context in the shot, since unlike other APIs notifications appear on the desktop, not in or around the browser.  Context like the top-right corners of the desktop and browser.

>--- a/static-files/md/dev-guide/addon-development/implementing-simple-addon.md
>+++ b/static-files/md/dev-guide/addon-development/implementing-simple-addon.md

>+![translator context-menu](media/screenshots/translator/context-menu-osx.jpg)
>+
>+Select that item and the text you selected should be replaced with its English
>+translation:
>+
>+![translator context-menu](media/screenshots/translator/translated-osx.jpg)

Nits: I'm going to be really nitpicky here. :)  1) Add-on SDK is for Firefox 4 (hopefully anyway), so a screenshot of the Firefox 3.6 download page is backwards-looking.  (Maybe you could just crop out the "Firefox 3.6"; it's actually a little distracting.)  2) "Go to top gear" is... a weird translation.  It would be better if you could find an example that translated into remotely good English.  It's a nice moment when you first see this add-on in action, and a bad translation doesn't help convey that.  3) Only a small portion of the visible text was translated, so it takes more effort for the reader to parse what changed.  Better to translate a large chunk, or all of, the visible text.

Comment 9

[Will Bamberg [:wbamberg]]

Thanks for the feedback.
 
> >+<img src='media/screenshots/modules/widget-icon-osx.jpg' alt='Mozilla widget icon'>
> 
> Yeurgh, bug 628113 bites.  Could you add some right-padding so that the widget
> doesn't overlap the window resizer?
> 
> >+<img src='media/screenshots/modules/reddit-panel-osx.jpg' alt='Reddit panel'>
> 
> Same here?

I'm not sure about this. Isn't it better for the screenshots to reflect the reality? If bug 628113 will be fixed soon I'd like to wait until it is fixed before regenerating the screenshots and landing the patch. If it won't, I'd rather include these screenshots as they are.

Otherwise, if the icon looks OK in the screenshot but overlaps the resizer in reality, that will really confuse people.

Comment 10

[Drew Willcoxon :adw]

Yeah, I had the same thought but reconciled it like this: 1) It's possible (I assume) to define content as HTML containing a single image and right padding, so clever people will be able to replicate the screenshot.  2) Bug 628113 will be fixed for 1.0 (I assume, because it sucks).  3) It just looks really sloppy.

But you're probably right, so I leave it to your discretion. :)

Comment 11

[Will Bamberg [:wbamberg]]

 
> Nits: I'm going to be really nitpicky here. :)  1) Add-on SDK is for Firefox 4
> (hopefully anyway), so a screenshot of the Firefox 3.6 download page is
> backwards-looking.  (Maybe you could just crop out the "Firefox 3.6"; it's
> actually a little distracting.)  2) "Go to top gear" is... a weird translation.
>  It would be better if you could find an example that translated into remotely
> good English.  It's a nice moment when you first see this add-on in action, and
> a bad translation doesn't help convey that.  3) Only a small portion of the
> visible text was translated, so it takes more effort for the reader to parse
> what changed.  Better to translate a large chunk, or all of, the visible text.

That's hard! Google's translation is usually comprehensible but rarely reads well, and the more text that's included, the more likely it it to garble some of it. Also, the translator add-on doesn't preserve formatting, which means if you select multiple paragraphs, or multiple styles, it won't look pretty (we could presumably fix the translator here, but I think it would add more complexity than we want at this stage).

Anyway, I had a go and submitted the patch as:
https://github.com/mozilla/addon-sdk/commit/8f566c8eed97a9abf68b1e3cc5259492202c6fa0