[Webkit-unassigned] [Bug 226883] Web Inspector: add contextual documentation for CSS properties

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Jun 23 10:44:31 PDT 2021


https://bugs.webkit.org/show_bug.cgi?id=226883

Razvan Caliman <rcaliman at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rcaliman at apple.com

--- Comment #6 from Razvan Caliman <rcaliman at apple.com> ---
Comment on attachment 431883
  --> https://bugs.webkit.org/attachment.cgi?id=431883
Patch v1.0 -First Draft For Review

View in context: https://bugs.webkit.org/attachment.cgi?id=431883&action=review

>> Source/WebInspectorUI/UserInterface/Main.html:276
>> +    <script src= "Views/VSCodeCustomDataCSS.js"></script>
> 
> This should be placed below in alphabetical order with all the other `Views/`.

We should use a generic name for the data source. Perhaps something like `ContextualDocumentationDataset` or `ContextualDocumentationDB`

The fact that it comes form VSCode is an implementation detail. 
If we change the contents of this dataset to something else, the filename and references should not need to be changed.

>> Source/WebInspectorUI/UserInterface/Views/ComputedStyleDetailsPanel.css:60
>> +/* commented the above code and changed it below to align the go-to-arrow with the documentation-popover-info-button  */
> 
> We don't usually leave comments like this in the source. Instead, add an explanation in the changelog for this file.

Yes, code that is not used should not be shipped. Over time, it gets ambiguous why something lingers around.
`git blame` can be used to look back though a file's history if someone needs how/when something changed.

> Source/WebInspectorUI/UserInterface/Views/SpreadsheetStyleProperty.js:426
> +        console.log("trigged blur")

Oops? Remove leftover `console.log()`

>> Source/WebInspectorUI/UserInterface/Views/SpreadsheetStyleProperty.js:554
>> +                    this._infoElement.setAttribute("title","Click to show documentation")
> 
> We have a system for localizing strings that you should use for the content of this attribute (and other places above that I've missed). Anywhere there is text the user will see, it needs to be localized for a number of different languages. Search for uses of WI.UIString to see examples of proper usage.

The map of localized strings is in `Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js`.

A tip: you can replace plain strings in your code with `UI.String()` and the required parameters. 
Then, run the provided script to automatically update `localizedStrings.js` instead of doing it manually:

`/Tools/Scripts/extract-localizable-js-strings --utf8 Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js Source/WebInspectorUI/UserInterface`

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20210623/992d3c73/attachment.htm>


More information about the webkit-unassigned mailing list