Bug 194883 - Web Inspector: eliminate manual syncing of numeric constants used by JavaScript and CSS
Summary: Web Inspector: eliminate manual syncing of numeric constants used by JavaScri...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P5 Normal
Assignee: Devin Rousso
URL:
Keywords: InRadar
Depends on:
Blocks: 195689
  Show dependency treegraph
 
Reported: 2019-02-20 16:43 PST by Matt Baker
Modified: 2019-03-13 11:51 PDT (History)
5 users (show)

See Also:


Attachments
Patch (18.73 KB, patch)
2019-03-10 19:31 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff
Patch (14.31 KB, patch)
2019-03-11 13:05 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Matt Baker 2019-02-20 16:43:32 PST
Summary:
Eliminate manual syncing of numeric constants used by JavaScript and CSS.

Web Inspector CSS occasionally uses numeric constants that must also be available to code. Currently we rely on the developer to keep the JavaScript and CSS worlds in sync. These custom properties should have their values set by Web Inspector code so that can't get out of sync.

Note:
The CSS custom property `--recording-auto-capture-input-margin` in CanvasOverviewContentView.css has already become out of sync. Let's fix this!
Comment 1 Radar WebKit Bug Importer 2019-02-20 16:44:00 PST
<rdar://problem/48257785>
Comment 2 Devin Rousso 2019-03-10 19:31:29 PDT
Created attachment 364220 [details]
Patch
Comment 3 Joseph Pecoraro 2019-03-11 12:21:26 PDT
Comment on attachment 364220 [details]
Patch

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

r=me

> Source/WebInspectorUI/UserInterface/Views/MemoryTimelineView.js:117
> +    static get _memoryCategoryViewheight() { return 75; }

Typo: "...Viewheight" => "...ViewHeight"

I also really dislike _ prefixed getters. They are very confusing / unexpected to work work and this is a constant that doesn't need to be "private".

> Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.css:119
> +.network-table .cell.name > .status {
> +    -webkit-margin-left: 4px;
> +}

Seems like this should be `-webkit-margin-start` not `-webkit-margin-left`.

The sync changes and the -webkit-margin changes should really be different patches. They are almost entirely unrelated changes each with their own possibility of causing regressions.

> Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.js:201
> +    static get _nodeWaterfallDOMEventSize() { return 8; }

Same, this doesn't need to be underscore prefixed.
Comment 4 Devin Rousso 2019-03-11 13:05:25 PDT
Created attachment 364280 [details]
Patch
Comment 5 WebKit Commit Bot 2019-03-11 13:35:27 PDT
Comment on attachment 364280 [details]
Patch

Clearing flags on attachment: 364280

Committed r242737: <https://trac.webkit.org/changeset/242737>
Comment 6 WebKit Commit Bot 2019-03-11 13:35:29 PDT
All reviewed patches have been landed.  Closing bug.