Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(409)

Issue 18516007: Add support for 'order' on grid items (Closed)

Created:
7 years, 5 months ago by Julien - ping for review
Modified:
7 years, 5 months ago
Reviewers:
ojan
CC:
blink-reviews, dglazkov+blink, eae+blinkwatch, leviw+renderwatch, jchaffraix+rendering, esprehn, eseidel
Visibility:
Public.

Description

Add support for 'order' on grid items The property is used to modify painting order and the order in which we consider grid items for the auto-flow algorithm. The code is build on the existing OrderIterator and matches what flexbox does. Also to avoid adding an extra child walk, refactored the explicit grid initialization so that we do also initialize the OrderIterator's Vector. This refactoring saves a child walk as we now compute both explicit grid's sizes in a single child walk. BUG=257078 TESTS=fast/css-grid-layout/grid-item-order-auto-flow-resolution.html fast/css-grid-layout/grid-item-order-paint-order-expected.html Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=153536

Patch Set 1 #

Total comments: 2

Patch Set 2 : Squashed the 2 constructors per Ojan's review comment #

Unified diffs Side-by-side diffs Delta from patch set Stats (+209 lines, -31 lines) Patch
A LayoutTests/fast/css-grid-layout/grid-item-order-auto-flow-resolution.html View 1 chunk +73 lines, -0 lines 0 comments Download
A LayoutTests/fast/css-grid-layout/grid-item-order-auto-flow-resolution-expected.txt View 1 chunk +8 lines, -0 lines 0 comments Download
A LayoutTests/fast/css-grid-layout/grid-item-order-paint-order.html View 1 chunk +45 lines, -0 lines 0 comments Download
A LayoutTests/fast/css-grid-layout/grid-item-order-paint-order-expected.html View 1 chunk +19 lines, -0 lines 0 comments Download
M Source/core/rendering/OrderIterator.h View 1 2 chunks +3 lines, -3 lines 0 comments Download
M Source/core/rendering/OrderIterator.cpp View 1 2 chunks +4 lines, -3 lines 0 comments Download
M Source/core/rendering/RenderGrid.h View 5 chunks +5 lines, -1 line 0 comments Download
M Source/core/rendering/RenderGrid.cpp View 5 chunks +52 lines, -24 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
Julien - ping for review
None of the trybot failures are grid related.
7 years, 5 months ago (2013-07-03 20:06:26 UTC) #1
ojan
lgtm https://codereview.chromium.org/18516007/diff/1/Source/core/rendering/OrderIterator.h File Source/core/rendering/OrderIterator.h (right): https://codereview.chromium.org/18516007/diff/1/Source/core/rendering/OrderIterator.h#newcode47 Source/core/rendering/OrderIterator.h:47: OrderIterator(const RenderGrid*); Lets just have it take a ...
7 years, 5 months ago (2013-07-03 20:21:57 UTC) #2
Julien - ping for review
https://codereview.chromium.org/18516007/diff/1/Source/core/rendering/OrderIterator.h File Source/core/rendering/OrderIterator.h (right): https://codereview.chromium.org/18516007/diff/1/Source/core/rendering/OrderIterator.h#newcode47 Source/core/rendering/OrderIterator.h:47: OrderIterator(const RenderGrid*); On 2013/07/03 20:21:57, ojan wrote: > Lets ...
7 years, 5 months ago (2013-07-03 21:38:16 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jchaffraix@chromium.org/18516007/6001
7 years, 5 months ago (2013-07-03 21:39:01 UTC) #4
commit-bot: I haz the power
7 years, 5 months ago (2013-07-03 23:40:14 UTC) #5
Message was sent while issue was closed.
Change committed as 153536

Powered by Google App Engine
This is Rietveld 408576698