Fix remaining Drupal 10 compatibility issues

OK, I fixed the remaining deprecated dependency. Now only the CSS issues remain, giving this issue to you @thomas.frobieter, all "#drupal-off-canvas" occurences should target "#drupal-off-canvas-wrapper" for D10 and "#drupal-off-canvas:not(.drupal-off-canvas-reset)" for Drupal 9, see: https://www.drupal.org/node/3305664 → .

@Anybody, since the first selector is Drupal 10 specific, I suggest we drop the Drupal 10 compatibility for the current branch and create a new 3.x branch which is D10 only.

Thx @Grevil! I agree, please merge this into 2.x and remove || ^10 and create a new 3.x branch with the proposed changes as separate issue afterwards. Assigned to @thomas.frobieter.

PS: Crazy and super cool that Drupal detected this!

The css changes need to be applied to the D9 and D10 Version, but different changes for each branch! As described in #3! I can try to solve them and let @thomasfrobieter review afterwards.

@Grevil yes please. You may create both MR's here. To make it easier for @thomas.frobieter to review.

Damn I should have read through the whole change record first...

To target all versions, simply combine the selectors

And also the css files are minified, let's just fix this in one branch I am not 100% sure were the changes need to get applied, so I reassign @thomas.frobieter here.

@Grevil: FYI: Minification will be removed on all Drupal project, still an open task for @thomas.frobieter when the time has come

thomas.frobieter → made their first commit to this issue’s fork.

Thx @Grevil! I agree, please merge this into 2.x and remove || ^10 and create a new 3.x branch with the proposed changes as separate issue afterwards. Assigned to @thomas.frobieter.

I disagree on this. #drupal-off-canvas is still present in D10, so there is no need to switch to the outer wrappers selector. So we should still have D9 compability, without extra release.

Important thing for the off-canvas styles to work: Uninstall GIN Layout Builder, as it contains a buggy sidebar. We should stick with the core styles, which seems more reliable now.

@thomas.frobieter I'm fine with any decision, as it's your part, just want to ensure we don't do something wrong here due to this message from drupal upgrade status:

web/themes/custom/webksdct/css/modules/backend/layout_builder.min.css:
┌─────────┬──────┬─────────────────────────────────────────────────────┐
│ STATUS  │ LINE │                       MESSAGE                       │
├─────────┼──────┼─────────────────────────────────────────────────────┤
│ Manuell │ 0    │ The #drupal-off-canvas selector is deprecated in    │
│ überpr� │      │ drupal:9.5.0 and is removed from drupal:10.0.0. See │
│ �fen    │      │ https://www.drupal.org/node/3305664.                │
│         │      │                                                     │
└─────────┴──────┴─────────────────────────────────────────────────────┘

which says it will be removed?!

See https://www.drupal.org/node/3305664 → where they say only the "-wrapper" one will be kept.

If you already read all that, fine! :)

I've checked it in our D10 instance, its present. The way the new CSS reset of the offcanvas work is kinda forcing us to use the other id. In fact, it doesnt matter, as long as the selector specicicy is heigher then the reset - what is the case. Since we need to support D9 + D10, this is the way to go.

Otherwise we need to drop D9 support, I don't want to maintain two versions of this.

D9 support may be dropped until end of this year. Until then it's easy to use cherry-picks to copy changes over.
You decide! :)

We have a working solution, so we should not make more work for ourselves than necessary.

@Anybody please read the change record!

https://www.drupal.org/node/3305664 →

To target all versions, simply combine the selectors. Be sure to test thoroughly.

#drupal-off-canvas:not(.drupal-off-canvas-reset),
#drupal-off-canvas-wrapper {
}

Both the d9 and d10 CSS selectors are present! So no harm, leaving the d9 selector.

The mentioned solution from the change notice will produce 200% CSS code, I don't see any need to do that.
We might switch the selectors when Drupal 9 is gone.

Howewer, we should not waste more time on this completely unimportant issue ;)

@thomas.frobieter: This introduced a critical regression, please always review carefully: 🐛 Regression: Not Drupal 10 compatible anymore Fixed

The merge removed the Drupal 10 compatibility. Hopefully no other things were reverted / broken? Please take a look at the commit above again, if anything else is fine! I'm going to fix the D10 compatibility in the other issue now.

No idea how that happened... but a good reason for careful reviews.

If anything else was broken by that commit, please create a separate issue for the fix.

I've checked my commits, looks like there are only CSS and library.yml changes. So what exactly is the point here with the D10 compability? They check for stylesheets, really?!

@thomas.frobieter, check them again:

Re: #23 don't really get the comment. See #13 that's the code that was merged and should please be re-reviewed. It removed the Drupal 10 compatibility.

Do not commit anything here anymore.

Heads-up, clarified my comment in #25

Oh, yeah sorry, missed that change. However that happened. Should have resulted in a merge conflict, didn't it?

The other commits (stylesheet tweaks) are tested, so they should be fine.