Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Web Inspector: CSS Grid Page Overlay support for masonry layout #20934

Conversation

rcaliman-apple
Copy link
Contributor

@rcaliman-apple rcaliman-apple commented Nov 27, 2023

9a3f6dd

Web Inspector: CSS Grid Page Overlay support for masonry layout
https://bugs.webkit.org/show_bug.cgi?id=265381
rdar://118833618

Reviewed by Patrick Angle.

There are no actual rows or columns in the masonry axis of a masonry layout.
But we can borrow the concept to draw the two lines at the start and end of the masonry axis.

There are rows or columns in the grid axis of a masonry layout, just like in a regular grid layout.
Their lines will wrap the masonry layout in the other direction.

* Source/WebCore/inspector/InspectorOverlay.cpp:
(WebCore::InspectorOverlay::buildGridOverlay):

Canonical link: https://commits.webkit.org/271332@main

ce94356

Misc iOS, tvOS & watchOS macOS Linux Windows
βœ… πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac βœ… πŸ›  wpe βœ… πŸ›  wincairo
βœ… πŸ§ͺ bindings βœ… πŸ›  ios-sim βœ… πŸ›  mac-AS-debug βœ… πŸ§ͺ wpe-wk2
βœ… πŸ§ͺ webkitperl βœ… πŸ§ͺ ios-wk2 βœ… πŸ§ͺ api-mac βœ… πŸ§ͺ api-wpe
βœ… πŸ§ͺ ios-wk2-wpt βœ… πŸ§ͺ mac-wk1 βœ… πŸ›  gtk
βœ… πŸ§ͺ api-ios βœ… πŸ§ͺ mac-wk2 βœ… πŸ§ͺ gtk-wk2
βœ… πŸ›  tv βœ… πŸ§ͺ mac-AS-debug-wk2 βœ… πŸ§ͺ api-gtk
βœ… πŸ›  tv-sim
βœ… πŸ›  πŸ§ͺ merge βœ… πŸ›  watch
βœ… πŸ›  watch-sim

@rcaliman-apple rcaliman-apple self-assigned this Nov 27, 2023
@rcaliman-apple rcaliman-apple added the Web Inspector Bugs related to the WebKit Web Inspector. label Nov 27, 2023
// But we can borrow the concept to draw the two lines at the start and end of the masonry axis.
if (renderGrid.areMasonryRows()) {
auto firstRowPosition = rowPositions[0];
auto lastRowPosition = masonryContentSize;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will the first row/column position always be 0? Or should lastRowPosition actually equal firstRowPosition + masonryContentSize?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! πŸ‘

No, the first row (or column) position isn't always zero because it includes the border, padding and offsets for the grid container itself:

positions[0] = borderAndPadding + offset.positionOffset;

Fixed in the latest patch.

Screenshot before
Screenshot 2023-11-29 at 21 05 55

Screenshot after
Screenshot 2023-11-29 at 21 00 55

(the gap between the last item and the bottom line is the default margin for a <p> from the browser styles)

@rcaliman-apple rcaliman-apple force-pushed the eng/Web-Inspector-CSS-Grid-Page-Overlay-support-for-masonry-layout branch from 46abb52 to ce94356 Compare November 29, 2023 20:14
@rcaliman-apple rcaliman-apple added the merge-queue Applied to send a pull request to merge-queue label Nov 30, 2023
https://bugs.webkit.org/show_bug.cgi?id=265381
rdar://118833618

Reviewed by Patrick Angle.

There are no actual rows or columns in the masonry axis of a masonry layout.
But we can borrow the concept to draw the two lines at the start and end of the masonry axis.

There are rows or columns in the grid axis of a masonry layout, just like in a regular grid layout.
Their lines will wrap the masonry layout in the other direction.

* Source/WebCore/inspector/InspectorOverlay.cpp:
(WebCore::InspectorOverlay::buildGridOverlay):

Canonical link: https://commits.webkit.org/271332@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/Web-Inspector-CSS-Grid-Page-Overlay-support-for-masonry-layout branch from ce94356 to 9a3f6dd Compare November 30, 2023 12:07
@webkit-commit-queue
Copy link
Collaborator

Committed 271332@main (9a3f6dd): https://commits.webkit.org/271332@main

Reviewed commits have been landed. Closing PR #20934 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit 9a3f6dd into WebKit:main Nov 30, 2023
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Nov 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Web Inspector Bugs related to the WebKit Web Inspector.
Projects
None yet
4 participants