CSS Grid 1/9: Grid style types and public API#1893
CSS Grid 1/9: Grid style types and public API#1893intergalacticspacehighway wants to merge 4 commits into
Conversation
Summary: Add the foundational data types, enums, style properties, and C API for expressing CSS Grid layouts in Yoga. Includes: - Grid style types (GridLine.h, GridTrack.h, GridTrackType.h) - Updated enums (Display::Grid, Align::Start/End, Justify::Auto/Stretch/Start/End) - Grid event (LayoutPassReason::kGridLayout) - Style property accessors and member variables - Public C API (YGGridTrackList.h/cpp, YGNodeStyle grid setters/getters) - Layout helpers updated for new enum values (Align.h, AbsoluteLayout.cpp, CalculateLayout.cpp/h partial) - Node.h: relativePosition made public - React Native mirror of all C++ changes Differential Revision: D93946262
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
| /** | ||
| * Set the grid-template-rows property on a node. | ||
| */ | ||
| YG_EXPORT void YGNodeStyleSetGridTemplateRows( | ||
| YGNodeRef node, | ||
| YGGridTrackListRef trackList); | ||
|
|
||
| /** | ||
| * Set the grid-template-columns property on a node. | ||
| */ | ||
| YG_EXPORT void YGNodeStyleSetGridTemplateColumns( | ||
| YGNodeRef node, | ||
| YGGridTrackListRef trackList); | ||
|
|
||
| /** | ||
| * Set the grid-auto-rows property on a node. | ||
| */ | ||
| YG_EXPORT void YGNodeStyleSetGridAutoRows( | ||
| YGNodeRef node, | ||
| YGGridTrackListRef trackList); | ||
|
|
||
| /** | ||
| * Set the grid-auto-columns property on a node. | ||
| */ | ||
| YG_EXPORT void YGNodeStyleSetGridAutoColumns( | ||
| YGNodeRef node, | ||
| YGGridTrackListRef trackList); | ||
|
|
There was a problem hiding this comment.
These should probably go in YGNodeStyle.h
|
|
||
| // Internal representation of a grid track value | ||
| struct YGGridTrackValue { | ||
| enum class Type { Points, Percent, Fr, Auto, MinMax }; |
There was a problem hiding this comment.
What does Auto mean here?
There was a problem hiding this comment.
It's one of the track sizes https://www.w3.org/TR/css-grid-1/#track-sizes. e.g. grid-template-columns: "auto 100px 1fr"
| */ | ||
| YG_EXPORT void YGNodeStyleSetGridTemplateRows( | ||
| YGNodeRef node, | ||
| YGGridTrackListRef trackList); |
There was a problem hiding this comment.
It's not clear from me in this API, who takes ownership of the TrackList? Does the Yoga node free it at some point, or must the user?
There was a problem hiding this comment.
User would need to free it.
auto gridTemplateColumns = YGGridTrackListCreate();
YGGridTrackListAddTrack(gridTemplateColumns, YGPoints(40));
YGGridTrackListAddTrack(gridTemplateColumns, YGPoints(40));
// free gridTemplateColumns
YGGridTrackListFree(gridTemplateColumns);it was inspired from YGConfigNew and YGNodeNewWithConfig implementation. Maybe YGGridTrackListCreate could be renamed to YGGridTrackListNew?
| delete minValue; | ||
| delete maxValue; |
There was a problem hiding this comment.
Can we use RAII-style handles where we can get away with it for internals?
| for (auto* track : tracks) { | ||
| delete track; | ||
| } |
There was a problem hiding this comment.
See other comment about RAII. If we want the list to have assumed ownership, would be best to represent internally as unique_ptr, so move checking can catch any issues.
| } | ||
| }; | ||
|
|
||
| // Internal representation of a grid track list |
There was a problem hiding this comment.
Is this comment accurate?
From what I could tell, internally, we use std::vector<GridTrack>, and this type is more for the public API, before internal version?
There was a problem hiding this comment.
Yes, it's inaccurate, removed.
| /** | ||
| * Create a grid track value with a points (px) length. | ||
| */ | ||
| YG_EXPORT YGGridTrackValueRef YGPoints(float points); | ||
|
|
||
| /** | ||
| * Create a grid track value with a percentage length. | ||
| */ | ||
| YG_EXPORT YGGridTrackValueRef YGPercent(float percent); | ||
|
|
||
| /** | ||
| * Create a grid track value with a flexible (fr) length. | ||
| */ | ||
| YG_EXPORT YGGridTrackValueRef YGFr(float fr); | ||
|
|
||
| /** | ||
| * Create a grid track value with auto sizing. | ||
| */ | ||
| YG_EXPORT YGGridTrackValueRef YGAuto(void); |
There was a problem hiding this comment.
Since these aren't namespaced, it means that we would have global YGPoints, etc, which is grid specific. If Grid specific units, would be better to include in API naming.
There was a problem hiding this comment.
Updated, will also handle it in gentest-cpp.js
| YGGridTrackList(YGGridTrackList&&) = delete; | ||
| YGGridTrackList& operator=(YGGridTrackList&&) = delete; | ||
|
|
||
| GridTrackList toGridTrackList() const { |
There was a problem hiding this comment.
Trying to think if there is a way to avoid doing the extra allocations for setting style, vs internal.
Can the operations to add track size to the style on a Node, add directly to underlying yoga::style vector, instead of exposing heap allocated list as the primitive? Or do you think exposing it will be helpful, e.g. for RN integration?
There was a problem hiding this comment.
Revisited it here. I added this API when testing gentest fixtures but didn't revisit later 😅. New changes does not require manual free or extra allocation and directly updates internal vector.
YGNodeStyleSetGridTemplateColumnsCount(node, 3);
YGNodeStyleSetGridTemplateColumn(node, 0, YGGridTrackTypePoints, 40);
YGNodeStyleSetGridTemplateColumn(node, 1, YGGridTrackTypePercent, 20);
YGNodeStyleSetGridTemplateColumnMinMax(minmax, 2, YGGridTrackTypePoints, 20, YGGridTrackTypePoints, 40);| } | ||
|
|
||
| void YGGridTrackListFree(YGGridTrackListRef list) { | ||
| delete list; |
There was a problem hiding this comment.
Subtle bug here, we don't have concrete type of what we are deleting, beyond the marker interface, and would not call destructors.
| delete list; | |
| delete static_cast<YGGridTrackList*>(list); |
In other places, we provide overload of resolveRef to convert from C ABI type, to concrete C++ internal type, but it's just a convenience for the static_cast in less characters.
There was a problem hiding this comment.
Thanks. Just removed this API in the new changes 😅
| bool operator==(const GridLine& other) const { | ||
| return type == other.type && integer == other.integer; | ||
| } | ||
|
|
||
| bool operator!=(const GridLine& other) const { | ||
| return !(*this == other); | ||
| } |
There was a problem hiding this comment.
This will generate everything needed.
| bool operator==(const GridLine& other) const { | |
| return type == other.type && integer == other.integer; | |
| } | |
| bool operator!=(const GridLine& other) const { | |
| return !(*this == other); | |
| } | |
| bool operator==(const GridLine& other) const = default; |
| // Line position (1, 2, -1, -2, etc) | ||
| int32_t integer; | ||
|
|
||
| static GridLine auto_() { |
There was a problem hiding this comment.
nit: I think all of these can be constexpr
| bool operator==(const GridTrackSize& other) const { | ||
| return minSizingFunction == other.minSizingFunction && | ||
| maxSizingFunction == other.maxSizingFunction; | ||
| } | ||
|
|
||
| bool operator!=(const GridTrackSize& other) const { | ||
| return !(*this == other); | ||
| } |
There was a problem hiding this comment.
| bool operator==(const GridTrackSize& other) const { | |
| return minSizingFunction == other.minSizingFunction && | |
| maxSizingFunction == other.maxSizingFunction; | |
| } | |
| bool operator!=(const GridTrackSize& other) const { | |
| return !(*this == other); | |
| } | |
| bool operator==(const GridTrackSize& other) const = default; |
| bool infinitelyGrowable = false; | ||
|
|
||
| // Static factory methods for common cases | ||
| static GridTrackSize auto_() { |
There was a problem hiding this comment.
I think the reserved keyword is how we ended up with ofAuto in a lot of places 😅
| // These are used in the grid layout algorithm when distributing spaces among | ||
| // tracks | ||
| // TODO: maybe move them to TrackSizing since these are track states | ||
| float baseSize = 0.0f; | ||
| float growthLimit = 0.0f; | ||
| bool infinitelyGrowable = false; |
There was a problem hiding this comment.
If GridTrack is on the yoga::Style, we definitely want to avoid keep layout state on it.
There was a problem hiding this comment.
Yes, I have a version without it. Can i do it in the algorithm PR when we merge this one?
5a92823 to
e614f12
Compare
| YGNodeRef node, | ||
| YGGridTrackListRef trackList); | ||
| YG_EXPORT void YGNodeStyleSetGridTemplateColumns( | ||
| uint32_t count); |
There was a problem hiding this comment.
The rest of the public API exposes size_t for indices
|
@NickGerleman has imported this pull request. If you are a Meta employee, you can view this in D94867121. |
Summary: - Add foundational data types, enums, style properties, and C API for expressing CSS Grid layouts - Includes `GridLine.h`, `GridTrack.h`, `GridTrackType.h`, `Display::Grid`, new alignment enums (`Align::Start/End`, `Justify::Auto/Stretch/Start/End`) - C API: `YGGridTrackList.h/cpp`, `YGNodeStyle` grid setters/getters - Layout helper updates, `Node.h` `relativePosition` made public Split from facebook/yoga#1865. Part of the CSS Grid implementation series. X-link: facebook/yoga#1893 Differential Revision: D94867121 Pulled By: NickGerleman
|
@NickGerleman merged this pull request in 0752485. |
Summary: Pull Request resolved: #55876 - Add foundational data types, enums, style properties, and C API for expressing CSS Grid layouts - Includes `GridLine.h`, `GridTrack.h`, `GridTrackType.h`, `Display::Grid`, new alignment enums (`Align::Start/End`, `Justify::Auto/Stretch/Start/End`) - C API: `YGGridTrackList.h/cpp`, `YGNodeStyle` grid setters/getters - Layout helper updates, `Node.h` `relativePosition` made public Split from facebook/yoga#1865. Part of the CSS Grid implementation series. Changelog: [Internal] X-link: facebook/yoga#1893 Reviewed By: sammy-SC Differential Revision: D94867121 Pulled By: NickGerleman fbshipit-source-id: adcb6d107cdd782550a614f87f84cff3ecefd806
Summary: Pull Request resolved: facebook#55876 - Add foundational data types, enums, style properties, and C API for expressing CSS Grid layouts - Includes `GridLine.h`, `GridTrack.h`, `GridTrackType.h`, `Display::Grid`, new alignment enums (`Align::Start/End`, `Justify::Auto/Stretch/Start/End`) - C API: `YGGridTrackList.h/cpp`, `YGNodeStyle` grid setters/getters - Layout helper updates, `Node.h` `relativePosition` made public Split from facebook/yoga#1865. Part of the CSS Grid implementation series. Changelog: [Internal] X-link: facebook/yoga#1893 Reviewed By: sammy-SC Differential Revision: D94867121 Pulled By: NickGerleman fbshipit-source-id: adcb6d107cdd782550a614f87f84cff3ecefd806
Summary
GridLine.h,GridTrack.h,GridTrackType.h,Display::Grid, new alignment enums (Align::Start/End,Justify::Auto/Stretch/Start/End)YGGridTrackList.h/cpp,YGNodeStylegrid setters/gettersNode.hrelativePositionmade publicSplit from #1865. Part of the CSS Grid implementation series.