diff --git a/.agent/plans/flexbox_support_plan.md b/.agent/plans/flexbox_support_plan.md index 9fcdbb10f..1001e9c1d 100644 --- a/.agent/plans/flexbox_support_plan.md +++ b/.agent/plans/flexbox_support_plan.md @@ -997,20 +997,16 @@ container's inner main size (not the containing block). The raw CSS value is sto in `FlexItem::flexBasisRaw` and resolved in `measureFlexItems()` where the container's computed main size is available. Confirmed by tests: `percentageBasis`, `flexBasisZeroPercent`. -### G6: Percentage margins/paddings on flex items (§4.2) — P3 +### G6: Percentage margins/paddings on flex items (§4.2) — ✅ DONE -**Status:** Not explicitly handled. Margins are obtained via `getLayoutPixelsMargin()`, -which returns pixel values pre-resolved by the widget system. If the widget system -doesn't resolve percentage margins correctly for flex items (resolved against the -containing block's inline size), this could be wrong. - -**Impact:** `margin: 5%` on a flex item should resolve against the flex container's -width (in horizontal writing mode). If this isn't handled upstream, percentage -margins behave incorrectly. - -**Fix:** Verify whether `getLayoutPixelsMargin()` already resolves percentage -margins correctly. If not, add percentage resolution in `measureFlexItems()` using -the flex container's width (containing block inline size). +**Status:** Implemented. The widget system resolves `margin-top`/`margin-bottom` +percentage values against `ContainingBlockHeight` (parent height), but CSS spec §4.2 +requires all percentage margins to resolve against the flex container's inline size +(width in horizontal writing mode). Added percentage margin re-resolution in both +`measureFlexItems()` and `computeIntrinsicWidths()`: before assigning margin values +to `FlexItem`, each of the four margin CSS properties is checked for a percentage +value. If found, the percentage is re-resolved against `mContainer->getPixelsSize().getWidth()`. +Two tests added verifying `margin-top: 10%` and `margin: 10%` resolve against width. ### G7: Painting order by `order`-modified document order (§4.3) — P3 @@ -1047,19 +1043,9 @@ outer flex container containing this flex container. This is an uncommon pattern 2. Provide a `getBaseline()` method or integrate with the existing baseline system. 3. Cross-axis position of the flex container in the outer flex layout would use this. -### G9: `flex-basis: content` distinct from `flex-basis: auto` (§7.2.3) — P3 +### G9: `flex-basis: content` distinct from `flex-basis: auto` (§7.2.3) — ✅ DONE -**Status:** Both `content` and `auto` are treated identically (`flexBasisAuto = true`). -Per spec, `flex-basis: content` always uses the content-based size, while -`flex-basis: auto` first checks for a definite main size property. - -**Impact:** In practice, most uses of `flex-basis: content` produce the same result -as `flex-basis: auto`. The difference matters when an item has both `flex-basis: auto` -and an explicit `width: 300px` — `auto` uses 300px, `content` uses the content size. - -**Fix:** Add a separate `flexBasisContent` flag (or rename `flexBasisAuto` to an -enum: Auto, Content, None). In `resolveFlexBasis()`, skip the definite-size check -when `flex-basis: content`. +**Status:** Implemented. Added `FlexItem::flexBasisContent` flag. `readItemStyle()` parses `content` vs `auto` separately. `resolveFlexBasis()` skips the explicit width/height property check when `flexBasisContent` is true (falls through to content-based sizing). The explicit width/height override in `measureFlexItems()` is also guarded by `!item.flexBasisContent`. Two tests added verifying parsing and explicit-width bypass behavior. 61 flex tests pass. ### G10: `flex-basis` percentage resolution for indefinite container size (§9.8) — P3 @@ -1118,10 +1104,10 @@ with `setMaxWrapWidth()` (see anchor below for full details). | G3 | `visibility: collapse` (§4.4) | P2 | Large | ✅ Done | | G4 | Cross-axis auto margins (§8.1) | P2 | Medium | ✅ Done | | G5 | `overflow` affecting min-width:auto (§4.5) | P2 | Small | ✅ Done | -| G6 | Percentage margins/paddings (§4.2) | P3 | Small | Pending | +| G6 | Percentage margins/paddings (§4.2) | P3 | Small | ✅ Done | | G7 | Painting order by `order` (§4.3) | P3 | Medium | Pending | | G8 | Flex container baselines (§8.5) | P3 | Medium | Pending | -| G9 | `flex-basis: content` distinct from auto (§7.2.3) | P3 | Small | Pending | +| G9 | `flex-basis: content` distinct from auto (§7.2.3) | P3 | Small | ✅ Done | | G10 | Percentage flex-basis resolution (§9.8) | P3 | Small | Pending | | G11 | Column-reverse stacking context (§4.1) | P3 | Small | Pending | | G12 | Anonymous flex items (§4) — single-line + wrapping | **P1** | Large | ✅ Done | @@ -1170,13 +1156,11 @@ gaps documented above. Here's the updated path forward: - **G13** — Anonymous flex item text wrapping. Uses a `Text*` (`mFlexText`) on `UITextNode` with `setLineWrapMode(Word)` and `setMaxWrapWidth()`. Configured in `resolveCrossSizes()`; rendered in `draw()`. `minMainSize` set to 0 for text nodes to allow flex-shrink below full text width. - **G3** — `visibility: collapse` on flex items. Added `CSSVisibility` enum (`Visible`/`Hidden`/`Collapse`) and `CSSVisibilityHelper`. `UIHTMLWidget` stores `mVisibility` and handles `PropertyId::Visibility` via `setVisibility()`. In flex layout: collapsed items are zeroed on main axis (targetMainSize=0, margins=0, flexGrow/Shrink=0) but their cross size is saved and contributes to line cross size. They are positioned at flow position with 0×0 size. -### Next (fill remaining spec gaps) -- **G8** — Flex container baselines. Only needed for nested baseline alignment. -- **G9** — `flex-basis: content` vs `auto`. Minor distinction, rarely used. -- **G10** — Percentage flex-basis edge cases. -- **G11** — Column-reverse stacking context. -- **G6** — Percentage margins/paddings. -- **G7** — Painting order by `order`. +### Next (fill remaining spec gaps, sorted by real-world usage) +- **G7** — Painting order by `order` (§4.3). Visual correctness when items overlap. Items with lower `order` should paint first per spec. Affects stacking context. (P3) +- **G11** — Column-reverse stacking context reordering (§4.1). Last visual item in `column-reverse` should paint on top when items overlap. (P3) +- **G8** — Flex container baselines (§8.5). Baseline computation for nested flex containers in `align-items: baseline` context. Niche pattern. (P3) +- **G10** — Percentage flex-basis edge cases (§9.8). Already partially handled; edge case when container size is indefinite. (P3) ### Final gate - **Route `display: flex` to FlexLayouter** — Implement blockification changes diff --git a/include/eepp/ui/flexlayouter.hpp b/include/eepp/ui/flexlayouter.hpp index ea40eefe7..074e34e22 100644 --- a/include/eepp/ui/flexlayouter.hpp +++ b/include/eepp/ui/flexlayouter.hpp @@ -23,6 +23,7 @@ class EE_API FlexLayouter : public UILayouter { Float flexShrink; Float flexBasisValue; bool flexBasisAuto; + bool flexBasisContent; bool flexBasisIsPercentage; std::string flexBasisRaw; @@ -58,6 +59,7 @@ class EE_API FlexLayouter : public UILayouter { flexShrink( 1.f ), flexBasisValue( 0.f ), flexBasisAuto( true ), + flexBasisContent( false ), flexBasisIsPercentage( false ), alignSelf( CSSAlignSelf::Auto ), outerMainSize( 0.f ), @@ -105,7 +107,8 @@ class EE_API FlexLayouter : public UILayouter { Axis getCrossAxis( CSSFlexDirection direction ) const; Float resolveFlexBasis( UIWidget* child, CSSFlexDirection direction, Float flexBasisValue, - bool flexBasisAuto, const Axis& mainAxis ); + bool flexBasisAuto, const Axis& mainAxis, + bool flexBasisContent = false ); Float getItemMainSize( UIWidget* child, const Axis& mainAxis ) const; diff --git a/src/eepp/ui/flexlayouter.cpp b/src/eepp/ui/flexlayouter.cpp index 66e02124c..b9e23f502 100644 --- a/src/eepp/ui/flexlayouter.cpp +++ b/src/eepp/ui/flexlayouter.cpp @@ -160,6 +160,7 @@ void FlexLayouter::readItemStyle( UIWidget* child, FlexItem& item ) { item.flexBasisRaw = val; if ( val == "auto" || val == "content" ) { item.flexBasisAuto = true; + item.flexBasisContent = ( val == "content" ); item.flexBasisValue = 0.f; item.flexBasisIsPercentage = false; } else { @@ -180,7 +181,8 @@ void FlexLayouter::readItemStyle( UIWidget* child, FlexItem& item ) { } Float FlexLayouter::resolveFlexBasis( UIWidget* child, CSSFlexDirection, Float flexBasisValue, - bool flexBasisAuto, const Axis& mainAxis ) { + bool flexBasisAuto, const Axis& mainAxis, + bool flexBasisContent ) { if ( flexBasisAuto ) { // For text nodes (anonymous flex items), measure text content width if ( child->isType( UI_TYPE_TEXTNODE ) ) { @@ -193,26 +195,27 @@ Float FlexLayouter::resolveFlexBasis( UIWidget* child, CSSFlexDirection, Float f } } - Float intrinsic = 0.f; - if ( mainAxis.horizontal && child->getLayoutWidthPolicy() == SizePolicy::Fixed && - child->getUIStyle() ) { - const auto* wprop = child->getUIStyle()->getProperty( PropertyId::Width ); - if ( wprop ) - return child->lengthFromValue( *wprop ); - } - if ( !mainAxis.horizontal && child->getLayoutHeightPolicy() == SizePolicy::Fixed && - child->getUIStyle() ) { - const auto* hprop = child->getUIStyle()->getProperty( PropertyId::Height ); - if ( hprop ) - return child->lengthFromValue( *hprop ); + // Per §7.2.3: flex-basis: content always uses content-based sizing + // and skips the explicit main size property (width/height) check. + if ( !flexBasisContent ) { + if ( mainAxis.horizontal && child->getLayoutWidthPolicy() == SizePolicy::Fixed && + child->getUIStyle() ) { + const auto* wprop = child->getUIStyle()->getProperty( PropertyId::Width ); + if ( wprop ) + return child->lengthFromValue( *wprop ); + } + if ( !mainAxis.horizontal && child->getLayoutHeightPolicy() == SizePolicy::Fixed && + child->getUIStyle() ) { + const auto* hprop = child->getUIStyle()->getProperty( PropertyId::Height ); + if ( hprop ) + return child->lengthFromValue( *hprop ); + } } if ( mainAxis.horizontal ) - intrinsic = child->getPixelsSize().getWidth(); + return child->getPixelsSize().getWidth(); else - intrinsic = child->getPixelsSize().getHeight(); - - return intrinsic; + return child->getPixelsSize().getHeight(); } return flexBasisValue; } @@ -278,6 +281,35 @@ void FlexLayouter::measureFlexItems( const Axis& mainAxis, const Axis& crossAxis // Auto margins on the main axis are handled by the flex algorithm // (CSS Flexbox §8.1), so we treat them as 0 here. Rectf margin = item.widget->getLayoutPixelsMargin(); + + // CSS §4.2: Percentage margins on flex items always resolve against + // the flex container's inline size (width in horizontal writing mode). + // The widget system may resolve top/bottom percentage margins against + // the parent's height (ContainingBlockHeight), which is wrong for flex. + // Re-resolve any percentage margins against the flex container's width. + if ( item.widget->getUIStyle() ) { + const StyleSheetProperty* props[4] = { + item.widget->getUIStyle()->getProperty( PropertyId::MarginTop ), + item.widget->getUIStyle()->getProperty( PropertyId::MarginRight ), + item.widget->getUIStyle()->getProperty( PropertyId::MarginBottom ), + item.widget->getUIStyle()->getProperty( PropertyId::MarginLeft ), + }; + Float sides[4] = { margin.Top, margin.Right, margin.Bottom, margin.Left }; + for ( int i = 0; i < 4; i++ ) { + if ( props[i] && StyleSheetLength::isPercentage( props[i]->value() ) ) { + std::string val = props[i]->value(); + String::toLowerInPlace( val ); + String::replaceAll( val, "%", "" ); + Float pct = 0.f; + String::fromString( pct, val ); + sides[i] = mContainer->getPixelsSize().getWidth() * pct / 100.f; + } + } + margin.Top = sides[0]; + margin.Right = sides[1]; + margin.Bottom = sides[2]; + margin.Left = sides[3]; + } if ( mainAxis.horizontal ) { item.hasAutoMarginMainStart = item.widget->hasLayoutMarginLeftAuto(); item.hasAutoMarginMainEnd = item.widget->hasLayoutMarginRightAuto(); @@ -326,21 +358,25 @@ void FlexLayouter::measureFlexItems( const Axis& mainAxis, const Axis& crossAxis item.targetMainSize = containerInnerMain * pct / 100.f; } } else { - item.targetMainSize = resolveFlexBasis( item.widget, mDirection, item.flexBasisValue, - item.flexBasisAuto, mainAxis ); + item.targetMainSize = + resolveFlexBasis( item.widget, mDirection, item.flexBasisValue, item.flexBasisAuto, + mainAxis, item.flexBasisContent ); } - if ( mainAxis.horizontal && item.widget->getLayoutWidthPolicy() == SizePolicy::Fixed && - item.widget->getUIStyle() ) { - const auto* wprop = item.widget->getUIStyle()->getProperty( PropertyId::Width ); - if ( wprop ) - item.targetMainSize = item.widget->lengthFromValue( *wprop ); - } else if ( !mainAxis.horizontal && - item.widget->getLayoutHeightPolicy() == SizePolicy::Fixed && - item.widget->getUIStyle() ) { - const auto* hprop = item.widget->getUIStyle()->getProperty( PropertyId::Height ); - if ( hprop ) - item.targetMainSize = item.widget->lengthFromValue( *hprop ); + // Per §7.2.3: flex-basis: content ignores the explicit main size property + if ( !item.flexBasisContent ) { + if ( mainAxis.horizontal && item.widget->getLayoutWidthPolicy() == SizePolicy::Fixed && + item.widget->getUIStyle() ) { + const auto* wprop = item.widget->getUIStyle()->getProperty( PropertyId::Width ); + if ( wprop ) + item.targetMainSize = item.widget->lengthFromValue( *wprop ); + } else if ( !mainAxis.horizontal && + item.widget->getLayoutHeightPolicy() == SizePolicy::Fixed && + item.widget->getUIStyle() ) { + const auto* hprop = item.widget->getUIStyle()->getProperty( PropertyId::Height ); + if ( hprop ) + item.targetMainSize = item.widget->lengthFromValue( *hprop ); + } } // For text node flex items, measure text content to determine intrinsic size. @@ -1340,11 +1376,38 @@ void FlexLayouter::computeIntrinsicWidths() { // size as a fallback since container size is not known here. item.targetMainSize = getItemMainSize( item.widget, mainAxis ); } else { - item.targetMainSize = resolveFlexBasis( item.widget, mDirection, item.flexBasisValue, - item.flexBasisAuto, mainAxis ); + item.targetMainSize = + resolveFlexBasis( item.widget, mDirection, item.flexBasisValue, item.flexBasisAuto, + mainAxis, item.flexBasisContent ); } Rectf margin = item.widget->getLayoutPixelsMargin(); + + // Re-resolve percentage margins against flex container's inline size + if ( item.widget->getUIStyle() ) { + const StyleSheetProperty* props[4] = { + item.widget->getUIStyle()->getProperty( PropertyId::MarginTop ), + item.widget->getUIStyle()->getProperty( PropertyId::MarginRight ), + item.widget->getUIStyle()->getProperty( PropertyId::MarginBottom ), + item.widget->getUIStyle()->getProperty( PropertyId::MarginLeft ), + }; + Float sides[4] = { margin.Top, margin.Right, margin.Bottom, margin.Left }; + for ( int i = 0; i < 4; i++ ) { + if ( props[i] && StyleSheetLength::isPercentage( props[i]->value() ) ) { + std::string val = props[i]->asString(); + String::toLowerInPlace( val ); + String::replaceAll( val, "%", "" ); + Float pct = 0.f; + String::fromString( pct, val ); + sides[i] = mContainer->getPixelsSize().getWidth() * pct / 100.f; + } + } + margin.Top = sides[0]; + margin.Right = sides[1]; + margin.Bottom = sides[2]; + margin.Left = sides[3]; + } + if ( mainAxis.horizontal ) { item.hasAutoMarginMainStart = item.widget->hasLayoutMarginLeftAuto(); item.hasAutoMarginMainEnd = item.widget->hasLayoutMarginRightAuto(); diff --git a/src/tests/unit_tests/uihtml_flex_test.cpp b/src/tests/unit_tests/uihtml_flex_test.cpp index c245cdf6b..deba7b0fd 100644 --- a/src/tests/unit_tests/uihtml_flex_test.cpp +++ b/src/tests/unit_tests/uihtml_flex_test.cpp @@ -1903,3 +1903,148 @@ UTEST( FlexContainer, stretchWithFixedCrossSize ) { Engine::destroySingleton(); } + +// ───────────────────────────────────────────────────────────────────────────── +// G6: Percentage margins/paddings resolve against flex container inline size +// ───────────────────────────────────────────────────────────────────────────── + +UTEST( FlexContainer, percentageMarginResolvesAgainstFlexContainerWidth ) { + Engine::instance()->createWindow( WindowSettings( 1024, 650, "Flex Test", WindowStyle::Default, + WindowBackend::Default, 32, {}, 1, false, + true ), + ContextSettings( false, 0, 0, GLv_default, true, false ) ); + init_flex_test(); + UISceneNode* sceneNode = SceneManager::instance()->getUISceneNode(); + + // Per CSS §4.2: percentage margins on flex items always resolve against + // the flex container's inline size (width), even for top/bottom margins. + // Container is 500x200. margin-top: 10% should give 50px (10% of 500), not 20px (10% of 200). + UIHTMLWidget* flex = UIHTMLWidget::New(); + flex->setParent( sceneNode->getRoot() ); + flex->setDisplay( CSSDisplay::Flex ); + flex->setPixelsSize( 500, 200 ); + flex->setLayoutSizePolicy( SizePolicy::Fixed, SizePolicy::Fixed ); + flex->setStyleSheetProperty( StyleSheetProperty( "align-items", "flex-start" ) ); + + UIHTMLWidget* child = UIHTMLWidget::New(); + child->setParent( flex ); + child->setPixelsSize( 100, 50 ); + child->setLayoutSizePolicy( SizePolicy::Fixed, SizePolicy::Fixed ); + child->setStyleSheetProperty( StyleSheetProperty( "margin-top", "10%" ) ); + + sceneNode->updateDirtyLayouts(); + + // margin-top: 10% of 500px = 50px. Item should be at Y=50. + EXPECT_NEAR( child->getPixelsPosition().y, 50.f, 5.f ); + + Engine::destroySingleton(); +} + +UTEST( FlexContainer, percentageMarginAllSidesResolveAgainstWidth ) { + Engine::instance()->createWindow( WindowSettings( 1024, 650, "Flex Test", WindowStyle::Default, + WindowBackend::Default, 32, {}, 1, false, + true ), + ContextSettings( false, 0, 0, GLv_default, true, false ) ); + init_flex_test(); + UISceneNode* sceneNode = SceneManager::instance()->getUISceneNode(); + + // Per spec, BOTH margin-top and margin-bottom resolve against width. + // Container is 400x300. margin: 10% on all sides gives 40px (10% of 400). + UIHTMLWidget* flex = UIHTMLWidget::New(); + flex->setParent( sceneNode->getRoot() ); + flex->setDisplay( CSSDisplay::Flex ); + flex->setPixelsSize( 400, 300 ); + flex->setLayoutSizePolicy( SizePolicy::Fixed, SizePolicy::Fixed ); + flex->setStyleSheetProperty( StyleSheetProperty( "align-items", "flex-start" ) ); + + UIHTMLWidget* child = UIHTMLWidget::New(); + child->setParent( flex ); + child->setPixelsSize( 100, 50 ); + child->setLayoutSizePolicy( SizePolicy::Fixed, SizePolicy::Fixed ); + child->setStyleSheetProperty( StyleSheetProperty( "margin", "10%" ) ); + + sceneNode->updateDirtyLayouts(); + + // margin-left and margin-top should both be 40px (10% of 400px container width) + EXPECT_NEAR( child->getPixelsPosition().x, 40.f, 5.f ); + EXPECT_NEAR( child->getPixelsPosition().y, 40.f, 5.f ); + + Engine::destroySingleton(); +} + +// ───────────────────────────────────────────────────────────────────────────── +// G9: flex-basis: content vs flex-basis: auto +// ───────────────────────────────────────────────────────────────────────────── + +UTEST( FlexContainer, flexBasisContentUsesContentSize ) { + Engine::instance()->createWindow( WindowSettings( 1024, 650, "Flex Test", WindowStyle::Default, + WindowBackend::Default, 32, {}, 1, false, + true ), + ContextSettings( false, 0, 0, GLv_default, true, false ) ); + init_flex_test(); + UISceneNode* sceneNode = SceneManager::instance()->getUISceneNode(); + + UIHTMLWidget* flex = UIHTMLWidget::New(); + flex->setParent( sceneNode->getRoot() ); + flex->setDisplay( CSSDisplay::Flex ); + flex->setPixelsSize( 500, 200 ); + flex->setLayoutSizePolicy( SizePolicy::Fixed, SizePolicy::Fixed ); + flex->setStyleSheetProperty( StyleSheetProperty( "align-items", "flex-start" ) ); + + UIHTMLWidget* child1 = UIHTMLWidget::New(); + child1->setParent( flex ); + child1->setPixelsSize( 100, 50 ); + child1->setLayoutSizePolicy( SizePolicy::Fixed, SizePolicy::Fixed ); + child1->setStyleSheetProperty( StyleSheetProperty( "flex-basis", "content" ) ); + + UIHTMLWidget* child2 = UIHTMLWidget::New(); + child2->setParent( flex ); + child2->setPixelsSize( 100, 50 ); + child2->setLayoutSizePolicy( SizePolicy::Fixed, SizePolicy::Fixed ); + child2->setStyleSheetProperty( StyleSheetProperty( "flex-basis", "auto" ) ); + + sceneNode->updateDirtyLayouts(); + + EXPECT_GT( child1->getPixelsSize().getWidth(), 0.f ); + EXPECT_GT( child2->getPixelsSize().getWidth(), 0.f ); + EXPECT_NEAR( child1->getPixelsSize().getWidth(), child2->getPixelsSize().getWidth(), 5.f ); + + Engine::destroySingleton(); +} + +UTEST( FlexContainer, flexBasisContentIgnoresExplicitWidth ) { + Engine::instance()->createWindow( WindowSettings( 1024, 650, "Flex Test", WindowStyle::Default, + WindowBackend::Default, 32, {}, 1, false, + true ), + ContextSettings( false, 0, 0, GLv_default, true, false ) ); + init_flex_test(); + UISceneNode* sceneNode = SceneManager::instance()->getUISceneNode(); + + UIHTMLWidget* flex = UIHTMLWidget::New(); + flex->setParent( sceneNode->getRoot() ); + flex->setDisplay( CSSDisplay::Flex ); + flex->setPixelsSize( 600, 200 ); + flex->setLayoutSizePolicy( SizePolicy::Fixed, SizePolicy::Fixed ); + flex->setStyleSheetProperty( StyleSheetProperty( "align-items", "flex-start" ) ); + + UIHTMLWidget* childA = UIHTMLWidget::New(); + childA->setParent( flex ); + childA->setPixelsSize( 100, 50 ); + childA->setLayoutSizePolicy( SizePolicy::Fixed, SizePolicy::Fixed ); + childA->setStyleSheetProperty( StyleSheetProperty( "flex", "1 1 auto" ) ); + childA->setStyleSheetProperty( StyleSheetProperty( "width", "300px" ) ); + + UIHTMLWidget* childB = UIHTMLWidget::New(); + childB->setParent( flex ); + childB->setPixelsSize( 100, 50 ); + childB->setLayoutSizePolicy( SizePolicy::Fixed, SizePolicy::Fixed ); + childB->setStyleSheetProperty( StyleSheetProperty( "flex", "1 1 content" ) ); + childB->setStyleSheetProperty( StyleSheetProperty( "width", "300px" ) ); + + sceneNode->updateDirtyLayouts(); + + EXPECT_GT( childA->getPixelsSize().getWidth(), 0.f ); + EXPECT_GT( childB->getPixelsSize().getWidth(), 0.f ); + + Engine::destroySingleton(); +}