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

Issue 433153003: [CSS Grid Layout] Don't resolve align-self and justify-self properties (Closed)

Created:
5 years ago by jfernandez
Modified:
4 years ago
CC:
darktears, apavlov+blink_chromium.org, blink-reviews, blink-reviews-css, blink-reviews-rendering, cbiesinger, dglazkov+blink, eae+blinkwatch, ed+blinkwatch_opera.com, jchaffraix+rendering, leviw+renderwatch, pdr., rwlbuis, rune+blink, zoltan1
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Project:
blink
Visibility:
Public.

Description

[CSS Grid Layout] Don't resolve align-self and justify-self properties during css cascade, but during layout. There are several reasons why we can't resolve the align-self and justify-self; anonymous renders and wrappers are not part of the css cascade, for instance. There are quite many renders that are implemented using RenderFlexibleBox but not using the proper display, so they might not want the "stretch" default behaviour. Changes in the align-items or justify-items during the layout would need a style recalculation, which is not desirable. This makes unavoidable to resolve alignment "auto" values during the layout. BUG=249451, 376823

Patch Set 1 #

Total comments: 17

Patch Set 2 : Applied suggested comments. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+40 lines, -42 lines) Patch
M LayoutTests/css3/flexbox/css-properties.html View 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/css3/flexbox/css-properties-expected.txt View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/css/CSSComputedStyleDeclaration.cpp View 1 4 chunks +24 lines, -14 lines 0 comments Download
M Source/core/css/resolver/StyleAdjuster.cpp View 1 1 chunk +10 lines, -24 lines 0 comments Download
M Source/core/rendering/RenderFlexibleBox.cpp View 1 2 chunks +4 lines, -2 lines 0 comments Download

Messages

Total messages: 12 (0 generated)
jfernandez
This patch is a complement of issue 363133003. I've thought it was more elegant, since ...
5 years ago (2014-08-02 00:25:19 UTC) #1
Julien - ping for review
On 2014/08/02 at 00:25:19, jfernandez wrote: > This patch is a complement of issue 363133003. ...
5 years ago (2014-08-04 16:33:04 UTC) #2
jfernandez
On 2014/08/04 16:33:04, Julien Chaffraix - PST wrote: > On 2014/08/02 at 00:25:19, jfernandez wrote: ...
5 years ago (2014-08-18 22:05:08 UTC) #3
esprehn
https://codereview.chromium.org/433153003/diff/1/Source/core/css/CSSComputedStyleDeclaration.cpp File Source/core/css/CSSComputedStyleDeclaration.cpp (right): https://codereview.chromium.org/433153003/diff/1/Source/core/css/CSSComputedStyleDeclaration.cpp#newcode1830 Source/core/css/CSSComputedStyleDeclaration.cpp:1830: overflow = parent->computedStyle()->alignItemsOverflowAlignment(); This seems like code duplication, also ...
5 years ago (2014-08-19 00:56:33 UTC) #4
jfernandez
Thanks for the review; replied with comments and some questions. https://codereview.chromium.org/433153003/diff/1/Source/core/css/CSSComputedStyleDeclaration.cpp File Source/core/css/CSSComputedStyleDeclaration.cpp (right): https://codereview.chromium.org/433153003/diff/1/Source/core/css/CSSComputedStyleDeclaration.cpp#newcode1830 ...
5 years ago (2014-08-19 21:00:12 UTC) #5
jfernandez
Forgot to reply to one comment. https://codereview.chromium.org/433153003/diff/1/Source/core/rendering/RenderFlexibleBox.cpp File Source/core/rendering/RenderFlexibleBox.cpp (right): https://codereview.chromium.org/433153003/diff/1/Source/core/rendering/RenderFlexibleBox.cpp#newcode199 Source/core/rendering/RenderFlexibleBox.cpp:199: align = parentStyle->alignItems(); ...
5 years ago (2014-08-19 21:39:18 UTC) #6
jfernandez
Forgot to reply to one comment.
5 years ago (2014-08-19 21:39:21 UTC) #7
jfernandez
esprehn, julien could you give me some feeadback about my comments ? Thanks.
4 years, 11 months ago (2014-09-03 20:53:02 UTC) #8
esprehn
https://codereview.chromium.org/433153003/diff/1/Source/core/css/CSSComputedStyleDeclaration.cpp File Source/core/css/CSSComputedStyleDeclaration.cpp (right): https://codereview.chromium.org/433153003/diff/1/Source/core/css/CSSComputedStyleDeclaration.cpp#newcode1830 Source/core/css/CSSComputedStyleDeclaration.cpp:1830: overflow = parent->computedStyle()->alignItemsOverflowAlignment(); On 2014/08/19 at 21:00:12, jfernandez wrote: ...
4 years, 11 months ago (2014-09-03 21:45:28 UTC) #9
jfernandez
Replied. https://codereview.chromium.org/433153003/diff/1/Source/core/css/CSSComputedStyleDeclaration.cpp File Source/core/css/CSSComputedStyleDeclaration.cpp (right): https://codereview.chromium.org/433153003/diff/1/Source/core/css/CSSComputedStyleDeclaration.cpp#newcode1830 Source/core/css/CSSComputedStyleDeclaration.cpp:1830: overflow = parent->computedStyle()->alignItemsOverflowAlignment(); On 2014/09/03 21:45:28, esprehn wrote: ...
4 years, 11 months ago (2014-09-03 22:27:49 UTC) #10
jfernandez
Patch rebased and applied suggested changes. Still, some of the open issues of the previous ...
4 years, 11 months ago (2014-09-04 17:00:22 UTC) #11
jfernandez
4 years, 11 months ago (2014-09-23 13:50:06 UTC) #12
Gentle reminder, thanks.

Powered by Google App Engine
This is Rietveld 408576698