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

Actually this sounds like a code improvement, but not a hard requirement to finish before 10.0.0, so major is a better priority.

I will review this.

I have reviewed the last MR and still found usage of flex in files under components/ and layout/ like
comment.css, content-moderations.css - flex-direction, dropbutton.css, feed.css, etc.

Also do we have to change the display:inline-flex to display:inline-grid ?

Thanks for review, but i think that target of issue (simplification of flexbox grids and not removing flexbox) achieved in that MR.

@kostyashupenko Thanks for the MR.

Changing the layout format of the messages, vertical tabs, etc is out of scope, and I'm not quite sure it's needed just yet. Let's keep this to the layout templates.

@finnsky It looks like the MR you created is similar (if not a copy) to @kostyashupenko's. Are there differences? if so what?

RE #13
i would say difference in more smart grid calculation in these variables:

  --grid-2-col-width: calc(var(--grid-col-width)*2 + var(--grid-gap));
  --grid-3-col-width: calc(var(--grid-col-width)*3 + var(--grid-gap)*2);

Updated MR with removing `out of scope` changes
@mherchel
quick explanation of different approach:

In case of equal columns usage of 1fr is ok.
But in case of 25%/50%/25% or 33%/67% or others it calculated incorrect without gap size.

Layout of article with 25%/50%/25% in first MR:

In my approach i use gap size for calculations, so in my MR same layout:

It seems me better approach. Please review

MR needs to be rebased & re-targeted to 10.1.x

I have re-rolled the patch against d10. As the patch is no longer applied on d10. so not attaching the inter-diff.

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.

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

Seems #27 caused test failures.

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 + 25
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 with code review.

Before:

After:

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

Removing D10 beta tags as clearly missed that boat.

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

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