Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
525363003:
DevTools: [Documentation] Add signature section for renderer (Closed)
|
Created:
6 years, 3 months ago by semeny Modified:
6 years, 3 months ago CC:
blink-reviews, caseq+blink_chromium.org, loislo+blink_chromium.org, eustas+blink_chromium.org, malch+blink_chromium.org, yurys+blink_chromium.org, lushnikov+blink_chromium.org, vsevik+blink_chromium.org, pfeldman+blink_chromium.org, paulirish+reviews_chromium.org, apavlov+blink_chromium.org, devtools-reviews_chromium.org, sergeyv+blink_chromium.org, aandrey+blink_chromium.org Base URL:
https://chromium.googlesource.com/chromium/blink.git@iliia-patch Project:
blink Visibility:
Public. |
DescriptionDevTools: [Documentation] Add signature section for renderer
Drive-by fixes:
1) Fix constants documentation detection
2) Fix inherited Function methods problem
BUG=391593
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=181648
Patch Set 1 #Patch Set 2 : Change page title, move isMethod function #
Total comments: 9
Patch Set 3 : Add signature section #
Total comments: 10
Patch Set 4 : Improve documentation style #Patch Set 5 : Fix rebase problems #
Total comments: 35
Patch Set 6 : Comments addressed #
Total comments: 1
Patch Set 7 : Change signature css-class selector #
Total comments: 2
Patch Set 8 : Remove unnecessary feature #Patch Set 9 : Rebased on current version #Patch Set 10 : Remove unused css-class #Patch Set 11 : Rebased on current patch #
Messages
Total messages: 38 (16 generated)
semeny@google.com changed reviewers: + iliia@google.com, lushnikov@chromium.org
Patchset #2 (id:20001) has been deleted
Patchset #3 (id:60001) has been deleted
https://codereview.chromium.org/525363003/diff/40001/Source/devtools/front_en... File Source/devtools/front_end/documentation/DocumentationView.js (right): https://codereview.chromium.org/525363003/diff/40001/Source/devtools/front_en... Source/devtools/front_end/documentation/DocumentationView.js:25: view.element.removeChildren(); this looks like a left-over from a rebaseline, it shouldn't be needed (we removechildren somewhere deeper in view.showDocumentation) https://codereview.chromium.org/525363003/diff/40001/Source/devtools/front_en... Source/devtools/front_end/documentation/DocumentationView.js:113: function isMethod(name) wrong indent (9 chars?) https://codereview.chromium.org/525363003/diff/40001/Source/devtools/front_en... Source/devtools/front_end/documentation/DocumentationView.js:113: function isMethod(name) where do you use this method? https://codereview.chromium.org/525363003/diff/40001/Source/devtools/front_en... Source/devtools/front_end/documentation/DocumentationView.js:117: for (var i = 0; i < tokens.length; i++) { ++i https://codereview.chromium.org/525363003/diff/40001/Source/devtools/front_en... Source/devtools/front_end/documentation/DocumentationView.js:122: return (typeof(currentObject) === "function"); typeof is operator - no need for brackets
Patchset #4 (id:100001) has been deleted
Patchset #3 (id:80001) has been deleted
https://codereview.chromium.org/525363003/diff/40001/Source/devtools/front_en... File Source/devtools/front_end/documentation/DocumentationView.js (right): https://codereview.chromium.org/525363003/diff/40001/Source/devtools/front_en... Source/devtools/front_end/documentation/DocumentationView.js:25: view.element.removeChildren(); On 2014/09/04 12:51:37, lushnikov wrote: > this looks like a left-over from a rebaseline, it shouldn't be needed (we > removechildren somewhere deeper in view.showDocumentation) Done. https://codereview.chromium.org/525363003/diff/40001/Source/devtools/front_en... Source/devtools/front_end/documentation/DocumentationView.js:113: function isMethod(name) On 2014/09/04 12:51:37, lushnikov wrote: > wrong indent (9 chars?) Done. https://codereview.chromium.org/525363003/diff/40001/Source/devtools/front_en... Source/devtools/front_end/documentation/DocumentationView.js:117: for (var i = 0; i < tokens.length; i++) { On 2014/09/04 12:51:37, lushnikov wrote: > ++i Done. https://codereview.chromium.org/525363003/diff/40001/Source/devtools/front_en... Source/devtools/front_end/documentation/DocumentationView.js:122: return (typeof(currentObject) === "function"); On 2014/09/04 12:51:37, lushnikov wrote: > typeof is operator - no need for brackets Done.
apavlov@chromium.org changed reviewers: + apavlov@chromium.org
https://codereview.chromium.org/525363003/diff/120001/Source/devtools/front_e... File Source/devtools/front_end/documentation/DocumentationURLProvider.js (right): https://codereview.chromium.org/525363003/diff/120001/Source/devtools/front_e... Source/devtools/front_end/documentation/DocumentationURLProvider.js:254: * FIXME: object parameter is not annotated property due to crbug.com/407097 Typo: property->properly https://codereview.chromium.org/525363003/diff/120001/Source/devtools/front_e... File Source/devtools/front_end/documentation/DocumentationView.js (right): https://codereview.chromium.org/525363003/diff/120001/Source/devtools/front_e... Source/devtools/front_end/documentation/DocumentationView.js:103: this._createParametresSection(this._article.parameters); Parameters https://codereview.chromium.org/525363003/diff/120001/Source/devtools/front_e... Source/devtools/front_end/documentation/DocumentationView.js:164: methodName.textContent = this._searchItem.split(".").pop() + "("; pop() > peekLast() to avoid extraneous temporary array modification https://codereview.chromium.org/525363003/diff/120001/Source/devtools/front_e... Source/devtools/front_end/documentation/DocumentationView.js:167: var separator = signature.createChild("span"); Inline https://codereview.chromium.org/525363003/diff/120001/Source/devtools/front_e... Source/devtools/front_end/documentation/DocumentationView.js:178: var separator = signature.createChild("span"); Inline
https://codereview.chromium.org/525363003/diff/120001/Source/devtools/front_e... File Source/devtools/front_end/documentation/DocumentationURLProvider.js (right): https://codereview.chromium.org/525363003/diff/120001/Source/devtools/front_e... Source/devtools/front_end/documentation/DocumentationURLProvider.js:254: * FIXME: object parameter is not annotated property due to crbug.com/407097 On 2014/09/05 11:32:14, apavlov (OOO until Sept.19) wrote: > Typo: property->properly Done. https://codereview.chromium.org/525363003/diff/120001/Source/devtools/front_e... File Source/devtools/front_end/documentation/DocumentationView.js (right): https://codereview.chromium.org/525363003/diff/120001/Source/devtools/front_e... Source/devtools/front_end/documentation/DocumentationView.js:103: this._createParametresSection(this._article.parameters); On 2014/09/05 11:32:14, apavlov (OOO until Sept.19) wrote: > Parameters Done. https://codereview.chromium.org/525363003/diff/120001/Source/devtools/front_e... Source/devtools/front_end/documentation/DocumentationView.js:164: methodName.textContent = this._searchItem.split(".").pop() + "("; On 2014/09/05 11:32:14, apavlov (OOO until Sept.19) wrote: > pop() > peekLast() to avoid extraneous temporary array modification Done. https://codereview.chromium.org/525363003/diff/120001/Source/devtools/front_e... Source/devtools/front_end/documentation/DocumentationView.js:167: var separator = signature.createChild("span"); On 2014/09/05 11:32:14, apavlov (OOO until Sept.19) wrote: > Inline Done. https://codereview.chromium.org/525363003/diff/120001/Source/devtools/front_e... Source/devtools/front_end/documentation/DocumentationView.js:178: var separator = signature.createChild("span"); On 2014/09/05 11:32:14, apavlov (OOO until Sept.19) wrote: > Inline Done.
https://codereview.chromium.org/525363003/diff/160001/Source/devtools/front_e... File Source/devtools/front_end/documentation/DocumentationView.js (right): https://codereview.chromium.org/525363003/diff/160001/Source/devtools/front_e... Source/devtools/front_end/documentation/DocumentationView.js:161: var signature = section.createChild("div", "documentation-method-signature"); lets use "span" here instead of "div" and get rid of "display: inline" property in .documentation-method-signature https://codereview.chromium.org/525363003/diff/160001/Source/devtools/front_e... Source/devtools/front_end/documentation/DocumentationView.js:166: var separator = signature.createChild("inline"); there's no tag "inline". You probably would like to use signature.createTextChild(", ") instead https://codereview.chromium.org/525363003/diff/160001/Source/devtools/front_e... Source/devtools/front_end/documentation/DocumentationView.js:172: parameterType.className = "documentation-parameter-data-type-value"; You've already set this className in element creationg https://codereview.chromium.org/525363003/diff/160001/Source/devtools/front_e... Source/devtools/front_end/documentation/DocumentationView.js:176: parameterType.textContent = parameters[i].dataType + optional; parameterType.textContent = parameters[i].dataType + (parameters[i].optional ? "=" : ""); https://codereview.chromium.org/525363003/diff/160001/Source/devtools/front_e... Source/devtools/front_end/documentation/DocumentationView.js:178: var separator = signature.createChild("inline"); ditto https://codereview.chromium.org/525363003/diff/160001/Source/devtools/front_e... Source/devtools/front_end/documentation/DocumentationView.js:179: separator.textContent = ") : "; lets avoid space after ")" https://codereview.chromium.org/525363003/diff/160001/Source/devtools/front_e... File Source/devtools/front_end/documentationView.css (right): https://codereview.chromium.org/525363003/diff/160001/Source/devtools/front_e... Source/devtools/front_end/documentationView.css:40: font-family: dejavu sans mono, monospace; We never want to tweak font-family property in any style. Please, remove https://codereview.chromium.org/525363003/diff/160001/Source/devtools/front_e... Source/devtools/front_end/documentationView.css:70: font-family: monospace; the font-family property is very tricky. Instead of setting it as "monospace" here, you'd like to use "monospace" class fro the element https://codereview.chromium.org/525363003/diff/160001/Source/devtools/front_e... Source/devtools/front_end/documentationView.css:109: .documentation-color-box { The "documentation-color-box" css class doesn't help to understand which elements will be styled with this class; instead, it describes how they will look, which is not helpful as I can figure this out from its properties. Believe or not, but you don't want to extract common css properties into a separate class, if they only *happen* to be equal. https://codereview.chromium.org/525363003/diff/160001/Source/devtools/front_e... Source/devtools/front_end/documentationView.css:118: line-height: 160%; i believe you want smth like 1.5 here. We need @eustas to verify our typography here
lushnikov@chromium.org changed reviewers: + eustas@chromium.org
@eustas, could you please take a look on these styles? Do you like the way they're crafted?
Andrey's concern about the styling is valid enough. eustas will know better, but I think the class names should not have such as "documentation-". We can use ancestor selectors instead, combined with far shorter class names. https://codereview.chromium.org/525363003/diff/160001/Source/devtools/front_e... File Source/devtools/front_end/documentation/DocumentationView.js (right): https://codereview.chromium.org/525363003/diff/160001/Source/devtools/front_e... Source/devtools/front_end/documentation/DocumentationView.js:166: var separator = signature.createChild("inline"); On 2014/09/05 15:20:44, lushnikov wrote: > there's no tag "inline". You probably would like to use > signature.createTextChild(", ") instead Yes, I meant that the variable should be inlined https://codereview.chromium.org/525363003/diff/160001/Source/devtools/front_e... Source/devtools/front_end/documentation/DocumentationView.js:175: parameterType.classList.add("documentation-color-box"); This class can be provided during the element construction in createChild https://codereview.chromium.org/525363003/diff/160001/Source/devtools/front_e... Source/devtools/front_end/documentation/DocumentationView.js:197: parameter.classList.add("documentation-section-content"); Ditto https://codereview.chromium.org/525363003/diff/160001/Source/devtools/front_e... Source/devtools/front_end/documentation/DocumentationView.js:202: dataTypeValue.classList.add("documentation-color-box"); Ditto for these two classes https://codereview.chromium.org/525363003/diff/160001/Source/devtools/front_e... Source/devtools/front_end/documentation/DocumentationView.js:207: optional.classList.add("documentation-color-box"); Ditto for these two classes https://codereview.chromium.org/525363003/diff/160001/Source/devtools/front_e... Source/devtools/front_end/documentation/DocumentationView.js:231: returnValueDescription.classList.add("documentation-text"); Ditto https://codereview.chromium.org/525363003/diff/160001/Source/devtools/front_e... Source/devtools/front_end/documentation/DocumentationView.js:249: example.classList.add("documentation-section-content"); Ditto here and below https://codereview.chromium.org/525363003/diff/160001/Source/devtools/front_e... File Source/devtools/front_end/documentationView.css (right): https://codereview.chromium.org/525363003/diff/160001/Source/devtools/front_e... Source/devtools/front_end/documentationView.css:10: padding: 1em; In general, we don't use ems in our layout?
Andrey's concern about the styling is valid enough. eustas will know better, but I think the class names should not have such as "documentation-". We can use ancestor selectors instead, combined with far shorter class names. https://codereview.chromium.org/525363003/diff/160001/Source/devtools/front_e... File Source/devtools/front_end/documentation/DocumentationView.js (right): https://codereview.chromium.org/525363003/diff/160001/Source/devtools/front_e... Source/devtools/front_end/documentation/DocumentationView.js:166: var separator = signature.createChild("inline"); On 2014/09/05 15:20:44, lushnikov wrote: > there's no tag "inline". You probably would like to use > signature.createTextChild(", ") instead Yes, I meant that the variable should be inlined https://codereview.chromium.org/525363003/diff/160001/Source/devtools/front_e... Source/devtools/front_end/documentation/DocumentationView.js:175: parameterType.classList.add("documentation-color-box"); This class can be provided during the element construction in createChild https://codereview.chromium.org/525363003/diff/160001/Source/devtools/front_e... Source/devtools/front_end/documentation/DocumentationView.js:197: parameter.classList.add("documentation-section-content"); Ditto https://codereview.chromium.org/525363003/diff/160001/Source/devtools/front_e... Source/devtools/front_end/documentation/DocumentationView.js:202: dataTypeValue.classList.add("documentation-color-box"); Ditto for these two classes https://codereview.chromium.org/525363003/diff/160001/Source/devtools/front_e... Source/devtools/front_end/documentation/DocumentationView.js:207: optional.classList.add("documentation-color-box"); Ditto for these two classes https://codereview.chromium.org/525363003/diff/160001/Source/devtools/front_e... Source/devtools/front_end/documentation/DocumentationView.js:231: returnValueDescription.classList.add("documentation-text"); Ditto https://codereview.chromium.org/525363003/diff/160001/Source/devtools/front_e... Source/devtools/front_end/documentation/DocumentationView.js:249: example.classList.add("documentation-section-content"); Ditto here and below https://codereview.chromium.org/525363003/diff/160001/Source/devtools/front_e... File Source/devtools/front_end/documentationView.css (right): https://codereview.chromium.org/525363003/diff/160001/Source/devtools/front_e... Source/devtools/front_end/documentationView.css:10: padding: 1em; In general, we don't use ems in our layout?
Oops, meant to say "...such prefixes as..."
Patchset #6 (id:180001) has been deleted
https://codereview.chromium.org/525363003/diff/160001/Source/devtools/front_e... File Source/devtools/front_end/documentation/DocumentationView.js (right): https://codereview.chromium.org/525363003/diff/160001/Source/devtools/front_e... Source/devtools/front_end/documentation/DocumentationView.js:161: var signature = section.createChild("div", "documentation-method-signature"); On 2014/09/05 15:20:44, lushnikov wrote: > lets use "span" here instead of "div" and get rid of "display: inline" property > in .documentation-method-signature Done. https://codereview.chromium.org/525363003/diff/160001/Source/devtools/front_e... Source/devtools/front_end/documentation/DocumentationView.js:166: var separator = signature.createChild("inline"); On 2014/09/05 15:20:44, lushnikov wrote: > there's no tag "inline". You probably would like to use > signature.createTextChild(", ") instead Done. https://codereview.chromium.org/525363003/diff/160001/Source/devtools/front_e... Source/devtools/front_end/documentation/DocumentationView.js:172: parameterType.className = "documentation-parameter-data-type-value"; On 2014/09/05 15:20:44, lushnikov wrote: > You've already set this className in element creationg Done. https://codereview.chromium.org/525363003/diff/160001/Source/devtools/front_e... Source/devtools/front_end/documentation/DocumentationView.js:175: parameterType.classList.add("documentation-color-box"); On 2014/09/05 16:43:00, apavlov (OOO until Sept.19) wrote: > This class can be provided during the element construction in createChild Done. https://codereview.chromium.org/525363003/diff/160001/Source/devtools/front_e... Source/devtools/front_end/documentation/DocumentationView.js:176: parameterType.textContent = parameters[i].dataType + optional; On 2014/09/05 15:20:44, lushnikov wrote: > parameterType.textContent = parameters[i].dataType + (parameters[i].optional ? > "=" : ""); Done. https://codereview.chromium.org/525363003/diff/160001/Source/devtools/front_e... Source/devtools/front_end/documentation/DocumentationView.js:178: var separator = signature.createChild("inline"); On 2014/09/05 15:20:44, lushnikov wrote: > ditto Done. https://codereview.chromium.org/525363003/diff/160001/Source/devtools/front_e... Source/devtools/front_end/documentation/DocumentationView.js:179: separator.textContent = ") : "; On 2014/09/05 15:20:44, lushnikov wrote: > lets avoid space after ")" Done. https://codereview.chromium.org/525363003/diff/160001/Source/devtools/front_e... Source/devtools/front_end/documentation/DocumentationView.js:197: parameter.classList.add("documentation-section-content"); On 2014/09/05 16:43:00, apavlov (OOO until Sept.19) wrote: > Ditto Done. https://codereview.chromium.org/525363003/diff/160001/Source/devtools/front_e... Source/devtools/front_end/documentation/DocumentationView.js:202: dataTypeValue.classList.add("documentation-color-box"); On 2014/09/05 16:43:00, apavlov (OOO until Sept.19) wrote: > Ditto for these two classes Done. https://codereview.chromium.org/525363003/diff/160001/Source/devtools/front_e... Source/devtools/front_end/documentation/DocumentationView.js:207: optional.classList.add("documentation-color-box"); On 2014/09/05 16:43:00, apavlov (OOO until Sept.19) wrote: > Ditto for these two classes Done. https://codereview.chromium.org/525363003/diff/160001/Source/devtools/front_e... Source/devtools/front_end/documentation/DocumentationView.js:231: returnValueDescription.classList.add("documentation-text"); On 2014/09/05 16:43:00, apavlov (OOO until Sept.19) wrote: > Ditto Done. https://codereview.chromium.org/525363003/diff/160001/Source/devtools/front_e... Source/devtools/front_end/documentation/DocumentationView.js:249: example.classList.add("documentation-section-content"); On 2014/09/05 16:43:00, apavlov (OOO until Sept.19) wrote: > Ditto here and below Done. https://codereview.chromium.org/525363003/diff/160001/Source/devtools/front_e... File Source/devtools/front_end/documentationView.css (right): https://codereview.chromium.org/525363003/diff/160001/Source/devtools/front_e... Source/devtools/front_end/documentationView.css:10: padding: 1em; On 2014/09/05 16:43:00, apavlov (OOO until Sept.19) wrote: > In general, we don't use ems in our layout? Done. https://codereview.chromium.org/525363003/diff/160001/Source/devtools/front_e... Source/devtools/front_end/documentationView.css:40: font-family: dejavu sans mono, monospace; On 2014/09/05 15:20:44, lushnikov wrote: > We never want to tweak font-family property in any style. Please, remove Done. https://codereview.chromium.org/525363003/diff/160001/Source/devtools/front_e... Source/devtools/front_end/documentationView.css:70: font-family: monospace; On 2014/09/05 15:20:44, lushnikov wrote: > the font-family property is very tricky. Instead of setting it as "monospace" > here, you'd like to use "monospace" class fro the element Done. https://codereview.chromium.org/525363003/diff/160001/Source/devtools/front_e... Source/devtools/front_end/documentationView.css:109: .documentation-color-box { On 2014/09/05 15:20:44, lushnikov wrote: > The "documentation-color-box" css class doesn't help to understand > which elements will be styled with this class; instead, it > describes how they will look, which is not helpful as I can figure > this out from its properties. > > Believe or not, but you don't want to extract common css properties into a > separate class, if they only *happen* to be equal. Done. https://codereview.chromium.org/525363003/diff/160001/Source/devtools/front_e... Source/devtools/front_end/documentationView.css:118: line-height: 160%; On 2014/09/05 15:20:44, lushnikov wrote: > i believe you want smth like 1.5 here. We need @eustas to verify our typography > here Done.
semeny@google.com changed reviewers: + loislo@chromium.org
https://codereview.chromium.org/525363003/diff/200001/Source/devtools/front_e... File Source/devtools/front_end/documentation/DocumentationURLProvider.js (right): https://codereview.chromium.org/525363003/diff/200001/Source/devtools/front_e... Source/devtools/front_end/documentation/DocumentationURLProvider.js:167: return this._articleList.get(searchTerm) || []; this construction looks strange
https://codereview.chromium.org/525363003/diff/220001/Source/devtools/front_e... File Source/devtools/front_end/documentation/DocumentationURLProvider.js (right): https://codereview.chromium.org/525363003/diff/220001/Source/devtools/front_e... Source/devtools/front_end/documentation/DocumentationURLProvider.js:166: if (searchTerm.toUpperCase() !== searchTerm) I meant if download has been finished then you need to return the answer despite the fact is it a constant or not.
https://codereview.chromium.org/525363003/diff/220001/Source/devtools/front_e... File Source/devtools/front_end/documentation/DocumentationURLProvider.js (right): https://codereview.chromium.org/525363003/diff/220001/Source/devtools/front_e... Source/devtools/front_end/documentation/DocumentationURLProvider.js:166: if (searchTerm.toUpperCase() !== searchTerm) Looks like we should return the answer immediately if the search term is a constant. and fetch the pages in the other case.
Please attach a screenshot. lgtm given loislo's comments are addressed.
Patchset #8 (id:240001) has been deleted
The CQ bit was checked by loislo@chromium.org
lgtm
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/semeny@google.com/525363003/280001
The CQ bit was unchecked by commit-bot@chromium.org
Failed to apply patch for
Source/devtools/front_end/documentation/DocumentationView.js:
While running patch -p1 --forward --force --no-backup-if-mismatch;
patching file Source/devtools/front_end/documentation/DocumentationView.js
Hunk #6 FAILED at 240.
Hunk #7 succeeded at 275 (offset 1 line).
Hunk #8 succeeded at 327 (offset 1 line).
1 out of 8 hunks FAILED -- saving rejects to file
Source/devtools/front_end/documentation/DocumentationView.js.rej
Patch: Source/devtools/front_end/documentation/DocumentationView.js
Index: Source/devtools/front_end/documentation/DocumentationView.js
diff --git a/Source/devtools/front_end/documentation/DocumentationView.js
b/Source/devtools/front_end/documentation/DocumentationView.js
index
5960b4bb7b2e2d5d425c81cecca02aabd396782e..b8cae771e03ea0ceff57a8a870eb0e517b8b141d
100644
--- a/Source/devtools/front_end/documentation/DocumentationView.js
+++ b/Source/devtools/front_end/documentation/DocumentationView.js
@@ -102,8 +102,9 @@ WebInspector.DocumentationView.Renderer.prototype = {
{
this._createPageTitle(this._article.pageTitle, this._searchItem);
this._createStandardizationStatus(this._article.standardizationStatus);
- this._createTextSectionWithTitle("Summary", this._article.summary);
this._createSignatureSection(this._article.parameters,
this._article.methods);
+ this._createTextSectionWithTitle("Summary", this._article.summary);
+ this._createParametersSection(this._article.parameters);
this._createReturnValueSection(this._article.methods);
this._createExamplesSection(this._article.examples);
this._createTextSectionWithTitle("Remarks", this._article.remarks);
@@ -148,6 +149,7 @@ WebInspector.DocumentationView.Renderer.prototype = {
title.textContent = titleText;
var text = this._renderBlock(article);
text.classList.add("documentation-text");
+ text.classList.add("documentation-section-content");
section.appendChild(text);
},
@@ -157,13 +159,35 @@ WebInspector.DocumentationView.Renderer.prototype = {
*/
_createSignatureSection: function(parameters, method)
{
+ if (!method)
+ return;
+ var section = this._element.createChild("p", "documentation-section");
+ var signature = section.createChild("span",
"documentation-method-signature documentation-section-content monospace");
+ var methodName = signature.createChild("span",
"documentation-method-name");
+ methodName.textContent = this._searchItem.split(".").peekLast() + "(";
+ for (var i = 0; i < parameters.length; ++i) {
+ if (i > 0)
+ signature.createTextChild(", ")
+ var parameterType = signature.createChild("span",
"documentation-parameter-data-type-value");
+ parameterType.textContent = parameters[i].dataType +
(parameters[i].optional ? "=" : "");;
+ }
+ signature.createTextChild("): ");
+ var returnTypeElement = signature.createChild("span",
"documentation-parameter-data-type-value");
+ returnTypeElement.textContent = method.returnValueName;
+ },
+
+ /**
+ * @param {!Array.<!WebInspector.JSArticle.Parameter>} parameters
+ */
+ _createParametersSection: function(parameters)
+ {
if (!parameters.length)
return;
var section = this._element.createChild("div",
"documentation-section");
var title = section.createChild("div", "documentation-section-title");
title.textContent = "Parameters";
for (var i = 0; i < parameters.length; ++i) {
- var parameter = section.createChild("div",
"documentation-parameter");
+ var parameter = section.createChild("div", "documentation-parameter
documentation-section-content");
var header = parameter.createChild("div",
"documentation-parameter-header");
var name = header.createChild("span",
"documentation-parameter-name");
name.textContent = parameters[i].name;
@@ -186,13 +210,12 @@ WebInspector.DocumentationView.Renderer.prototype = {
{
if (!method)
return;
-
var section = this._element.createChild("div",
"documentation-section");
var title = section.createChild("div", "documentation-section-title");
title.textContent = "Return Value";
- var returnValueName = section.createChild("div",
"documentation-return-value");
+ var returnValueName = section.createChild("div",
"documentation-section-content documentation-return-value");
returnValueName.textContent = WebInspector.UIString("Returns an object
of type " + method.returnValueName + ".");
- var returnValueDescription = section.createChild("div",
"documentation-return-value");
+ var returnValueDescription = section.createChild("div",
"documentation-section-content documentation-text");
returnValueDescription.textContent =
WebInspector.UIString(method.returnValueDescription);
},
@@ -209,7 +232,7 @@ WebInspector.DocumentationView.Renderer.prototype = {
title.textContent = "Examples";
for (var i = 0; i < examples.length; ++i) {
- var example = section.createChild("div", "documentation-example");
+ var example = section.createChild("div", "documentation-example
documentation-section-content");
var exampleDescription = example.createChild("div",
"documentation-example-description-section");
if (examples[i].liveUrl) {
var liveUrl = exampleDescription.createChild("a",
"documentation-example-link");
@@ -217,9 +240,10 @@ WebInspector.DocumentationView.Renderer.prototype = {
liveUrl.href = examples[i].liveUrl;
liveUrl.textContent = WebInspector.UIString("Example");
}
-
exampleDescription.appendChild(this._renderBlock(examples[i].description));
- var code = example.createChild("div", "documentation-code");
- code.classList.add("source-code");
+ var description = this._renderBlock(examples[i].description);
+ description.classList.add("documentation-text");
+ exampleDescription.appendChild(description);
+ var code = example.createChild("div", "documentation-code
source-code");
code.textContent = examples[i].code;
if (!examples[i].language)
continue;
@@ -251,8 +275,7 @@ WebInspector.DocumentationView.Renderer.prototype = {
element = document.createElementWithClass("span",
"documentation-code-tag");
break;
case elementTypes.CodeBlock:
- element = document.createElementWithClass("pre",
"documentation-code");
- element.classList.add("source-code");
+ element = document.createElementWithClass("pre",
"documentation-code source-code");
element.textContent = article.code();
break;
case elementTypes.PlainText:
@@ -304,12 +327,15 @@
WebInspector.DocumentationView.ContextMenuProvider.prototype = {
return;
if (descriptors.length === 1) {
var formatString = WebInspector.useLowerCaseMenuTitles() ? "Show
documentation for %s.%s" : "Show Documentation for %s.%s";
- contextMenu.appendItem(WebInspector.UIString(formatString,
descriptors[0].name(), descriptors[0].searchItem()),
WebInspector.DocumentationView.showDocumentationURL.bind(null,
descriptors[0].url(), descriptors[0].searchItem()));
+ var methodName = String.sprintf("%s.%s", descriptors[0].name(),
descriptors[0].searchItem());
+ contextMenu.appendItem(WebInspector.UIString(formatString,
descriptors[0].name(), descriptors[0].searchItem()),
WebInspector.DocumentationView.showDocumentationURL.bind(null,
descriptors[0].url(), methodName));
return;
}
var subMenuItem =
contextMenu.appendSubMenuItem(WebInspector.UIString(WebInspector.useLowerCaseMenuTitles()
? "Show documentation for..." : "Show Documentation for..."));
- for (var i = 0; i < descriptors.length; ++i)
- subMenuItem.appendItem(String.sprintf("%s.%s",
descriptors[i].name(), descriptors[i].searchItem()),
WebInspector.DocumentationView.showDocumentationURL.bind(null,
descriptors[i].url(), descriptors[i].searchItem()));
+ for (var i = 0; i < descriptors.length; ++i) {
+ var methodName = String.sprintf("%s.%s", descriptors[i].name(),
descriptors[i].searchItem());
+ subMenuItem.appendItem(methodName,
WebInspector.DocumentationView.showDocumentationURL.bind(null,
descriptors[i].url(), methodName));
+ }
},
/**
Patchset #11 (id:320001) has been deleted
The CQ bit was checked by semeny@google.com
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/semeny@google.com/525363003/300001
The CQ bit was unchecked by commit-bot@chromium.org
Failed to apply patch for
Source/devtools/front_end/documentation/DocumentationView.js:
While running patch -p1 --forward --force --no-backup-if-mismatch;
patching file Source/devtools/front_end/documentation/DocumentationView.js
Hunk #6 FAILED at 240.
Hunk #7 succeeded at 275 (offset 1 line).
Hunk #8 succeeded at 327 (offset 1 line).
1 out of 8 hunks FAILED -- saving rejects to file
Source/devtools/front_end/documentation/DocumentationView.js.rej
Patch: Source/devtools/front_end/documentation/DocumentationView.js
Index: Source/devtools/front_end/documentation/DocumentationView.js
diff --git a/Source/devtools/front_end/documentation/DocumentationView.js
b/Source/devtools/front_end/documentation/DocumentationView.js
index
5960b4bb7b2e2d5d425c81cecca02aabd396782e..b8cae771e03ea0ceff57a8a870eb0e517b8b141d
100644
--- a/Source/devtools/front_end/documentation/DocumentationView.js
+++ b/Source/devtools/front_end/documentation/DocumentationView.js
@@ -102,8 +102,9 @@ WebInspector.DocumentationView.Renderer.prototype = {
{
this._createPageTitle(this._article.pageTitle, this._searchItem);
this._createStandardizationStatus(this._article.standardizationStatus);
- this._createTextSectionWithTitle("Summary", this._article.summary);
this._createSignatureSection(this._article.parameters,
this._article.methods);
+ this._createTextSectionWithTitle("Summary", this._article.summary);
+ this._createParametersSection(this._article.parameters);
this._createReturnValueSection(this._article.methods);
this._createExamplesSection(this._article.examples);
this._createTextSectionWithTitle("Remarks", this._article.remarks);
@@ -148,6 +149,7 @@ WebInspector.DocumentationView.Renderer.prototype = {
title.textContent = titleText;
var text = this._renderBlock(article);
text.classList.add("documentation-text");
+ text.classList.add("documentation-section-content");
section.appendChild(text);
},
@@ -157,13 +159,35 @@ WebInspector.DocumentationView.Renderer.prototype = {
*/
_createSignatureSection: function(parameters, method)
{
+ if (!method)
+ return;
+ var section = this._element.createChild("p", "documentation-section");
+ var signature = section.createChild("span",
"documentation-method-signature documentation-section-content monospace");
+ var methodName = signature.createChild("span",
"documentation-method-name");
+ methodName.textContent = this._searchItem.split(".").peekLast() + "(";
+ for (var i = 0; i < parameters.length; ++i) {
+ if (i > 0)
+ signature.createTextChild(", ")
+ var parameterType = signature.createChild("span",
"documentation-parameter-data-type-value");
+ parameterType.textContent = parameters[i].dataType +
(parameters[i].optional ? "=" : "");;
+ }
+ signature.createTextChild("): ");
+ var returnTypeElement = signature.createChild("span",
"documentation-parameter-data-type-value");
+ returnTypeElement.textContent = method.returnValueName;
+ },
+
+ /**
+ * @param {!Array.<!WebInspector.JSArticle.Parameter>} parameters
+ */
+ _createParametersSection: function(parameters)
+ {
if (!parameters.length)
return;
var section = this._element.createChild("div",
"documentation-section");
var title = section.createChild("div", "documentation-section-title");
title.textContent = "Parameters";
for (var i = 0; i < parameters.length; ++i) {
- var parameter = section.createChild("div",
"documentation-parameter");
+ var parameter = section.createChild("div", "documentation-parameter
documentation-section-content");
var header = parameter.createChild("div",
"documentation-parameter-header");
var name = header.createChild("span",
"documentation-parameter-name");
name.textContent = parameters[i].name;
@@ -186,13 +210,12 @@ WebInspector.DocumentationView.Renderer.prototype = {
{
if (!method)
return;
-
var section = this._element.createChild("div",
"documentation-section");
var title = section.createChild("div", "documentation-section-title");
title.textContent = "Return Value";
- var returnValueName = section.createChild("div",
"documentation-return-value");
+ var returnValueName = section.createChild("div",
"documentation-section-content documentation-return-value");
returnValueName.textContent = WebInspector.UIString("Returns an object
of type " + method.returnValueName + ".");
- var returnValueDescription = section.createChild("div",
"documentation-return-value");
+ var returnValueDescription = section.createChild("div",
"documentation-section-content documentation-text");
returnValueDescription.textContent =
WebInspector.UIString(method.returnValueDescription);
},
@@ -209,7 +232,7 @@ WebInspector.DocumentationView.Renderer.prototype = {
title.textContent = "Examples";
for (var i = 0; i < examples.length; ++i) {
- var example = section.createChild("div", "documentation-example");
+ var example = section.createChild("div", "documentation-example
documentation-section-content");
var exampleDescription = example.createChild("div",
"documentation-example-description-section");
if (examples[i].liveUrl) {
var liveUrl = exampleDescription.createChild("a",
"documentation-example-link");
@@ -217,9 +240,10 @@ WebInspector.DocumentationView.Renderer.prototype = {
liveUrl.href = examples[i].liveUrl;
liveUrl.textContent = WebInspector.UIString("Example");
}
-
exampleDescription.appendChild(this._renderBlock(examples[i].description));
- var code = example.createChild("div", "documentation-code");
- code.classList.add("source-code");
+ var description = this._renderBlock(examples[i].description);
+ description.classList.add("documentation-text");
+ exampleDescription.appendChild(description);
+ var code = example.createChild("div", "documentation-code
source-code");
code.textContent = examples[i].code;
if (!examples[i].language)
continue;
@@ -251,8 +275,7 @@ WebInspector.DocumentationView.Renderer.prototype = {
element = document.createElementWithClass("span",
"documentation-code-tag");
break;
case elementTypes.CodeBlock:
- element = document.createElementWithClass("pre",
"documentation-code");
- element.classList.add("source-code");
+ element = document.createElementWithClass("pre",
"documentation-code source-code");
element.textContent = article.code();
break;
case elementTypes.PlainText:
@@ -304,12 +327,15 @@
WebInspector.DocumentationView.ContextMenuProvider.prototype = {
return;
if (descriptors.length === 1) {
var formatString = WebInspector.useLowerCaseMenuTitles() ? "Show
documentation for %s.%s" : "Show Documentation for %s.%s";
- contextMenu.appendItem(WebInspector.UIString(formatString,
descriptors[0].name(), descriptors[0].searchItem()),
WebInspector.DocumentationView.showDocumentationURL.bind(null,
descriptors[0].url(), descriptors[0].searchItem()));
+ var methodName = String.sprintf("%s.%s", descriptors[0].name(),
descriptors[0].searchItem());
+ contextMenu.appendItem(WebInspector.UIString(formatString,
descriptors[0].name(), descriptors[0].searchItem()),
WebInspector.DocumentationView.showDocumentationURL.bind(null,
descriptors[0].url(), methodName));
return;
}
var subMenuItem =
contextMenu.appendSubMenuItem(WebInspector.UIString(WebInspector.useLowerCaseMenuTitles()
? "Show documentation for..." : "Show Documentation for..."));
- for (var i = 0; i < descriptors.length; ++i)
- subMenuItem.appendItem(String.sprintf("%s.%s",
descriptors[i].name(), descriptors[i].searchItem()),
WebInspector.DocumentationView.showDocumentationURL.bind(null,
descriptors[i].url(), descriptors[i].searchItem()));
+ for (var i = 0; i < descriptors.length; ++i) {
+ var methodName = String.sprintf("%s.%s", descriptors[i].name(),
descriptors[i].searchItem());
+ subMenuItem.appendItem(methodName,
WebInspector.DocumentationView.showDocumentationURL.bind(null,
descriptors[i].url(), methodName));
+ }
},
/**
The CQ bit was checked by semeny@google.com
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/semeny@google.com/525363003/340001
Message was sent while issue was closed.
Committed patchset #11 (id:340001) as 181648 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
