|
|
Created:
6 years, 9 months ago by paulmeyer Modified:
6 years, 9 months ago CC:
chromium-reviews, chromium-apps-reviews_chromium.org, extensions-reviews_chromium.org Base URL:
svn://svn.chromium.org/chrome/trunk/src Visibility:
Public. |
DescriptionAdded documentation about the find API for https://developer.chrome.com/apps/tags/webview.
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=255977
Patch Set 1 #
Total comments: 16
Patch Set 2 : Addressed comments. #Patch Set 3 : Added a few more <code> tags. #Messages
Total messages: 19 (0 generated)
LGTM. Instructions on how to update the documentation would be nice to have in the Noogler ramp up doc!
On 2014/03/07 15:00:39, Fady Samuel wrote: > LGTM. Instructions on how to update the documentation would be nice to have in > the Noogler ramp up doc! Okay, good call, I'll add that.
+kalman for webview_tag.json
lgtm. comments are all in a similar vein, if I missed some then please fix those too. https://codereview.chromium.org/188743005/diff/1/chrome/common/extensions/api... File chrome/common/extensions/api/webview_tag.json (right): https://codereview.chromium.org/188743005/diff/1/chrome/common/extensions/api... chrome/common/extensions/api/webview_tag.json:116: "description": "The number of times |searchText| was matched on the page." use <code>searchText</code> not |searchText|, it renders better. https://codereview.chromium.org/188743005/diff/1/chrome/common/extensions/api... chrome/common/extensions/api/webview_tag.json:139: "description": "Flag to find matches in reverse order.", Describe the default value. https://codereview.chromium.org/188743005/diff/1/chrome/common/extensions/api... chrome/common/extensions/api/webview_tag.json:144: "description": "Flag to match with case-sensitivity.", Describe the default value. https://codereview.chromium.org/188743005/diff/1/chrome/common/extensions/api... chrome/common/extensions/api/webview_tag.json:266: "description": "<p>Describes a rectangle in screen coordinates.</p><p>The containment semantics are array-like; that is, the coordinate (left, top) is considered to be contained by the rectangle, but the coordinate (left + width, top) is not.</p>", I suggest putting the mathematical things in <code> like <code>(left, top)</code>. https://codereview.chromium.org/188743005/diff/1/chrome/common/extensions/api... chrome/common/extensions/api/webview_tag.json:388: "optional": true Describe when this doesn't exist (you've said it's optional) https://codereview.chromium.org/188743005/diff/1/chrome/common/extensions/api... chrome/common/extensions/api/webview_tag.json:482: "description": "Determines what to do with the active match after the find session has ended. 'clear' will clear the highlighting over the active match; 'keep' will keep the active match highlighted; 'activate' will keep the active match highlighted and simulate a user click on that match. The default action is 'keep'.", say <code>clear</code>, <code>activate</code>, etc. https://codereview.chromium.org/188743005/diff/1/chrome/common/extensions/api... chrome/common/extensions/api/webview_tag.json:589: "description": "The number of matches found for |searchText| on the page so far." <code> https://codereview.chromium.org/188743005/diff/1/chrome/common/extensions/api... chrome/common/extensions/api/webview_tag.json:609: "description": "Indicates that all find requests have completed and that no more |findupdate| events will be fired until more find requests are made." <code>"findupdate"</code>
Addressed comments. https://codereview.chromium.org/188743005/diff/1/chrome/common/extensions/api... File chrome/common/extensions/api/webview_tag.json (right): https://codereview.chromium.org/188743005/diff/1/chrome/common/extensions/api... chrome/common/extensions/api/webview_tag.json:116: "description": "The number of times |searchText| was matched on the page." On 2014/03/07 17:40:45, kalman wrote: > use <code>searchText</code> not |searchText|, it renders better. Done. https://codereview.chromium.org/188743005/diff/1/chrome/common/extensions/api... chrome/common/extensions/api/webview_tag.json:139: "description": "Flag to find matches in reverse order.", On 2014/03/07 17:40:45, kalman wrote: > Describe the default value. Done. https://codereview.chromium.org/188743005/diff/1/chrome/common/extensions/api... chrome/common/extensions/api/webview_tag.json:144: "description": "Flag to match with case-sensitivity.", On 2014/03/07 17:40:45, kalman wrote: > Describe the default value. Done. https://codereview.chromium.org/188743005/diff/1/chrome/common/extensions/api... chrome/common/extensions/api/webview_tag.json:266: "description": "<p>Describes a rectangle in screen coordinates.</p><p>The containment semantics are array-like; that is, the coordinate (left, top) is considered to be contained by the rectangle, but the coordinate (left + width, top) is not.</p>", On 2014/03/07 17:40:45, kalman wrote: > I suggest putting the mathematical things in <code> like <code>(left, > top)</code>. Done. https://codereview.chromium.org/188743005/diff/1/chrome/common/extensions/api... chrome/common/extensions/api/webview_tag.json:388: "optional": true On 2014/03/07 17:40:45, kalman wrote: > Describe when this doesn't exist (you've said it's optional) Done. https://codereview.chromium.org/188743005/diff/1/chrome/common/extensions/api... chrome/common/extensions/api/webview_tag.json:482: "description": "Determines what to do with the active match after the find session has ended. 'clear' will clear the highlighting over the active match; 'keep' will keep the active match highlighted; 'activate' will keep the active match highlighted and simulate a user click on that match. The default action is 'keep'.", On 2014/03/07 17:40:45, kalman wrote: > say <code>clear</code>, <code>activate</code>, etc. Done. https://codereview.chromium.org/188743005/diff/1/chrome/common/extensions/api... chrome/common/extensions/api/webview_tag.json:589: "description": "The number of matches found for |searchText| on the page so far." On 2014/03/07 17:40:45, kalman wrote: > <code> Done. https://codereview.chromium.org/188743005/diff/1/chrome/common/extensions/api... chrome/common/extensions/api/webview_tag.json:609: "description": "Indicates that all find requests have completed and that no more |findupdate| events will be fired until more find requests are made." On 2014/03/07 17:40:45, kalman wrote: > <code>"findupdate"</code> Done.
The CQ bit was checked by paulmeyer@chromium.org
The CQ bit was unchecked by paulmeyer@chromium.org
The CQ bit was checked by paulmeyer@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/paulmeyer@chromium.org/188743005/30001
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/paulmeyer@chromium.org/188743005/30001
The CQ bit was unchecked by commit-bot@chromium.org
Retried try job too often on mac_rel for step(s) telemetry_perf_unittests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...
The CQ bit was checked by paulmeyer@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/paulmeyer@chromium.org/188743005/30001
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/paulmeyer@chromium.org/188743005/30001
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/paulmeyer@chromium.org/188743005/30001
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/paulmeyer@chromium.org/188743005/30001
Message was sent while issue was closed.
Change committed as 255977 |