RESOLVED FIXED 222007
Web Inspector: Implement Grid Overlay "Show track sizes" drawing
https://bugs.webkit.org/show_bug.cgi?id=222007
Summary Web Inspector: Implement Grid Overlay "Show track sizes" drawing
Patrick Angle
Reported 2021-02-16 15:34:08 PST
Add drawing of track size labels to the grid overlay.
Attachments
WIP v0.1 - First pass (7.55 KB, patch)
2021-02-17 11:45 PST, Patrick Angle
no flags
WIP v0.2 - Almost all authored values (10.17 KB, patch)
2021-02-17 17:46 PST, Patrick Angle
no flags
Screenshot 1 of WIP v0.2 (647.49 KB, image/png)
2021-02-17 17:49 PST, Patrick Angle
no flags
Screenshot 2 of WIP v0.2 (5.01 MB, image/png)
2021-02-17 17:49 PST, Patrick Angle
no flags
Screenshot 3 of WIP v0.2 (824.33 KB, image/png)
2021-02-17 17:49 PST, Patrick Angle
no flags
Patch v1.0 - Label style adjustments and cleanup (11.59 KB, patch)
2021-02-17 20:04 PST, Patrick Angle
no flags
Screenshot of Patch v1.0 (3.54 MB, image/png)
2021-02-17 20:05 PST, Patrick Angle
no flags
Patch v1.1 - Review notes, track size label spacing adjustment (11.95 KB, patch)
2021-02-18 10:03 PST, Patrick Angle
no flags
Screenshot of Patch v1.1 (3.54 MB, image/png)
2021-02-18 10:10 PST, Patrick Angle
no flags
Radar WebKit Bug Importer
Comment 1 2021-02-16 15:34:25 PST
Patrick Angle
Comment 2 2021-02-17 11:45:58 PST
Created attachment 420686 [details] WIP v0.1 - First pass
Patrick Angle
Comment 3 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.
Patrick Angle
Comment 4 2021-02-17 17:46:40 PST
Created attachment 420769 [details] WIP v0.2 - Almost all authored values
Patrick Angle
Comment 5 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`.
Patrick Angle
Comment 6 2021-02-17 17:49:19 PST
Created attachment 420770 [details] Screenshot 1 of WIP v0.2
Patrick Angle
Comment 7 2021-02-17 17:49:31 PST
Created attachment 420771 [details] Screenshot 2 of WIP v0.2
Patrick Angle
Comment 8 2021-02-17 17:49:45 PST
Created attachment 420772 [details] Screenshot 3 of WIP v0.2
Patrick Angle
Comment 9 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.
Patrick Angle
Comment 10 2021-02-17 20:04:52 PST
Created attachment 420787 [details] Patch v1.0 - Label style adjustments and cleanup
Patrick Angle
Comment 11 2021-02-17 20:05:29 PST
Created attachment 420788 [details] Screenshot of Patch v1.0
Blaze Burg
Comment 12 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.
Patrick Angle
Comment 13 2021-02-18 10:03:29 PST
Created attachment 420839 [details] Patch v1.1 - Review notes, track size label spacing adjustment
Patrick Angle
Comment 14 2021-02-18 10:10:46 PST
Created attachment 420841 [details] Screenshot of Patch v1.1
EWS
Comment 15 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].
Note You need to log in before you can comment on or make changes to this bug.