Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add masonry as valid value to grid-template-rows/grid-template-columns. #5517

Conversation

sammygill
Copy link
Contributor

@sammygill sammygill commented Oct 18, 2022

ec92864

Add masonry as valid value to grid-template-rows/grid-template-columns.
https://bugs.webkit.org/show_bug.cgi?id=246541
rdar://101188057

Reviewed by Antti Koivisto.

This patch adds a CSS Masonry Layout flag and adds the 'masonry'
value as a valid value for the grid-template-rows and
grid-template-columns properties. A new CSSMasonryValue has been added
to represent this new value.

When we parse the grid-template-rows/columns property, we check to
see if the token is of CSSValueMasonry and consume it as an ident if it
is since the value of the syntax is straightforward.

When we build the track list, we check the ID of the primitive value
to see if it also matches CSSMasonryValue. In this scenario a new
masonry entry is added to the track list so that the serialization can
later set m_masonryRows or m_masonryColumns in StyleGridData correctly.

* LayoutTests/imported/w3c/web-platform-tests/css/css-grid/masonry/tentative/masonry-grid-template-columns-computed-withcontent-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-grid/masonry/tentative/masonry-parsing-expected.txt:
* Source/WTF/Scripts/Preferences/WebPreferencesExperimental.yaml:
* Source/WebCore/animation/CSSPropertyAnimation.cpp:
(WebCore::canInterpolate):
(WebCore::blendFunc):
* Source/WebCore/css/CSSValueKeywords.in:
* Source/WebCore/css/ComputedStyleExtractor.cpp:
(WebCore::valueForGridTrackListi):
If the track list is empty and the value for the track is not subgrid,
then we check to see if it is set to masonry.

* Source/WebCore/css/parser/CSSParserContext.cpp:
(WebCore::operator==):
(WebCore::add):
* Source/WebCore/css/parser/CSSParserContext.h:
* Source/WebCore/css/parser/CSSPropertyParser.cpp:
(WebCore::consumeGridTemplatesRowsOrColumns):
Since 'masonry' is the only value that will trigger masonry layout and
there is no other form for the syntax, we can just consume the token
as an ident. However, later on when we actually build the track list,
we will need to check whether the value is a CSSPrimitive and compare
the ID of it.

* Source/WebCore/rendering/style/RenderStyle.h:
(WebCore::RenderStyle::gridMasonryRows const):
(WebCore::RenderStyle::gridMasonryColumns const):
* Source/WebCore/rendering/style/StyleGridData.cpp:
(WebCore::StyleGridData::StyleGridData):
(WebCore::StyleGridData::setRows):
(WebCore::StyleGridData::setColumns):
(WebCore::StyleGridData::computeCachedTrackData):
(WebCore::operator<<):
* Source/WebCore/rendering/style/StyleGridData.h:
(WebCore::GridTrackEntryMasonry::operator== const):
(WebCore::StyleGridData::masonryRows const):
(WebCore::StyleGridData::masonryColumns const):
* Source/WebCore/style/StyleBuilderConverter.h:
(WebCore::Style::BuilderConverter::createGridTrackList):

Canonical link: https://commits.webkit.org/255888@main

5216a53

Misc iOS, tvOS & watchOS macOS Linux Windows
βœ… πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac βœ… πŸ›  wpe βœ… πŸ›  πŸ§ͺ win
βœ… πŸ§ͺ bindings βœ… πŸ›  ios-sim βœ… πŸ›  mac-debug βœ… πŸ›  gtk βœ… πŸ›  wincairo
βœ… πŸ§ͺ webkitperl βœ… πŸ§ͺ ios-wk2 βœ… πŸ›  mac-AS-debug βœ… πŸ§ͺ gtk-wk2
βœ… πŸ§ͺ api-ios βœ… πŸ§ͺ api-mac βœ… πŸ§ͺ api-gtk
βœ… πŸ›  πŸ§ͺ jsc βœ… πŸ›  tv βœ… πŸ§ͺ mac-wk1 βœ… πŸ›  jsc-armv7
βœ… πŸ›  tv-sim βœ… πŸ§ͺ mac-wk2 βœ… πŸ§ͺ jsc-armv7-tests
βœ… πŸ›  πŸ§ͺ merge βœ… πŸ›  watch βœ… πŸ§ͺ mac-AS-debug-wk2 βœ… πŸ›  jsc-mips
βœ… πŸ›  watch-sim βœ… πŸ§ͺ mac-wk2-stress βœ… πŸ§ͺ jsc-mips-tests

@sammygill sammygill self-assigned this Oct 18, 2022
@sammygill sammygill added CSS Cascading Style Sheets implementation WebKit Nightly Build labels Oct 18, 2022
@@ -168,6 +177,9 @@ class StyleGridData : public RefCounted<StyleGridData> {
bool m_subgridRows;
bool m_subgridColumns;

bool m_masonryRows;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be worth combining this into m_subgridRows with a tri-state enum (normal, subgrid, masonry), so that the invalid state of both masonry+subgrid can't be represented.

@sammygill sammygill force-pushed the eng/css-grid-Add-masonry-flag branch from 75eeb8b to 0b1dee3 Compare October 19, 2022 21:55
Comment on lines 126 to 127
const bool& masonryRows() const { return m_masonryRows; }
const bool& masonryColumns() const { return m_masonryColumns; }
Copy link
Contributor

@anttijk anttijk Oct 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do these variables need to be added to operator==?

Why do these function (and some earlier ones) return references instead of just bools?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes they probably should be. Added!

These did not need to so I went ahead and changed the methods, but I'm not familiar with the ones above.

@sammygill sammygill force-pushed the eng/css-grid-Add-masonry-flag branch from 0b1dee3 to 5216a53 Compare October 21, 2022 17:37
@sammygill sammygill added the merge-queue Applied to send a pull request to merge-queue label Oct 23, 2022
https://bugs.webkit.org/show_bug.cgi?id=246541
rdar://101188057

Reviewed by Antti Koivisto.

This patch adds a CSS Masonry Layout flag and adds the 'masonry'
value as a valid value for the grid-template-rows and
grid-template-columns properties. A new CSSMasonryValue has been added
to represent this new value.

When we parse the grid-template-rows/columns property, we check to
see if the token is of CSSValueMasonry and consume it as an ident if it
is since the value of the syntax is straightforward.

When we build the track list, we check the ID of the primitive value
to see if it also matches CSSMasonryValue. In this scenario a new
masonry entry is added to the track list so that the serialization can
later set m_masonryRows or m_masonryColumns in StyleGridData correctly.

* LayoutTests/imported/w3c/web-platform-tests/css/css-grid/masonry/tentative/masonry-grid-template-columns-computed-withcontent-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-grid/masonry/tentative/masonry-parsing-expected.txt:
* Source/WTF/Scripts/Preferences/WebPreferencesExperimental.yaml:
* Source/WebCore/animation/CSSPropertyAnimation.cpp:
(WebCore::canInterpolate):
(WebCore::blendFunc):
* Source/WebCore/css/CSSValueKeywords.in:
* Source/WebCore/css/ComputedStyleExtractor.cpp:
(WebCore::valueForGridTrackListi):
If the track list is empty and the value for the track is not subgrid,
then we check to see if it is set to masonry.

* Source/WebCore/css/parser/CSSParserContext.cpp:
(WebCore::operator==):
(WebCore::add):
* Source/WebCore/css/parser/CSSParserContext.h:
* Source/WebCore/css/parser/CSSPropertyParser.cpp:
(WebCore::consumeGridTemplatesRowsOrColumns):
Since 'masonry' is the only value that will trigger masonry layout and
there is no other form for the syntax, we can just consume the token
as an ident. However, later on when we actually build the track list,
we will need to check whether the value is a CSSPrimitive and compare
the ID of it.

* Source/WebCore/rendering/style/RenderStyle.h:
(WebCore::RenderStyle::gridMasonryRows const):
(WebCore::RenderStyle::gridMasonryColumns const):
* Source/WebCore/rendering/style/StyleGridData.cpp:
(WebCore::StyleGridData::StyleGridData):
(WebCore::StyleGridData::setRows):
(WebCore::StyleGridData::setColumns):
(WebCore::StyleGridData::computeCachedTrackData):
(WebCore::operator<<):
* Source/WebCore/rendering/style/StyleGridData.h:
(WebCore::GridTrackEntryMasonry::operator== const):
(WebCore::StyleGridData::masonryRows const):
(WebCore::StyleGridData::masonryColumns const):
* Source/WebCore/style/StyleBuilderConverter.h:
(WebCore::Style::BuilderConverter::createGridTrackList):

Canonical link: https://commits.webkit.org/255888@main
@webkit-commit-queue
Copy link
Collaborator

Committed 255888@main (ec92864): https://commits.webkit.org/255888@main

Reviewed commits have been landed. Closing PR #5517 and removing active labels.

@webkit-early-warning-system webkit-early-warning-system merged commit ec92864 into WebKit:main Oct 23, 2022
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Oct 23, 2022
@sammygill sammygill deleted the eng/css-grid-Add-masonry-flag branch November 30, 2022 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CSS Cascading Style Sheets implementation
Projects
None yet
6 participants