Write a second set of documentation for extension options now that
options_ui is available.
For now this will be hosted on a new page options2.html since the
new options aren't stable yet. When they are, I'll switch over the
URLs.
R=mkearney@chromium.org
BUG=406890
Committed: https://crrev.com/8b1744726bc40f866df4c96293e472551c918600
Cr-Commit-Position: refs/heads/master@{#302357}
Hey Meggin, here are the new docs I wrote. Like I mentioned the _patch URL
doesn't work, and annoyingly neither does "git cl patch" (not sure why). I've
tried to start a server on my work machine, I'll send you the URL.
Also annoying: the tree is closed so I can't rebase this against my other patch,
so you'll just have to ignore that little refactor. I'll update it ASAP.
mkearney1
One general comment-- sort of an annoying fix, but a good idea. Can we make ...
Sweet, thanks.
Re the optionsv1 vs optionsv2 comment - I wanted to leave the canonical version
of the docs at the main options link, and then when Chrome 40 is stable switch
it over (e.g. then move options.html to optionsv1.html, move optionsv2.html to
options.html, add redirect from optionsv2 to options).
Because I wouldn't want people to only write code that works in dev channel -
and while it's not *hard* to explain, it's more words, and people don't read too
many words.
Btw I like "optionsv2" or "optionsV2" better than "options2". I'll update that
in a future patch.
https://codereview.chromium.org/691283003/diff/60001/chrome/common/extensions...
File chrome/common/extensions/docs/templates/articles/options2.html (right):
https://codereview.chromium.org/691283003/diff/60001/chrome/common/extensions...
chrome/common/extensions/docs/templates/articles/options2.html:18: <img
src="{{static}}/images/options_ui.png" alt="Options UI screenshot">
On 2014/10/31 20:48:24, mkearney1 wrote:
> image name is options-ui.png, so image isn't showing itself.
Oops. That's so weird, works on my mac. Fixed.
https://codereview.chromium.org/691283003/diff/60001/chrome/common/extensions...
chrome/common/extensions/docs/templates/articles/options2.html:21: Note: Always
use the $(ref:storage.sync) API to persist your options. This will
On 2014/10/31 20:48:24, mkearney1 wrote:
> I'd be inclined to drop the "Note:". Just assert... "Always use...".
Done.
https://codereview.chromium.org/691283003/diff/60001/chrome/common/extensions...
chrome/common/extensions/docs/templates/articles/options2.html:56: Here is an
example options page:
On 2014/10/31 20:48:24, mkearney1 wrote:
> Slight rewrite:
>
> Here's an example, including an options page and JavaScript to persist the
> options.
Done. With or without the : at the end?
https://codereview.chromium.org/691283003/diff/60001/chrome/common/extensions...
chrome/common/extensions/docs/templates/articles/options2.html:130: behavior
that should be considered while writing them, mostly caused by not
On 2014/10/31 20:48:24, mkearney1 wrote:
> Replace 'that should be considered' with 'you should consider' for brevity
sake.
Done.
https://codereview.chromium.org/691283003/diff/60001/chrome/common/extensions...
chrome/common/extensions/docs/templates/articles/options2.html:136: Options page
code cannot assume that they are hosted inside a tab, and this may
On 2014/10/31 20:48:24, mkearney1 wrote:
> 'that they are' better as 'that it's'.
Done.
https://codereview.chromium.org/691283003/diff/60001/chrome/common/extensions...
chrome/common/extensions/docs/templates/articles/options2.html:176: If this is
an issue, we recommend that you provide fixed minimum dimensions for
On 2014/10/31 20:48:23, mkearney1 wrote:
> Can you drop 'we recommend that you' and go with 'provide' straight up?
Done.
https://codereview.chromium.org/691283003/diff/60001/chrome/common/extensions...
chrome/common/extensions/docs/templates/articles/options2.html:186: Older
version of Chrome will only recognize <code>options_page</code>,
On 2014/10/31 20:48:24, mkearney1 wrote:
> Nit: 'Older version' should be 'Older versions'.
Done.
https://codereview.chromium.org/691283003/diff/60001/chrome/common/extensions...
chrome/common/extensions/docs/templates/articles/options2.html:187: and only
open in tabs. Chrome 40 and onwards will prefer to use the
On 2014/10/31 20:48:24, mkearney1 wrote:
> 'will prefer' better as 'prefers'.
Done.
https://codereview.chromium.org/691283003/diff/60001/chrome/common/extensions...
chrome/common/extensions/docs/templates/articles/options2.html:190: You can use
this to provide separate options pages for compatibility.
On 2014/10/31 20:48:23, mkearney1 wrote:
> Do you need line 190? Seems redundant.
I think it's redundant in a way, though pointing out something that may not be
clear.
It's actually in reaction to a bug somebody filed where they wanted some way to
tell, programmatically, whether their options page was in a tab vs a dialogue.
My answer was that we shouldn't add that because it's temporary anyway; instead,
use both options_page and options_ui together and provide different options
pages.
This sentence was my attempt to express that. Maybe it's already evident in this
article anyway? Should I update somewhere in this article to imply that more
strongly, and delete this paragraph? Change this paragraph? Neither?
https://codereview.chromium.org/691283003/diff/60001/chrome/common/extensions...
chrome/common/extensions/docs/templates/articles/options2.html:222: In a future
version of Chrome any extension which specifies
On 2014/10/31 20:48:24, mkearney1 wrote:
> Nit: comma after "In a future version of Chrome".
Done.
mkearney1
lgtm... I still think it's ok to delete this sentence: "You can use this to ...
lgtm... I still think it's ok to delete this sentence: "You can use this to
provide separate options pages for compatibility." Above you say to include both
in the manifest, so no need to explain again. Also, the real issue with this
sentence is what does 'this' stand for. If you wanted to keep in the sentence,
just explicitly state 'this'.
not at google - send to devlin
On 2014/10/31 22:10:16, mkearney1 wrote: > lgtm... I still think it's ok to delete this ...
On 2014/10/31 22:10:16, mkearney1 wrote:
> lgtm... I still think it's ok to delete this sentence: "You can use this to
> provide separate options pages for compatibility." Above you say to include
both
> in the manifest, so no need to explain again. Also, the real issue with this
> sentence is what does 'this' stand for. If you wanted to keep in the sentence,
> just explicitly state 'this'.
Thanks - I'll just delete the sentence.
Issue 691283003: Write docs for options_ui
(Closed)
Created 6 years, 1 month ago by not at google - send to devlin
Modified 6 years, 1 month ago
Reviewers: mkearney1, Ken Rockot(use gerrit already)
Base URL: https://chromium.googlesource.com/chromium/src.git@inline-type
Comments: 20
Chromium Code Reviews
Issue
691283003:
Write docs for options_ui (Closed)
