Olivero: Simplification of the grid layouts to use CSS grid (instead of Flexbox)

The Needs Review Queue Bot → tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Re-rolled #17

As a non-critical task this will only go into 10.1.x now; there is no functional bug to solve in 10.0.x here.

Successfully Patch Applied #19.
It's working fine as expected.
Attached screenshot.

I will double check patch #19

Done patch work successfully working as expected
I have attached a screenshot.

Patch no longer applies
error: patch failed: core/themes/olivero/css/layout/layout-builder-fourcol-section.css:11
error: core/themes/olivero/css/layout/layout-builder-fourcol-section.css: patch does not apply
error: patch failed: core/themes/olivero/css/layout/layout-builder-threecol-section.css:11
error: core/themes/olivero/css/layout/layout-builder-threecol-section.css: patch does not apply
error: patch failed: core/themes/olivero/css/layout/layout-builder-twocol-section.css:11
error: core/themes/olivero/css/layout/layout-builder-twocol-section.css: patch does not apply

Also tagging for screenshots of these being used in layout builder. 1 set should be enough.

Reroll against 11.x

Please include a diff or interdiff with patches even for rerolls.

Seems #27 caused test failures.

finnsky → changed the visibility of the branch 3262156-reroll-plus-fixes to hidden.

finnsky → changed the visibility of the branch 3262156-8 to hidden.

finnsky → changed the visibility of the branch 3262156-olivero-simplification-of to hidden.

I lile this new approach more than previous.
Now i've used `span 2 or 3`
so 25 + 25 + 50 has same middle gap as 50 + 25 + 50
etc.

Please review!

Agree with the approach used on #32. Much nicer to define using spans than calculating values.

Code looks good, shame we're still using 33-34-33 now that it's proper thirds, however I realise that that is perhaps above and beyond. Similar with 33-67.

I've checked the code, but have not had a chance to test, so won't mark as RTBC yet.

Thanks, looking forward to this change that I've often overridden. :)

This looks amazing. I love how grid can clean up the code so well. We had those funky margins in there because we had to support IE11 🤮

Anyway, I have one nice-to-have that I think we should get in here:

Instead of using --layout-threecol-grid: repeat(4, 1fr); to set up the grid, let's use minmax(0, 1fr) to set the maximum grid width to 1fr. Otherwise, if there's super wide content that doesn't fit, it will stretch the grid.

So, something like --layout-threecol-grid: repeat(4, 1fr); would become --layout-threecol-grid: repeat(4, minmax(0, 1fr));

Otherwise this is perfecto.

Fixed please review.

Hiding patches.

So moving to NW because the nightwatch tests keep failing, re-ran 3 times.

seems `3 times` of random failures. at least gone after rebase

Verified the changes on Drupal version 11.x and the changes are applied cleanly. Attaching before and after screenshots for reference. Need RTBC+1

Before:

After:

Believe feedback for this one has been resolved and no visual regression introduced.

Removing D10 beta tags as clearly missed that boat.

nod_ → closed merge request !8859

Committed 0ba8488 and pushed to 11.x. Thanks!
Committed 416ef9d and pushed to 10.4.x. Thanks!

Automatically closed - issue fixed for 2 weeks with no activity.

Retroactively crediting myself for triage. You're welcome, Acquia.