Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(72)

Issue 297483005: [CSS Grid Layout] Implementation of the align-self property in grid. (Closed)

Created:
5 years, 1 month ago by jfernandez
Modified:
4 years, 11 months ago
CC:
blink-reviews, blink-reviews-rendering, eae+blinkwatch, jchaffraix+rendering, jfernandez, leviw+renderwatch, pdr., Manuel Rego, rune+blink, svillar, zoltan1
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Visibility:
Public.

Description

[CSS Grid Layout] Implementation of the align-self and align-items properties in grid. BUG=376823 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=178006

Patch Set 1 #

Total comments: 8

Patch Set 2 : Applied suggested changes. #

Patch Set 3 : Forgot to update the test expectations. #

Total comments: 13

Patch Set 4 : Applied suggested changes. #

Patch Set 5 : New function to explicitly resolve align-self. #

Total comments: 6

Patch Set 6 : Adding/updating some comments. #

Patch Set 7 : Fixed build error. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+404 lines, -14 lines) Patch
A LayoutTests/fast/css-grid-layout/grid-align.html View 1 2 3 1 chunk +289 lines, -0 lines 0 comments Download
A + LayoutTests/fast/css-grid-layout/grid-align-expected.txt View 1 2 3 2 chunks +2 lines, -11 lines 0 comments Download
M Source/core/rendering/RenderGrid.h View 1 2 3 4 5 1 chunk +3 lines, -0 lines 0 comments Download
M Source/core/rendering/RenderGrid.cpp View 1 2 3 4 5 6 2 chunks +110 lines, -3 lines 0 comments Download

Messages

Total messages: 16 (0 generated)
jfernandez
5 years, 1 month ago (2014-05-20 23:47:00 UTC) #1
Julien - ping for review
Ojan: you should double-check this change as it's mostly mine. https://codereview.chromium.org/297483005/diff/1/Source/core/rendering/RenderGrid.cpp File Source/core/rendering/RenderGrid.cpp (right): https://codereview.chromium.org/297483005/diff/1/Source/core/rendering/RenderGrid.cpp#newcode1123 ...
5 years, 1 month ago (2014-05-21 14:41:37 UTC) #2
jfernandez
Applied suggested changes and adapted the layout test accordingly. https://codereview.chromium.org/297483005/diff/1/Source/core/rendering/RenderGrid.cpp File Source/core/rendering/RenderGrid.cpp (right): https://codereview.chromium.org/297483005/diff/1/Source/core/rendering/RenderGrid.cpp#newcode1123 Source/core/rendering/RenderGrid.cpp:1123: ...
5 years, 1 month ago (2014-05-23 14:06:48 UTC) #3
jfernandez
Could someone review the last patch, please ?
5 years ago (2014-06-16 09:27:40 UTC) #4
cbiesinger
https://codereview.chromium.org/297483005/diff/40001/Source/core/rendering/RenderGrid.cpp File Source/core/rendering/RenderGrid.cpp (right): https://codereview.chromium.org/297483005/diff/40001/Source/core/rendering/RenderGrid.cpp#newcode1126 Source/core/rendering/RenderGrid.cpp:1126: return startOfRow + std::max<LayoutUnit>(0, endOfRow - startOfRow - child->logicalHeight()) ...
5 years ago (2014-06-17 22:18:31 UTC) #5
jfernandez
Applied suggested changes. https://codereview.chromium.org/297483005/diff/40001/Source/core/rendering/RenderGrid.cpp File Source/core/rendering/RenderGrid.cpp (right): https://codereview.chromium.org/297483005/diff/40001/Source/core/rendering/RenderGrid.cpp#newcode1126 Source/core/rendering/RenderGrid.cpp:1126: return startOfRow + std::max<LayoutUnit>(0, endOfRow - ...
5 years ago (2014-06-20 10:46:00 UTC) #6
jfernandez
Applied suggested changes.
5 years ago (2014-06-20 20:41:06 UTC) #7
cbiesinger
lgtm thanks! https://codereview.chromium.org/297483005/diff/40001/Source/core/rendering/RenderGrid.cpp File Source/core/rendering/RenderGrid.cpp (right): https://codereview.chromium.org/297483005/diff/40001/Source/core/rendering/RenderGrid.cpp#newcode1177 Source/core/rendering/RenderGrid.cpp:1177: case ItemPositionStretch: On 2014/06/20 10:46:00, jfernandez wrote: ...
5 years ago (2014-06-21 00:52:36 UTC) #8
jfernandez
4 years, 12 months ago (2014-06-27 09:07:51 UTC) #9
Julien - ping for review
lgtm https://codereview.chromium.org/297483005/diff/80001/Source/core/rendering/RenderGrid.cpp File Source/core/rendering/RenderGrid.cpp (right): https://codereview.chromium.org/297483005/diff/80001/Source/core/rendering/RenderGrid.cpp#newcode1216 Source/core/rendering/RenderGrid.cpp:1216: } I assume this code is temporary as ...
4 years, 11 months ago (2014-07-10 22:59:59 UTC) #10
jfernandez
Applied suggested changes. https://codereview.chromium.org/297483005/diff/80001/Source/core/rendering/RenderGrid.cpp File Source/core/rendering/RenderGrid.cpp (right): https://codereview.chromium.org/297483005/diff/80001/Source/core/rendering/RenderGrid.cpp#newcode1216 Source/core/rendering/RenderGrid.cpp:1216: } On 2014/07/10 22:59:59, Julien Chaffraix ...
4 years, 11 months ago (2014-07-12 22:29:48 UTC) #11
jfernandez
The CQ bit was checked by jfernandez@igalia.com
4 years, 11 months ago (2014-07-12 22:30:00 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jfernandez@igalia.com/297483005/100001
4 years, 11 months ago (2014-07-12 22:30:17 UTC) #13
jfernandez
The CQ bit was checked by jfernandez@igalia.com
4 years, 11 months ago (2014-07-12 23:02:24 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jfernandez@igalia.com/297483005/120001
4 years, 11 months ago (2014-07-12 23:02:26 UTC) #15
commit-bot: I haz the power
4 years, 11 months ago (2014-07-13 00:07:10 UTC) #16
Message was sent while issue was closed.
Change committed as 178006

Powered by Google App Engine
This is Rietveld 408576698