Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
12221067:
Move Chrome Apps Codelab docs to developer.chrome.com (Closed)
|
Created:
7 years, 10 months ago by Renato Mangini (chromium) Modified:
7 years, 9 months ago Base URL:
svn://svn.chromium.org/chrome/trunk/src Visibility:
Public. |
DescriptionMove Chrome Apps Codelab docs to developer.chrome.com
This is a first HTML draft of the Codelab Markdown files, created by a conversion script. These pages are unreachable from sidebar navigation menu on purpose.
BUG=174899
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=184126
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=185028
Patch Set 1 #Patch Set 2 : fixed references to github, accordingly to the new github angularjs subdirectories; added a link to… #
Total comments: 72
Patch Set 3 : Addressed reviewer's comments #Patch Set 4 : addressed last reviewer comments on codelab #Patch Set 5 : Fixed a few more issues from a pass over all docs #
Total comments: 4
Patch Set 6 : addressed mike's comments #Patch Set 7 : Changed links to the new format of "$ref:" #
Total comments: 6
Patch Set 8 : Fixing codelab images #Patch Set 9 : Addressed Fang Jue comments #Patch Set 10 : Addressed comments from Fang Jue #Messages
Total messages: 22 (0 generated)
Hey, Renato These comments are super-easy to fix. I didn't dwell on the big stuff, as I will work on those fixes once we get the lab out there. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... File chrome/common/extensions/docs/templates/articles/app_codelab.html (right): https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab.html:12: Here's a <a href="https://docs.google.com/a/google.com/forms/d/1x3309vpp-KTiHqZWOCQhjVrIWxkm0wEBp2IWMG2ywbU/viewform?id=1x3309vpp-KTiHqZWOCQhjVrIWxkm0wEBp2IWMG2ywbU">very simple feedback form</a>.</p> replace "Here's a very simple feedback form" with: "Feedback is welcome at <a href="http://groups.google.com/a/chromium.org/group/chromium-apps">#chromium-apps</a>. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab.html:30: <p>Once you've comleted the tutorial, Spelling: comleted should be completed. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... File chrome/common/extensions/docs/templates/articles/app_codelab2_basic.html (right): https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab2_basic.html:13: <p>Lets look at each of these components at their simplest level. </p> "Lets" should be "Let's". Missing '. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab2_basic.html:72: <h2 id="debug_fix_and_reload_app_">Debug, fix, and reload app.</h2> Remove full-stop from header. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab2_basic.html:83: <li><p>Change the main.js background script to create two windows instead of one. Don't bother to create another html. For now, you can open index.html on both.</p></li> Link to main.js file here (a second time). https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... File chrome/common/extensions/docs/templates/articles/app_codelab3_mvc.html (right): https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab3_mvc.html:14: <li><p><a href="http://developer.chrome.com/apps/angular_framework.html">Build Apps with AngularJS</a> tutorial</p></li> This link is broken. You probably just need #angular_framework.html. Good idea to test it though. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab3_mvc.html:141: <li><p>Chrome apps are offline first, so the recommended way to include third-party scripts is to download and package them inside your app.</p></li> Make "offline first" a link to #offline_apps.html. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab3_mvc.html:149: Right now this lab is a work-in-progress. <a href="https://docs.google.com/a/google.com/forms/d/1x3309vpp-KTiHqZWOCQhjVrIWxkm0wEBp2IWMG2ywbU/viewform?id=1x3309vpp-KTiHqZWOCQhjVrIWxkm0wEBp2IWMG2ywbU">Please let us know what you need to test your apps</a>. Remove sentence on feedback. You might want to also remove 'then' in sentence "You can then skip..." https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... File chrome/common/extensions/docs/templates/articles/app_codelab4_testing.html (right): https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab4_testing.html:3: <p>We don't have a great testing story at the moment. Please let us know what you need to test your apps. Need to simplify this so there isn't a link to the feedback form and maybe make it a bit friendlier. Something like: "We're working on a better testing story... stay-tuned." https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... File chrome/common/extensions/docs/templates/articles/app_codelab5_data.html (right): https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab5_data.html:3: <p>The sample from Lab 3 uses a static array of Todos. Every time your app restarts, whatever you've changed is lost. In this section, we will save every change using <a href="http://developer.chrome.com/trunk/apps/storage.html">chrome.storage.sync</a>. This lets you store <em>small things</em> that automatically sync to the cloud if you are online and logged in to Chrome. If you are offline or unlogged, it saves locally and transparently: you don't have to handle online check and offline fallback in your application.</p> Link to Lab 3 doc here. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab5_data.html:45: </pre></li><li>In the HTML, call save() whenever the data changes. There are many other ways of doing this in Angular, like using $watchers on the scope. The one used here makes the save() calls explicit. save() * 2 and $watchers should be wrapped in <code> https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab5_data.html:170: <li><p>Use chrome.storage.sync to save small data that you need to be sync'ed among devices, like configuration options, application state, etc. The sync is automatic, as long as the same user is logged into Chrome on all devices.</p></li> This should link to the storage API. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab5_data.html:171: <li><p>Chrome apps support almost all HTML5 APIs, such as drag and drop. HTML Filesystem API is also supported, with extra features from the Chrome app's Filesystem API extension, like asking the user to pick files on her local disk for read and write. The vanilla HTML5 Filesystem API only allows access to a sandboxed filesystem.</p></li> This should link to the Chrome fileSystem API. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... File chrome/common/extensions/docs/templates/articles/app_codelab6_lifecycle.html (right): https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab6_lifecycle.html:19: <p>onLaunched is the most important event. It fires when the user clicks on your app's icon with the intent of launching it. For most simpler apps, the event page will listen for this event and open a window when it fires. See our <code>main.js</code> for the most common usage.</p> This should be a link to the onLaunched API. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab6_lifecycle.html:19: <p>onLaunched is the most important event. It fires when the user clicks on your app's icon with the intent of launching it. For most simpler apps, the event page will listen for this event and open a window when it fires. See our <code>main.js</code> for the most common usage.</p> This should really link to main.js https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab6_lifecycle.html:38: <p>The onRestarted event is not as essential as onLaunched, but it might be relevant to certain types of apps. This should link to the onRestarted and onLaunched API docs. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab6_lifecycle.html:46: <p>Let's change our code to save the Todo input field in <a href="http://developer.chrome.com/trunk/apps/storage.html">chrome.storage.local</a> as the user types, only restoring it if the onRestarted event is triggered.</p> Wrap onRestarted in <code> https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab6_lifecycle.html:48: <p class="note"><b>Note:</b> We learned about chrome.storage.sync before, but chrome.storage.local wasn't mentioned until now. Both have exactly the same syntax, but the semantics of chrome.storage.local is, as the name says, completely local. Link to storage API. Also wrap API in <code>. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab6_lifecycle.html:111: <p>If Chrome and the app shuts down for any reason (other than a user-gesture), the onRestarted event is fired. onRestarted should be in <code>. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab6_lifecycle.html:122: </ul> The font looks a bit small here. Any idea why? https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... File chrome/common/extensions/docs/templates/articles/app_codelab7_useridentification.html (right): https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab7_useridentification.html:15: <p class="note"><b>Note:</b> Apps with authentication require the experimental permission in the manifest.json and, until they came out of experimental state, they cannot be uploaded to the Chrome Web Store. Put manifest.json in <code>. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab7_useridentification.html:22: <h2 id="integrating_with_a_thrid_party_service">Integrating with a thrid-party service</h2> Spelling: "thrid" should be "third". https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... File chrome/common/extensions/docs/templates/articles/app_codelab8_webresources.html (right): https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab8_webresources.html:38: <a ng-show="todo.uri" href="" ng-click="showUri(todo.uri)">(view url)</a></p> The (view url) is coming up as a link, and I don't think it is suppose to be. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab8_webresources.html:68: <p>If you try to add an <code><img></code> tag to your index.html, and point its <code>src</code> attribute to any site on the web, the following exception is thrown in the console and the image isn't loaded:</p> Put index.html in <code>. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab8_webresources.html:123: <li><p>In the controller.js, drop() method, change the handling of URIs to appropriately detect a valid image. For simplicity sake, we only tested for png and jpg extensions. Feel free to have a better coverage in your code. Wrap in <code>. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab8_webresources.html:166: <p>The loadImage() method above is not the best solution for this problem, because it doesn't handle errors correctly and it could cache images in a local filesystem. Put method in <code>. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... File chrome/common/extensions/docs/templates/articles/app_codelab9_multipleviews.html (right): https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab9_multipleviews.html:31: <li><p>In the existing controller.js, add the call to create a new window: Wrap controller.js in <code>. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab9_multipleviews.html:41: <p>And that's all. Thanks to the hierarchical scope support on Angular, the DropCtrl controller is a child of the TodoCtrl and inherits all the context of its parent.</p> Wrap controllers in <code>. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... File chrome/common/extensions/docs/templates/articles/app_codelab_10_publishing.html (right): https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab_10_publishing.html:3: <p>An app without users is just a piece of code. To get users, you need to distribute your application. General comment: images on this page are huge! Can we get them compressed? https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab_10_publishing.html:26: <p>The functionality to upload new-style Chrome Apps has only just been enabled for developers. Remove feedback sentence. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab_10_publishing.html:32: <li> Compress your applications root directory (the folder containing the manifest.json file).</li> Wrap manifest.json in <code>. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab_10_publishing.html:42: Just make sure it contains the manifest.json.</p> Wrap manifest.json in <code>.
Addressed comments from Meggin. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... File chrome/common/extensions/docs/templates/articles/app_codelab.html (right): https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab.html:12: Here's a <a href="https://docs.google.com/a/google.com/forms/d/1x3309vpp-KTiHqZWOCQhjVrIWxkm0wEBp2IWMG2ywbU/viewform?id=1x3309vpp-KTiHqZWOCQhjVrIWxkm0wEBp2IWMG2ywbU">very simple feedback form</a>.</p> On 2013/02/12 22:22:11, mkearney1 wrote: > replace "Here's a very simple feedback form" with: > > "Feedback is welcome at <a > href="http://groups.google.com/a/chromium.org/group/chromium-apps">#chromium-apps</a>. Done. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab.html:30: <p>Once you've comleted the tutorial, On 2013/02/12 22:22:11, mkearney1 wrote: > Spelling: comleted should be completed. Done. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... File chrome/common/extensions/docs/templates/articles/app_codelab2_basic.html (right): https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab2_basic.html:13: <p>Lets look at each of these components at their simplest level. </p> On 2013/02/12 22:22:11, mkearney1 wrote: > "Lets" should be "Let's". Missing '. Done. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab2_basic.html:72: <h2 id="debug_fix_and_reload_app_">Debug, fix, and reload app.</h2> On 2013/02/12 22:22:11, mkearney1 wrote: > Remove full-stop from header. Done. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab2_basic.html:83: <li><p>Change the main.js background script to create two windows instead of one. Don't bother to create another html. For now, you can open index.html on both.</p></li> On 2013/02/12 22:22:11, mkearney1 wrote: > Link to main.js file here (a second time). Done. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... File chrome/common/extensions/docs/templates/articles/app_codelab3_mvc.html (right): https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab3_mvc.html:14: <li><p><a href="http://developer.chrome.com/apps/angular_framework.html">Build Apps with AngularJS</a> tutorial</p></li> On 2013/02/12 22:22:11, mkearney1 wrote: > This link is broken. You probably just need #angular_framework.html. Good idea > to test it though. Done. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab3_mvc.html:141: <li><p>Chrome apps are offline first, so the recommended way to include third-party scripts is to download and package them inside your app.</p></li> On 2013/02/12 22:22:11, mkearney1 wrote: > Make "offline first" a link to #offline_apps.html. Done. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab3_mvc.html:149: Right now this lab is a work-in-progress. <a href="https://docs.google.com/a/google.com/forms/d/1x3309vpp-KTiHqZWOCQhjVrIWxkm0wEBp2IWMG2ywbU/viewform?id=1x3309vpp-KTiHqZWOCQhjVrIWxkm0wEBp2IWMG2ywbU">Please let us know what you need to test your apps</a>. On 2013/02/12 22:22:11, mkearney1 wrote: > Remove sentence on feedback. You might want to also remove 'then' in sentence > "You can then skip..." Done. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... File chrome/common/extensions/docs/templates/articles/app_codelab5_data.html (right): https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab5_data.html:3: <p>The sample from Lab 3 uses a static array of Todos. Every time your app restarts, whatever you've changed is lost. In this section, we will save every change using <a href="http://developer.chrome.com/trunk/apps/storage.html">chrome.storage.sync</a>. This lets you store <em>small things</em> that automatically sync to the cloud if you are online and logged in to Chrome. If you are offline or unlogged, it saves locally and transparently: you don't have to handle online check and offline fallback in your application.</p> On 2013/02/12 22:22:11, mkearney1 wrote: > Link to Lab 3 doc here. Done. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab5_data.html:45: </pre></li><li>In the HTML, call save() whenever the data changes. There are many other ways of doing this in Angular, like using $watchers on the scope. The one used here makes the save() calls explicit. On 2013/02/12 22:22:11, mkearney1 wrote: > save() * 2 and $watchers should be wrapped in <code> Done. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab5_data.html:170: <li><p>Use chrome.storage.sync to save small data that you need to be sync'ed among devices, like configuration options, application state, etc. The sync is automatic, as long as the same user is logged into Chrome on all devices.</p></li> On 2013/02/12 22:22:11, mkearney1 wrote: > This should link to the storage API. Done. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab5_data.html:171: <li><p>Chrome apps support almost all HTML5 APIs, such as drag and drop. HTML Filesystem API is also supported, with extra features from the Chrome app's Filesystem API extension, like asking the user to pick files on her local disk for read and write. The vanilla HTML5 Filesystem API only allows access to a sandboxed filesystem.</p></li> On 2013/02/12 22:22:11, mkearney1 wrote: > This should link to the Chrome fileSystem API. Done. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... File chrome/common/extensions/docs/templates/articles/app_codelab6_lifecycle.html (right): https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab6_lifecycle.html:19: <p>onLaunched is the most important event. It fires when the user clicks on your app's icon with the intent of launching it. For most simpler apps, the event page will listen for this event and open a window when it fires. See our <code>main.js</code> for the most common usage.</p> On 2013/02/12 22:22:11, mkearney1 wrote: > This should be a link to the onLaunched API. Done. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab6_lifecycle.html:19: <p>onLaunched is the most important event. It fires when the user clicks on your app's icon with the intent of launching it. For most simpler apps, the event page will listen for this event and open a window when it fires. See our <code>main.js</code> for the most common usage.</p> On 2013/02/12 22:22:11, mkearney1 wrote: > This should be a link to the onLaunched API. Done. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab6_lifecycle.html:38: <p>The onRestarted event is not as essential as onLaunched, but it might be relevant to certain types of apps. On 2013/02/12 22:22:11, mkearney1 wrote: > This should link to the onRestarted and onLaunched API docs. Where are these docs? For onLaunched, I've linked to the lifecycle doc. But I can't figure where onRestarted is documented. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab6_lifecycle.html:46: <p>Let's change our code to save the Todo input field in <a href="http://developer.chrome.com/trunk/apps/storage.html">chrome.storage.local</a> as the user types, only restoring it if the onRestarted event is triggered.</p> On 2013/02/12 22:22:11, mkearney1 wrote: > Wrap onRestarted in <code> Done. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab6_lifecycle.html:48: <p class="note"><b>Note:</b> We learned about chrome.storage.sync before, but chrome.storage.local wasn't mentioned until now. Both have exactly the same syntax, but the semantics of chrome.storage.local is, as the name says, completely local. On 2013/02/12 22:22:11, mkearney1 wrote: > Link to storage API. Also wrap API in <code>. Done. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab6_lifecycle.html:111: <p>If Chrome and the app shuts down for any reason (other than a user-gesture), the onRestarted event is fired. On 2013/02/12 22:22:11, mkearney1 wrote: > onRestarted should be in <code>. Done. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab6_lifecycle.html:122: </ul> On 2013/02/12 22:22:11, mkearney1 wrote: > The font looks a bit small here. Any idea why? Done. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... File chrome/common/extensions/docs/templates/articles/app_codelab7_useridentification.html (right): https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab7_useridentification.html:15: <p class="note"><b>Note:</b> Apps with authentication require the experimental permission in the manifest.json and, until they came out of experimental state, they cannot be uploaded to the Chrome Web Store. On 2013/02/12 22:22:11, mkearney1 wrote: > Put manifest.json in <code>. Done. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab7_useridentification.html:22: <h2 id="integrating_with_a_thrid_party_service">Integrating with a thrid-party service</h2> On 2013/02/12 22:22:11, mkearney1 wrote: > Spelling: "thrid" should be "third". Done. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... File chrome/common/extensions/docs/templates/articles/app_codelab8_webresources.html (right): https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab8_webresources.html:38: <a ng-show="todo.uri" href="" ng-click="showUri(todo.uri)">(view url)</a></p> On 2013/02/12 22:22:11, mkearney1 wrote: > The (view url) is coming up as a link, and I don't think it is suppose to be. Done. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab8_webresources.html:68: <p>If you try to add an <code><img></code> tag to your index.html, and point its <code>src</code> attribute to any site on the web, the following exception is thrown in the console and the image isn't loaded:</p> On 2013/02/12 22:22:11, mkearney1 wrote: > Put index.html in <code>. Done. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab8_webresources.html:123: <li><p>In the controller.js, drop() method, change the handling of URIs to appropriately detect a valid image. For simplicity sake, we only tested for png and jpg extensions. Feel free to have a better coverage in your code. On 2013/02/12 22:22:11, mkearney1 wrote: > Wrap in <code>. Done. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab8_webresources.html:166: <p>The loadImage() method above is not the best solution for this problem, because it doesn't handle errors correctly and it could cache images in a local filesystem. On 2013/02/12 22:22:11, mkearney1 wrote: > Put method in <code>. Done. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... File chrome/common/extensions/docs/templates/articles/app_codelab9_multipleviews.html (right): https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab9_multipleviews.html:31: <li><p>In the existing controller.js, add the call to create a new window: On 2013/02/12 22:22:11, mkearney1 wrote: > Wrap controller.js in <code>. Done. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab9_multipleviews.html:41: <p>And that's all. Thanks to the hierarchical scope support on Angular, the DropCtrl controller is a child of the TodoCtrl and inherits all the context of its parent.</p> On 2013/02/12 22:22:11, mkearney1 wrote: > Wrap controllers in <code>. Done. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab9_multipleviews.html:41: <p>And that's all. Thanks to the hierarchical scope support on Angular, the DropCtrl controller is a child of the TodoCtrl and inherits all the context of its parent.</p> On 2013/02/12 22:22:11, mkearney1 wrote: > Wrap controllers in <code>. Done. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... File chrome/common/extensions/docs/templates/articles/app_codelab_10_publishing.html (right): https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab_10_publishing.html:3: <p>An app without users is just a piece of code. To get users, you need to distribute your application. On 2013/02/12 22:22:11, mkearney1 wrote: > General comment: images on this page are huge! Can we get them compressed? Done. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab_10_publishing.html:26: <p>The functionality to upload new-style Chrome Apps has only just been enabled for developers. On 2013/02/12 22:22:11, mkearney1 wrote: > Remove feedback sentence. Done. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab_10_publishing.html:32: <li> Compress your applications root directory (the folder containing the manifest.json file).</li> On 2013/02/12 22:22:11, mkearney1 wrote: > Wrap manifest.json in <code>. Done. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab_10_publishing.html:42: Just make sure it contains the manifest.json.</p> On 2013/02/12 22:22:11, mkearney1 wrote: > Wrap manifest.json in <code>. Done. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab_10_publishing.html:42: Just make sure it contains the manifest.json.</p> On 2013/02/12 22:22:11, mkearney1 wrote: > Wrap manifest.json in <code>. Done.
Hey, Renato Just a few little nits left. Can you send me a link to the latest version of the docs staged too... same place? Meggin https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... File chrome/common/extensions/docs/templates/articles/app_codelab2_basic.html (right): https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab2_basic.html:72: <h2 id="debug_fix_and_reload_app_">Debug, fix, and reload app.</h2> Full-stop still appearing (Debug, fix, and reload app.) Get rid of period/full-stop after 'app'. On 2013/02/13 22:55:33, mangini wrote: > On 2013/02/12 22:22:11, mkearney1 wrote: > > Remove full-stop from header. > > Done. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... File chrome/common/extensions/docs/templates/articles/app_codelab6_lifecycle.html (right): https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab6_lifecycle.html:38: <p>The onRestarted event is not as essential as onLaunched, but it might be relevant to certain types of apps. There in app.runtime api: http://developer.chrome.com/apps/app.runtime.html On 2013/02/13 22:55:33, mangini wrote: > On 2013/02/12 22:22:11, mkearney1 wrote: > > This should link to the onRestarted and onLaunched API docs. > > Where are these docs? For onLaunched, I've linked to the lifecycle doc. But I > can't figure where onRestarted is documented. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... File chrome/common/extensions/docs/templates/articles/app_codelab_10_publishing.html (right): https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab_10_publishing.html:42: Just make sure it contains the manifest.json.</p> Still not showing 'manifest.json' in <code>. On 2013/02/13 22:55:33, mangini wrote: > On 2013/02/12 22:22:11, mkearney1 wrote: > > Wrap manifest.json in <code>. > > Done.
Ok, now I think I've addressed all your comments :-) Thanks, Meggin! https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... File chrome/common/extensions/docs/templates/articles/app_codelab2_basic.html (right): https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab2_basic.html:72: <h2 id="debug_fix_and_reload_app_">Debug, fix, and reload app.</h2> sorry, this got lost when I converted back from (.md). Done now. On 2013/02/13 23:17:57, mkearney1 wrote: > Full-stop still appearing (Debug, fix, and reload app.) Get rid of > period/full-stop after 'app'. > On 2013/02/13 22:55:33, mangini wrote: > > On 2013/02/12 22:22:11, mkearney1 wrote: > > > Remove full-stop from header. > > > > Done. > https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... File chrome/common/extensions/docs/templates/articles/app_codelab4_testing.html (right): https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab4_testing.html:3: <p>We don't have a great testing story at the moment. Please let us know what you need to test your apps. On 2013/02/12 22:22:11, mkearney1 wrote: > Need to simplify this so there isn't a link to the feedback form and maybe make > it a bit friendlier. Something like: > > "We're working on a better testing story... stay-tuned." Done. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... File chrome/common/extensions/docs/templates/articles/app_codelab6_lifecycle.html (right): https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab6_lifecycle.html:38: <p>The onRestarted event is not as essential as onLaunched, but it might be relevant to certain types of apps. On 2013/02/13 23:17:57, mkearney1 wrote: > There in app.runtime api: http://developer.chrome.com/apps/app.runtime.html > > On 2013/02/13 22:55:33, mangini wrote: > > On 2013/02/12 22:22:11, mkearney1 wrote: > > > This should link to the onRestarted and onLaunched API docs. > > > > Where are these docs? For onLaunched, I've linked to the lifecycle doc. But I > > can't figure where onRestarted is documented. > Done. https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... File chrome/common/extensions/docs/templates/articles/app_codelab_10_publishing.html (right): https://chromiumcodereview.appspot.com/12221067/diff/1030/chrome/common/exten... chrome/common/extensions/docs/templates/articles/app_codelab_10_publishing.html:42: Just make sure it contains the manifest.json.</p> On 2013/02/13 23:17:57, mkearney1 wrote: > Still not showing 'manifest.json' in <code>. > > On 2013/02/13 22:55:33, mangini wrote: > > On 2013/02/12 22:22:11, mkearney1 wrote: > > > Wrap manifest.json in <code>. > > > > Done. > Done.
lgtm
No LGTM from a valid reviewer yet. Only full committers are accepted. Even if an LGTM may have been provided, it was from a non-committer or a lowly provisional committer, _not_ a full super star committer. See http://www.chromium.org/getting-involved/become-a-committer Note that this has nothing to do with OWNERS files.
No LGTM from a valid reviewer yet. Only full committers are accepted. Even if an LGTM may have been provided, it was from a non-committer or a lowly provisional committer, _not_ a full super star committer. See http://www.chromium.org/getting-involved/become-a-committer Note that this has nothing to do with OWNERS files.
No LGTM from a valid reviewer yet. Only full committers are accepted. Even if an LGTM may have been provided, it was from a non-committer or a lowly provisional committer, _not_ a full super star committer. See http://www.chromium.org/getting-involved/become-a-committer Note that this has nothing to do with OWNERS files.
Mike, can you review and lgtm, please? Meggin couldn't commit for some strange reason (she usually can). Thanks! On 2013/02/14 17:45:06, I haz the power (commit-bot) wrote: > No LGTM from a valid reviewer yet. Only full committers are accepted. > Even if an LGTM may have been provided, it was from a non-committer or > a lowly provisional committer, _not_ a full super star committer. > See http://www.chromium.org/getting-involved/become-a-committer > Note that this has nothing to do with OWNERS files.
Rubber-stamp LGTM https://chromiumcodereview.appspot.com/12221067/diff/21006/chrome/common/exte... File chrome/common/extensions/docs/templates/articles/app_codelab1_setup.html (right): https://chromiumcodereview.appspot.com/12221067/diff/21006/chrome/common/exte... chrome/common/extensions/docs/templates/articles/app_codelab1_setup.html:25: <p class="note"><b>Note:</b> Open the <code>chrome://extensions</code> page in it's own window and keep it assessible as you work through the lab. its not it's https://chromiumcodereview.appspot.com/12221067/diff/21006/chrome/common/exte... File chrome/common/extensions/docs/templates/articles/app_codelab2_basic.html (right): https://chromiumcodereview.appspot.com/12221067/diff/21006/chrome/common/exte... chrome/common/extensions/docs/templates/articles/app_codelab2_basic.html:8: <li>The manifest that descibes meta-information about your applicaiton: name, description, version number and how to launch your app</li> application
addressed minor comments. https://chromiumcodereview.appspot.com/12221067/diff/21006/chrome/common/exte... File chrome/common/extensions/docs/templates/articles/app_codelab1_setup.html (right): https://chromiumcodereview.appspot.com/12221067/diff/21006/chrome/common/exte... chrome/common/extensions/docs/templates/articles/app_codelab1_setup.html:25: <p class="note"><b>Note:</b> Open the <code>chrome://extensions</code> page in it's own window and keep it assessible as you work through the lab. On 2013/02/14 21:49:50, miket wrote: > its not it's Done. https://chromiumcodereview.appspot.com/12221067/diff/21006/chrome/common/exte... File chrome/common/extensions/docs/templates/articles/app_codelab2_basic.html (right): https://chromiumcodereview.appspot.com/12221067/diff/21006/chrome/common/exte... chrome/common/extensions/docs/templates/articles/app_codelab2_basic.html:8: <li>The manifest that descibes meta-information about your applicaiton: name, description, version number and how to launch your app</li> On 2013/02/14 21:49:50, miket wrote: > application Done.
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mangini@chromium.org/12221067/27001
Presubmit check for 12221067-27001 failed and returned exit status 1. INFO:root:Found 28 file(s). Running presubmit commit checks ... Running /b/commit-queue/workdir/chromium/PRESUBMIT.py Running /b/commit-queue/workdir/chromium/chrome/PRESUBMIT.py Running /b/commit-queue/workdir/chromium/chrome/common/extensions/PRESUBMIT.py ** Presubmit Warnings ** File chrome/common/extensions/docs/templates/articles/app_codelab6_lifecycle.html may have an old-style <a> link to an API page. Please run docs/server2/link_converter.py to convert the link[s], or convert them manually. Suggested changes are: Line 15: -There are two important lifecycle events, <a href="http://developer.chrome.com/trunk/apps/app.runtime.html#event-onLaunched">onLaunched</a> and <a href="http://developer.chrome.com/trunk/apps/app.runtime.html#event-onRestarted">onRestarted</a>.</p> +There are two important lifecycle events, $ref:http:developer.chrome.comtrunkappsapp.runtime.onLaunched and $ref:http:developer.chrome.comtrunkappsapp.runtime.onRestarted.</p> Line 19: -<p><a href="http://developer.chrome.com/apps/app.runtime.html#event-onLaunched">onLaunched</a> is the most important event. It fires when the user clicks on your app's icon with the intent of launching it. For most simpler apps, the event page will listen for this event and open a window when it fires. See our <a href="https://github.com/GoogleChrome/chrome-app-codelab/blob/master/lab6_lifecycle/angularjs/main.js">main.js</a> for the most common usage.</p> +<p>$ref:http:developer.chrome.comappsapp.runtime.onLaunched is the most important event. It fires when the user clicks on your app's icon with the intent of launching it. For most simpler apps, the event page will listen for this event and open a window when it fires. See our <a href="https://github.com/GoogleChrome/chrome-app-codelab/blob/master/lab6_lifecycle/angularjs/main.js">main.js</a> for the most common usage.</p> Line 23: -<p>The <a href="http://developer.chrome.com/trunk/apps/app.window.html#method-create">chrome.app.window.create</a> method can associate an ID to the window being opened. Currently, the most interesting use for this is to restore a window's width, height and location and its associated Developer Tools window, if opened, when the app is launched. </p> +<p>The $ref:http:developer.chrome.comtrunkappsapp.window.create method can associate an ID to the window being opened. Currently, the most interesting use for this is to restore a window's width, height and location and its associated Developer Tools window, if opened, when the app is launched. </p> Line 38: -The <a href="http://developer.chrome.com/apps/app.runtime.html#event-onRestarted">onRestarted</a> event is not as essential as <code>onLaunched</code>, but it might be relevant to certain types of apps. +The $ref:http:developer.chrome.comappsapp.runtime.onRestarted event is not as essential as <code>onLaunched</code>, but it might be relevant to certain types of apps. File chrome/common/extensions/docs/templates/articles/app_codelab7_useridentification.html may have an old-style <a> link to an API page. Please run docs/server2/link_converter.py to convert the link[s], or convert them manually. Suggested changes are: Line 24: -<p>Chrome apps have a dedicated API for lauching the authentication flow to any third-party OAuth2 service, called <a href="http://developer.chrome.com/trunk/apps/experimental.identity.html#method-launchWebAuthFlow">launchWebAuthFlow</a>. +<p>Chrome apps have a dedicated API for lauching the authentication flow to any third-party OAuth2 service, called $ref:http:developer.chrome.comtrunkappsexperimental.identity.launchWebAuthFlow. Presubmit checks took 8.2s to calculate.
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mangini@chromium.org/12221067/27003
Message was sent while issue was closed.
Change committed as 184126
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mangini@chromium.org/12221067/34007
Retried try job too often on mac_rel for step(s) interactive_ui_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mangini@chromium.org/12221067/34007
Hi Renato, Here's some problems with these docs I found recently. https://chromiumcodereview.appspot.com/12221067/diff/27003/chrome/common/exte... File chrome/common/extensions/docs/templates/articles/app_codelab1_setup.html (right): https://chromiumcodereview.appspot.com/12221067/diff/27003/chrome/common/exte... chrome/common/extensions/docs/templates/articles/app_codelab1_setup.html:14: <img src="/static/images/codelab/screenshot1.png" alt="Checking Chrome version"></p></li> Should be <img src="{{static}}/images/... One more at line 22 and 4 in app_codelab_10_publishing.html needs to be corrected. https://chromiumcodereview.appspot.com/12221067/diff/27003/chrome/common/exte... File chrome/common/extensions/docs/templates/articles/app_codelab6_lifecycle.html (right): https://chromiumcodereview.appspot.com/12221067/diff/27003/chrome/common/exte... chrome/common/extensions/docs/templates/articles/app_codelab6_lifecycle.html:38: The $ref:app.runtime.onRestarted event is not as essential as <code>onLaunched</code>, but it might be relevant to certain types of apps. Did you forget <p> for these paragraphs? https://chromiumcodereview.appspot.com/12221067/diff/27003/chrome/common/exte... chrome/common/extensions/docs/templates/articles/app_codelab6_lifecycle.html:115: load the unpacked <a href="app_codelab6_lifecycle.html">lab6_lifecycle</a>, and launch the app from a new tab.</p> It's linked to itself? Is it "https://github.com/GoogleChrome/chrome-app-codelab/tree/master/lab6_lifecycle/angularjs"
All comments addressed, thanks! https://chromiumcodereview.appspot.com/12221067/diff/27003/chrome/common/exte... File chrome/common/extensions/docs/templates/articles/app_codelab1_setup.html (right): https://chromiumcodereview.appspot.com/12221067/diff/27003/chrome/common/exte... chrome/common/extensions/docs/templates/articles/app_codelab1_setup.html:14: <img src="/static/images/codelab/screenshot1.png" alt="Checking Chrome version"></p></li> On 2013/02/27 01:01:38, 方觉(Fang Jue) wrote: > Should be <img src="{{static}}/images/... > One more at line 22 and 4 in app_codelab_10_publishing.html needs to be > corrected. Done. https://chromiumcodereview.appspot.com/12221067/diff/27003/chrome/common/exte... File chrome/common/extensions/docs/templates/articles/app_codelab6_lifecycle.html (right): https://chromiumcodereview.appspot.com/12221067/diff/27003/chrome/common/exte... chrome/common/extensions/docs/templates/articles/app_codelab6_lifecycle.html:38: The $ref:app.runtime.onRestarted event is not as essential as <code>onLaunched</code>, but it might be relevant to certain types of apps. On 2013/02/27 01:01:38, 方觉(Fang Jue) wrote: > Did you forget <p> for these paragraphs? Done. https://chromiumcodereview.appspot.com/12221067/diff/27003/chrome/common/exte... chrome/common/extensions/docs/templates/articles/app_codelab6_lifecycle.html:115: load the unpacked <a href="app_codelab6_lifecycle.html">lab6_lifecycle</a>, and launch the app from a new tab.</p> On 2013/02/27 01:01:38, 方觉(Fang Jue) wrote: > It's linked to itself? Is it > "https://github.com/GoogleChrome/chrome-app-codelab/tree/master/lab6_lifecycle/angularjs" Done.
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mangini@chromium.org/12221067/30022
Message was sent while issue was closed.
Change committed as 185028 |
