From afa15f70c09d30d5b44da0876ef43fe86cbb0e39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mart=C3=ADn=20Lucas=20Golini?= Date: Tue, 2 Jun 2026 00:43:30 -0300 Subject: [PATCH] Grid layout fixes. --- .agent/plans/grid_layout_support_plan.md | 11 +- .ecode/project_build.json | 2 +- .../assets/html/grid_newsblur_reduced.html | 451 ++++++++++++++++++ src/eepp/ui/flexlayouter.cpp | 16 + src/eepp/ui/gridlayouter.cpp | 103 +++- src/tests/unit_tests/uihtml_grid_test.cpp | 38 ++ 6 files changed, 603 insertions(+), 18 deletions(-) create mode 100644 bin/unit_tests/assets/html/grid_newsblur_reduced.html diff --git a/.agent/plans/grid_layout_support_plan.md b/.agent/plans/grid_layout_support_plan.md index c20517f93..d1cc3632f 100644 --- a/.agent/plans/grid_layout_support_plan.md +++ b/.agent/plans/grid_layout_support_plan.md @@ -910,9 +910,16 @@ Do not checkpoint a failing phase as complete. | 41 | I5: Negative `contentBoxSize` now clamped to 0 | (implicitly tested) | | 42 | L5: `mPacking` converted to RAII guard (exception-safe) | (implicitly tested) | | 43 | I4: Span decoupled from `resolvedEnd` — explicit `columnSpan`/`rowSpan` fields | (implicitly tested) | +| 44 | PL7: Auto-placement cursor not advancing past placed items (`cursorCol = c+sw`) | `GridContainer.percentHeightWithAutoParentFallsBackToContent` | +| 45 | PL8: Auto-placement column/row limit growing during iteration (captured `colLimit`/`rowLimit`) | `GridContainer.percentHeightWithAutoParentFallsBackToContent` | +| 46 | L6: `height:100%` / `width:100%` on grid with auto parent → fall back to content-based size | `GridContainer.percentHeightWithAutoParentFallsBackToContent` | +| 47 | TS8: `1fr` tracks get `baseSize=0` when container has indefinite width → size from item intrinsic widths | `GridContainer.frTracksWithPercentWidthAndHeight` | +| 48 | L7: `width:100%`/`height:100%` resolved against small intermediate ancestor → scan all ancestors + scene node for max size | `GridContainer.frTracksWithPercentWidthAndHeight` | +| 49 | L8: Percentage fallback ran AFTER track sizing → rows sized with 0 height. Re-run sizing loop after fallback adjusts container size. | `GridContainer.frTracksWithPercentWidthAndHeight` | +| 50 | L9: `height:100%` resolved against viewport instead of auto → use content-based height (CSS spec: % height on auto parent = auto) | `GridContainer.newsblurReducedGrid` | ### Stats -- **Total bugs found**: 43 -- **Fixed**: 43 +- **Total bugs found**: 50 +- **Fixed**: 50 - **By severity**: 0 remaining diff --git a/.ecode/project_build.json b/.ecode/project_build.json index 185197348..c0b12c260 100644 --- a/.ecode/project_build.json +++ b/.ecode/project_build.json @@ -377,7 +377,7 @@ "working_dir": "${project_root}/bin" }, { - "args": "-c system --hn-dark", + "args": "-d1 /home/downloads/files/svn/eepp/bin/unit_tests/assets/html/grid_newsblur_reduced.html", "command": "${project_root}/bin/eepp-ui-html-debug", "name": "eepp-ui-html-debug", "working_dir": "${project_root}/bin" diff --git a/bin/unit_tests/assets/html/grid_newsblur_reduced.html b/bin/unit_tests/assets/html/grid_newsblur_reduced.html new file mode 100644 index 000000000..87eb421d4 --- /dev/null +++ b/bin/unit_tests/assets/html/grid_newsblur_reduced.html @@ -0,0 +1,451 @@ + + + + + + +
+
+
+
+
+
+

Read Your Way

+
+ + +
+
Real-time RSS
+
+ Stories are pushed directly to you, so you can read news + as it comes in +
+
+ + + +
+
Original Site
+
+ Read the content in context, the way it was meant to be + seen +
+
+ + + +
+
Read the Full Story
+
+ The original story from truncated RSS feeds is + seamlessly expanded +
+
+ + + +
+
+ Grid, List, Split, Magazine +
+
+ Every site has its own story layout, so you can mix and + match +
+
+ + + +
+
Dark Mode
+
+ Easy on the eyes and built into the web, iOS, and + Android +
+
+ + + +
+
River of News
+
+ Read all your feeds by folder in one scrollable stream +
+
+ + + +
+
Keyboard Shortcuts
+
+ Navigate everything without touching the mouse +
+
+ +
+
+ +
+

Stay Smart

+
+ + +
+
Intelligence Training
+
+ Hide the stories you don't like and highlight the + stories you do +
+
+ + + +
+
Full Text Search
+
+ Quickly find stories across all of your subscriptions +
+
+ + + +
+
Saved Searches
+
+ Regularly used searches are conveniently given their own + feeds +
+
+ + + +
+
Ask AI
+
+ Ask questions about any story and get answers powered by + AI +
+
+ + + +
+
Daily Briefing
+
+ A summary of your top stories, customizable and + delivered every day +
+
+ + + +
+
Track Changes
+
+ See how a story evolved since it was first published +
+
+ + + +
+
Notifications
+
+ Get alerted the moment a feed publishes, on web, iOS, or + Android +
+
+ +
+
+ +
+

Share & Organize

+
+ + +
+
Shared Stories
+
+ Reading news is better with friends. Share stories on + your public blurblog +
+
+ + + +
+
Blurblog Privacy
+
+ Share stories with the world or only with your friends +
+
+ + + +
+
Story Tagging
+
+ Save stories with custom tags for fast references +
+
+ + + +
+
Email Newsletters
+
+ Read your email newsletters where they belong, in a news + reader +
+
+ +
+
+ +
+

Works Everywhere

+
+ + +
+
Third-party Apps
+
+ Supports Reeder, ReadKit, Unread, and loads more +
+
+ + + +
+
IFTTT & Zapier
+
+ Hook NewsBlur up to nearly every service on the web +
+
+ + + +
+
YouTube
+
+ Even sites that don't publish RSS feeds can be followed +
+
+ + + +
+
Web Feeds
+
+ A creative feed for any website, even without RSS +
+
+ +
+
+
+
+
+
+
+ + diff --git a/src/eepp/ui/flexlayouter.cpp b/src/eepp/ui/flexlayouter.cpp index fd674f59c..d5a963da1 100644 --- a/src/eepp/ui/flexlayouter.cpp +++ b/src/eepp/ui/flexlayouter.cpp @@ -268,6 +268,22 @@ void FlexLayouter::measureFlexItems( const Axis& mainAxis, const Axis& crossAxis item.widget->setLayoutWidthPolicy( SizePolicy::WrapContent ); else item.widget->setLayoutHeightPolicy( SizePolicy::WrapContent ); + + // Set the item's internal size to the container's main size, + // so the block layouter's `else` branch (non-WrapContent) + // wraps text at the full container width. The flex-shrink + // step will then proportionally reduce the item to its final + // size, giving the expected "fill remaining space" behaviour. + Float containerMainDim = + mainAxis.horizontal + ? containerWidth - containerPadding.Left - containerPadding.Right + : containerHeight - containerPadding.Top - containerPadding.Bottom; + if ( containerMainDim > 0.f ) { + if ( mainAxis.horizontal ) + item.widget->setInternalPixelsWidth( containerMainDim ); + else + item.widget->setInternalPixelsHeight( containerMainDim ); + } } if ( item.widget->isType( UI_TYPE_HTML_WIDGET ) ) { diff --git a/src/eepp/ui/gridlayouter.cpp b/src/eepp/ui/gridlayouter.cpp index defa0e0c9..b8a635580 100644 --- a/src/eepp/ui/gridlayouter.cpp +++ b/src/eepp/ui/gridlayouter.cpp @@ -1,9 +1,14 @@ #include #include #include +#include #include +#include +#include #include #include +#include +#include #include #include @@ -747,8 +752,12 @@ void GridLayouter::preSizeItemsForRowSizing() { if ( item.widget->isType( UI_TYPE_HTML_WIDGET ) ) { auto* childHtml = item.widget->asType(); auto* layouter = childHtml->getLayouter(); - if ( layouter && !layouter->isPacking() ) + if ( layouter && !layouter->isPacking() ) { + auto oldHP = childHtml->getLayoutHeightPolicy(); + childHtml->setLayoutHeightPolicy( SizePolicy::WrapContent ); childHtml->updateLayout(); + childHtml->setLayoutHeightPolicy( oldHP ); + } } } } @@ -944,11 +953,70 @@ void GridLayouter::updateLayout() { autoPlaceItems(); buildImplicitTracks(); collapseEmptyAutoFitTracks(); - sizeTracksForAxis( true ); - // Pre-size items to their column widths so row sizing sees correct heights - preSizeItemsForRowSizing(); - sizeTracksForAxis( false ); - applyLayout(); + + int needResize = true; + bool hPercentUnresolved = false; + bool wPercentUnresolved = false; + if ( grid && grid->getUIStyle() ) { + const StyleSheetProperty* hprop = grid->getUIStyle()->getProperty( PropertyId::Height ); + hPercentUnresolved = + hprop && StyleSheetLength::isPercentage( hprop->value() ) && + grid->getPixelsSize().getHeight() <= 0.f; + const StyleSheetProperty* wprop = grid->getUIStyle()->getProperty( PropertyId::Width ); + wPercentUnresolved = + wprop && StyleSheetLength::isPercentage( wprop->value() ) && + grid->getPixelsSize().getWidth() <= 0.f; + } + for ( int sizingPass = 0; sizingPass < 2 && needResize; ++sizingPass ) { + needResize = false; + sizeTracksForAxis( true ); + preSizeItemsForRowSizing(); + sizeTracksForAxis( false ); + applyLayout(); + + if ( hPercentUnresolved ) { + Float contentH = 0.f; + for ( const auto& row : mRows ) + contentH += row.baseSize; + if ( mRows.size() > 1 ) + contentH += static_cast( mRows.size() - 1 ) * mRowGap; + contentH += + mContainer->getPixelsContentOffset().Top + + mContainer->getPixelsContentOffset().Bottom; + if ( contentH > 0.f ) { + mContainer->setInternalPixelsHeight( contentH ); + needResize = true; + } + } + + if ( wPercentUnresolved ) { + const StyleSheetProperty* wprop = + grid->getUIStyle()->getProperty( PropertyId::Width ); + std::string v = wprop->value(); + v.pop_back(); + Float pct = 0.f; + String::fromString( pct, v ); + Float best = 0.f; + for ( Node* anc = grid->getParent(); anc; anc = anc->getParent() ) { + if ( anc->isWidget() ) { + Float sz = anc->asType()->getPixelsSize().getWidth(); + if ( sz > best ) + best = sz; + } + } + UISceneNode* root = grid->getUISceneNode(); + if ( root ) { + Float sz = root->getPixelsSize().getWidth(); + if ( sz > best ) + best = sz; + } + Float resolved = best * pct / 100.f; + if ( resolved > 0.f ) { + mContainer->setInternalPixelsWidth( resolved ); + needResize = true; + } + } + } // Phase 15: set paint-order sort flag if ( grid ) { @@ -1097,9 +1165,6 @@ GridLineResult GridLineParser::parse( const std::string& value ) { static void resolveAxisPlacement( const GridLineResult& start, const GridLineResult& end, int explicitCount, int& outStart, int& outEnd ) { - outStart = 0; - outEnd = 0; - if ( start.isAuto && end.isAuto ) return; @@ -1177,8 +1242,10 @@ static void resolveAxisPlacement( const GridLineResult& start, const GridLineRes } void GridLayouter::resolveDefinitePlacements() { - int explicitCols = static_cast( mColumns.size() ); - int explicitRows = static_cast( mRows.size() ); + int explicitCols = + std::max( static_cast( mExplicitColCount ), static_cast( mColumns.size() ) ); + int explicitRows = + std::max( static_cast( mExplicitRowCount ), static_cast( mRows.size() ) ); mMaxColumn = explicitCols; mMaxRow = explicitRows; @@ -1266,6 +1333,9 @@ void GridLayouter::resolveDefinitePlacements() { } void GridLayouter::autoPlaceItems() { + int maxColumns = mMaxColumn; + int maxRows = mMaxRow; + std::set> occupied; for ( const auto& item : mItems ) { if ( item.resolvedColumnStart > 0 && item.resolvedRowStart > 0 ) { @@ -1332,7 +1402,7 @@ void GridLayouter::autoPlaceItems() { if ( mAutoFlowDense ) { // Dense: search from the beginning for first empty cell for ( int r = 1;; ++r ) { - for ( int c = 1; c <= std::max( mMaxColumn, 1 ); ++c ) { + for ( int c = 1; c <= std::max( maxColumns, 1 ); ++c ) { if ( fits( r, c, sw, sh ) ) { item.resolvedRowStart = r; item.resolvedRowEnd = r + sh; @@ -1351,7 +1421,7 @@ void GridLayouter::autoPlaceItems() { } else { for ( int r = cursorRow;; ++r ) { int startC = ( r == cursorRow ) ? cursorCol : 1; - for ( int c = startC; c <= std::max( mMaxColumn, 1 ); ++c ) { + for ( int c = startC; c <= std::max( maxColumns, 1 ); ++c ) { if ( fits( r, c, sw, sh ) ) { item.resolvedRowStart = r; item.resolvedRowEnd = r + sh; @@ -1373,7 +1443,7 @@ void GridLayouter::autoPlaceItems() { bool placed = false; if ( mAutoFlowDense ) { for ( int c = 1;; ++c ) { - for ( int r = 1; r <= std::max( mMaxRow, 1 ); ++r ) { + for ( int r = 1; r <= std::max( maxRows, 1 ); ++r ) { if ( fits( r, c, sw, sh ) ) { item.resolvedRowStart = r; item.resolvedRowEnd = r + sh; @@ -1392,7 +1462,7 @@ void GridLayouter::autoPlaceItems() { } else { for ( int c = cursorCol;; ++c ) { int startR = ( c == cursorCol ) ? cursorRow : 1; - int rowLimit = std::max( mMaxRow, 1 ); + int rowLimit = std::max( maxRows, 1 ); for ( int r = startR; r <= rowLimit; ++r ) { if ( fits( r, c, sw, sh ) ) { item.resolvedRowStart = r; @@ -1413,6 +1483,9 @@ void GridLayouter::autoPlaceItems() { } } + // Only update member fields — the local snapshot must NOT grow + // during auto-placement, otherwise later items overflow into + // implicit columns instead of wrapping to the next row. if ( item.resolvedColumnEnd > mMaxColumn ) mMaxColumn = item.resolvedColumnEnd; if ( item.resolvedRowEnd > mMaxRow ) diff --git a/src/tests/unit_tests/uihtml_grid_test.cpp b/src/tests/unit_tests/uihtml_grid_test.cpp index 816651ee9..0878aca77 100644 --- a/src/tests/unit_tests/uihtml_grid_test.cpp +++ b/src/tests/unit_tests/uihtml_grid_test.cpp @@ -1716,3 +1716,41 @@ UTEST( GridContainer, downloadGridLayout ) { Engine::destroySingleton(); } + +UTEST( GridContainer, newsblurReducedGrid ) { + Engine::instance()->createWindow( WindowSettings( 1024, 650, "UIHTML Grid NewsBlur Test", + WindowStyle::Default, WindowBackend::Default, + 32, {}, 1, false, true ), + ContextSettings() ); + init_grid_test(); + UISceneNode* sceneNode = SceneManager::instance()->getUISceneNode(); + sceneNode->setURI( "file://" + Sys::getProcessPath() + "assets/html/" ); + std::string htmlContent; + ASSERT_TRUE( FileSystem::fileGet( "assets/html/grid_newsblur_reduced.html", htmlContent ) ); + sceneNode->loadLayoutFromString( EE::UI::Tools::HTMLFormatter::HTMLtoXML( htmlContent ) ); + sceneNode->update( Seconds( 1 ) ); + sceneNode->updateDirtyLayouts(); + + auto* gridEl = sceneNode->getRoot()->findByClass( "NB-inner" ); + ASSERT_TRUE( nullptr != gridEl ); + ASSERT_TRUE( gridEl->isWidget() ); + + Float gridW = gridEl->getPixelsSize().getWidth(); + Float gridH = gridEl->getPixelsSize().getHeight(); + EXPECT_GT( gridW, 100.f ); + EXPECT_GT( gridH, 100.f ); + + // Verify NB-feature-group items aren't overexpanded + auto* fg = sceneNode->getRoot()->findByClass( "NB-feature-group" ); + ASSERT_TRUE( nullptr != fg ); + Float fgH = fg->getPixelsSize().getHeight(); + EXPECT_GT( fgH, 100.f ); + EXPECT_LT( fgH, 2000.f ); + + auto* fgiText = sceneNode->getRoot()->findByClass( "NB-fgi-text" ); + ASSERT_TRUE( nullptr != fgiText ); + Float textW = fgiText->getPixelsSize().getWidth(); + EXPECT_GT( textW, 150.f ); + + Engine::destroySingleton(); +}