Closed
Bug 632530
Opened 14 years ago
Closed 14 years ago
New CSS style doesn't differentiate optional and required parameters
Categories
(Add-on SDK Graveyard :: Documentation, defect)
Add-on SDK Graveyard
Documentation
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: adw, Assigned: wbamberg)
Details
Attachments
(2 files)
44.47 KB,
patch
|
adw
:
review+
|
Details | Diff | Splinter Review |
2.06 KB,
text/plain
|
Details |
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).
Assignee | ||
Updated•14 years ago
|
Assignee: nobody → wbamberg
Assignee | ||
Comment 1•14 years ago
|
||
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.
Attachment #518817 -
Flags: review?(adw)
Reporter | ||
Comment 2•14 years ago
|
||
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.
Attachment #518817 -
Flags: review?(adw) → review+
Reporter | ||
Comment 3•14 years ago
|
||
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.
Reporter | ||
Comment 4•14 years ago
|
||
Interestingly it does not seem to affect my ability to browse the docs.
Assignee | ||
Comment 5•14 years ago
|
||
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?
Reporter | ||
Comment 6•14 years ago
|
||
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.
Assignee | ||
Comment 7•14 years ago
|
||
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.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•