632530 - New CSS style doesn't differentiate optional and required parameters

Status: RESOLVED FIXED

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

Description

[Drew Willcoxon :adw]

The old CSS style styled optional parameters and options keys in italics, but the new style doesn't style them any differently from required parameters.  I'm not sure if subtle italics are the best way to signal "optional," but optional and required parameters need to be differentiated in some clear manner.

Also, the "Add-on SDK Documentation v1.0b3pre" text at the bottom left and "view source" link at the bottom right of every page need to be re-styled (or removed).

Comment 1

[Will Bamberg [:wbamberg]]

Attachment: Distinguish optional parameters using square brackets

Seems a bit more obvious to me than italics. This patch also cleans up the HTML emitted by apirenderer, which should have no effect on the output other than to make the source more readable.

Comment 2

[Drew Willcoxon :adw]

Comment on attachment 518817 [details] [diff] [review]
Distinguish optional parameters using square brackets

I like it!  My only concern is that, like the italics, it's not stated anywhere that brackets imply optionality.  Once you know that it's great, but until then I might wonder why some things have brackets and some don't.  The best alternative I can think of is to actually say "Optional" next to optional things.  If you like that idea feel free to use it, if not, that's fine.  r+!

>diff --git a/python-lib/cuddlefish/apirenderer.py b/python-lib/cuddlefish/apirenderer.py

> def tag_wrap(text, classname, tag = 'div'):
>-    return ''.join(['\n<' + tag + ' class="', classname, '">\n\n', \
>-                   text, '\n</'+ tag + '>\n\n'])
>+    return ''.join(['\n<' + tag + ' class="', classname, '">', \
>+                   text, '\n</'+ tag + '>\n'])
>+
>+def tag_wrap_inline(text, classname, tag = 'div'):
>+    return ''.join(['\n<' + tag + ' class="', classname, '">', \
>+                   text, '</'+ tag + '>\n'])

My comment in bug 636319 about tag_wrap_inline applies here too.

Comment 3

[Drew Willcoxon :adw]

Attachment: cfx error

Oh, I just noticed that cfx errored when I tested this patch.  It didn't happen immediately, so maybe it was ping-related?  The lines are long so I'm attaching the output so it's easier to read.

Comment 4

[Drew Willcoxon :adw]

Interestingly it does not seem to affect my ability to browse the docs.

Comment 5

[Will Bamberg [:wbamberg]]

I think this error means that a browser window has closed, and the timing is such that the Python web server has an HTTP response to send it, but can't because the browser's not listening any more. So it should not affect your ability to browse the docs, and I think can be considered non-harmful. Still.

I don't suppose it's reproducible, is it?

Comment 6

[Drew Willcoxon :adw]

I can't reproduce now with or without the patch.  I also don't recall closing a doc window before the error happened; I think I just ran cfx docs, browsed for a little bit, and then noticed the error.

Comment 7

[Will Bamberg [:wbamberg]]

Fixed by: https://github.com/mozilla/addon-sdk/commit/eda344d7b532f323d0be48e92216a45580442002

I'm not sure what to do about that error. I would suggest raising another bug, but it's hard for me to investigate it unless it's reproducible. I do see it myself, but very occasionally, and it does not stop me using the docs.