Bug 222007 - Web Inspector: Implement Grid Overlay "Show track sizes" drawing
Summary: Web Inspector: Implement Grid Overlay "Show track sizes" drawing
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Patrick Angle
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-02-16 15:34 PST by Patrick Angle
Modified: 2021-02-18 12:58 PST (History)
7 users (show)

See Also:


Attachments
WIP v0.1 - First pass (7.55 KB, patch)
2021-02-17 11:45 PST, Patrick Angle
no flags Details | Formatted Diff | Diff
WIP v0.2 - Almost all authored values (10.17 KB, patch)
2021-02-17 17:46 PST, Patrick Angle
no flags Details | Formatted Diff | Diff
Screenshot 1 of WIP v0.2 (647.49 KB, image/png)
2021-02-17 17:49 PST, Patrick Angle
no flags Details
Screenshot 2 of WIP v0.2 (5.01 MB, image/png)
2021-02-17 17:49 PST, Patrick Angle
no flags Details
Screenshot 3 of WIP v0.2 (824.33 KB, image/png)
2021-02-17 17:49 PST, Patrick Angle
no flags Details
Patch v1.0 - Label style adjustments and cleanup (11.59 KB, patch)
2021-02-17 20:04 PST, Patrick Angle
no flags Details | Formatted Diff | Diff
Screenshot of Patch v1.0 (3.54 MB, image/png)
2021-02-17 20:05 PST, Patrick Angle
no flags Details
Patch v1.1 - Review notes, track size label spacing adjustment (11.95 KB, patch)
2021-02-18 10:03 PST, Patrick Angle
no flags Details | Formatted Diff | Diff
Screenshot of Patch v1.1 (3.54 MB, image/png)
2021-02-18 10:10 PST, Patrick Angle
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Patrick Angle 2021-02-16 15:34:08 PST
Add drawing of track size labels to the grid overlay.
Comment 1 Radar WebKit Bug Importer 2021-02-16 15:34:25 PST
<rdar://problem/74409758>
Comment 2 Patrick Angle 2021-02-17 11:45:58 PST
Created attachment 420686 [details]
WIP v0.1 - First pass
Comment 3 Patrick Angle 2021-02-17 11:47:46 PST
Comment on attachment 420686 [details]
WIP v0.1 - First pass

This is a first pass, but has a big flaw which is that using the GridTrackSize doesn't give us access to the actual value that was authored in all cases. For example, using `vw` units in a track size will be resolved to pixels by the time we read it here. Probably need to use the actual `CSSValue` instead to figure this out.
Comment 4 Patrick Angle 2021-02-17 17:46:40 PST
Created attachment 420769 [details]
WIP v0.2 - Almost all authored values
Comment 5 Patrick Angle 2021-02-17 17:47:57 PST
Comment on attachment 420769 [details]
WIP v0.2 - Almost all authored values

The last remaining pieces of this puzzle are the two shorthand CSS properties for sizing tracks, `grid` and `grid-template`.
Comment 6 Patrick Angle 2021-02-17 17:49:19 PST
Created attachment 420770 [details]
Screenshot 1 of WIP v0.2
Comment 7 Patrick Angle 2021-02-17 17:49:31 PST
Created attachment 420771 [details]
Screenshot 2 of WIP v0.2
Comment 8 Patrick Angle 2021-02-17 17:49:45 PST
Created attachment 420772 [details]
Screenshot 3 of WIP v0.2
Comment 9 Patrick Angle 2021-02-17 19:10:16 PST
Comment on attachment 420769 [details]
WIP v0.2 - Almost all authored values

Upon further investigation, this actually does work for `grid` and `grid-template` shorthands because the CSS parser resolves those to their constituent longhand forms.
Comment 10 Patrick Angle 2021-02-17 20:04:52 PST
Created attachment 420787 [details]
Patch v1.0 - Label style adjustments and cleanup
Comment 11 Patrick Angle 2021-02-17 20:05:29 PST
Created attachment 420788 [details]
Screenshot of Patch v1.0
Comment 12 BJ Burg 2021-02-18 09:44:38 PST
Comment on attachment 420787 [details]
Patch v1.0 - Label style adjustments and cleanup

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

r=me

> Source/WebCore/ChangeLog:9
> +        `CSSValue`, not from the GridTrackSize as the later looses the exact authored syntax (e.g. `14vw` is already

Nit: loses

> Source/WebCore/inspector/InspectorOverlay.cpp:1283
> +        if (!styleRule)

IMO, it would be a programming error in .styleRulesForElement() if an empty RefPtr<StyleRule> was included, so please add an ASSERT(styleRule) as well.
Comment 13 Patrick Angle 2021-02-18 10:03:29 PST
Created attachment 420839 [details]
Patch v1.1 - Review notes, track size label spacing adjustment
Comment 14 Patrick Angle 2021-02-18 10:10:46 PST
Created attachment 420841 [details]
Screenshot of Patch v1.1
Comment 15 EWS 2021-02-18 12:58:10 PST
Committed r273098: <https://commits.webkit.org/r273098>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 420839 [details].