Summary: | [css-grid] GridSpan refactoring | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Manuel Rego Casasnovas <rego> | ||||||
Component: | Layout and Rendering | Assignee: | Manuel Rego Casasnovas <rego> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | commit-queue, jfernandez, simon.fraser, svillar | ||||||
Priority: | P2 | ||||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 153488 | ||||||||
Attachments: |
|
Description
Manuel Rego Casasnovas
2016-02-04 08:01:10 PST
Created attachment 270656 [details]
Patch
Comment on attachment 270656 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=270656&action=review > Source/WebCore/ChangeLog:8 > + Add new boolean to know if a GridSpan is definite or indefinite. I was going to complain asking for an enumerated type, but then I saw that you are actually using it. In any case you need to fix this sentence. > Source/WebCore/rendering/style/GridCoordinate.h:50 > + Extra line here > Source/WebCore/rendering/style/GridResolvedPosition.cpp:79 > + initialPosition = (direction == ForColumns) ? gridItem.style().gridItemColumnStart() : gridItem.style().gridItemRowStart(); Consider doing bool isRowAxis = direction == ForColumns; and using that in these two ternary operators (also you don't need parens). > Source/WebCore/rendering/style/GridResolvedPosition.cpp:311 > + // We can't get our grid positions without running the auto placement algorithm. You can move the comment out of the if block and remove the brackets. Created attachment 271549 [details]
Patch
Thanks for the review, applied suggested changes! Comment on attachment 271549 [details] Patch Clearing flags on attachment: 271549 Committed r196691: <http://trac.webkit.org/changeset/196691> All reviewed patches have been landed. Closing bug. |