[GFC] Implement step 2.4 of Distributing Extra Space - #71661
Conversation
https://bugs.webkit.org/show_bug.cgi?id=321799 <rdar://184933769> Reviewed by NOBODY (OOPS!). This PR implements Step 2.4 of https://drafts.csswg.org/css-grid-1/#extra-space tracksToGrowBeyondGrowthLimits() collects the tracks the item spans that should be unfrozen and grown beyond its limits. shouldTrackGrowBeyondGrowthLimits() checks whether the track's max sizing function is of the type that should be unfrozen in a given phase. * Source/WebCore/layout/formattingContexts/grid/TrackSizingAlgorithm.cpp: (WebCore::Layout::shouldTrackGrowBeyondGrowthLimits): (WebCore::Layout::tracksToGrowBeyondGrowthLimits): (WebCore::Layout::distributeExtraSpace): (WebCore::Layout::resolveIntrinsicTrackSizesWithSpanningItems):
|
EWS run on current version of this PR (hash 718cc2a) Details |
|
|
||
| // https://drafts.csswg.org/css-grid-1/#extra-space | ||
| // 2.4: whether this track should be affected when distributing extra space beyond limits. | ||
| static bool shouldTrackGrowBeyondGrowthLimits(const UnsizedTrack& track, GridItemSizeContributionType sizeContributionType) |
There was a problem hiding this comment.
I think this function needs to take in not only the type of contribution we are accommodating but also the type of the affected track (i.e. whether it is for the base sizes or the growth limit). Even if "Increase sizes to accommodate spanning items crossing content-sized tracks" is written in a way that makes the current approach ok, I think it requires a bit more of a complicated analysis to come to that conclusion compared to if we had both of those arguments here to match how the text is written.
Also I think we might want to revisit our name for GridItemSizeContributionType. It really isn't representing so much as the contribution type for the grid item but a characteristic of the tracks' sizing function. For example, ForIntrinsicMinimums is really about tracks with an intrinsic min track sizing function and accommodating the minimum contribution is just a side effect of what the spec text is using that information for here. If rename the current enum and have GridItemSizeContributionType just be the actual contribution types (e.g. GridItemSizeType::MinimumContribution) we remove that coupling and can reuse it more clearly here
718cc2a
718cc2a