Review markup / CSS before 2.0.0

Okay, so what I have found so far:

- We have z-index bug on Olivero → . Guess this is because i placed the Cookies UI block in the footer region, which is set to z-index: 1 by olivero. So this is probably more a documentation, not a code thing => "Place the COOKIES Ui block outside the regular page structure (directly inside the body element is recommended)". However, Olivero does not provide such a region, so I am not sure if this requires further work on the cookies side, as Olivero is the default Drupal theme.
- The BEM classnames are incorrect, if the library is ment to fit the Drupal standards this should be fixed? Example: We have cookiesjsr-banner and an 'active' class on the same wrapper, the active class should be cookiesjsr-banner--active instead (Formatter class). Another example are the child elements of cookiesjsr-banner, currently we have cookiesjsr-banner--info, this shouldn't be a formatter class, it should be a child element class: cookiesjsr-banner__info, etc.
- cookiesjsr-layer--overlay is a button element, which is quite unusual. I'd recommend to change it to a simple div. There is a high risk that the button element will recive unwanted stylings in some themes.
- .cookiesjsr-layer: This change is not mandandtory, but would improve the stability of the layers layout. Instead using padding to reserve space for the header and footer, the layer should use display: flex; flex-direction: column; ... so there is also no need to set a fixed height to the header and footer (=> flex: 0 0 auto; and flex: 1 1 auto on the .cookiesjsr-layer--body)

Nice!! So I think it would be best to prepare a MR for review by @JFeltkamp here and in https://github.com/jfeltkamp/cookiesjsr/issues/41

And I think then we should try to get things fixed and 2.0.0-rc1 out asap :) (Maybe synchronous in the lib and here)

I'd like to hear if @JFeltkamp agrees to this points before investing further time. I guess all the points (except the block/region issue) are related to the library, not the Drupal module?

@JFeltkamp could you give a short feedback please, as written in #7?

Hey Thomas, thank you for the review.

Here my thoughts about your points in detail.

1. Z-index problem Olivero Theme. Indeed we should provide a stable and good solution. But we should not modify the Olivero theme in our CSS. I think we should report an issue for Olivero and add a comment in our description section: E.g.

Known issues:
OLIVERO Theme - To avoid known display (z-index) problems, insert the COOKiES UI block at the end of the "Header" section.

2. BEM class name - This is not a big win to fix it. After the fix the CSS and JS has to be updated in sync. May be we have cross over issues. I would have done nothing here.

3. Button element: This is not a bug. Due to accessablity all clickable elements must be declared as button (or A-Tag ore needs a tabindex and aria-label). The background should be clickable to get rid of the Dialog.

4. CookiesJSR Layer Flexbox: This could be a good improvement.

Thanks @jfeltkamp! Regarding the BEM classes etc. I think we should make those clean-up changes now, because of the breaking changes we have in 2.0.0 anyway. This way the users with custom changes will only have to make the adjustments one time and it will be fixed for the future. So it's now or never :)

Great. let's get this all done here and in the lib!

That's true and I'm fine with it.

So if I shall do that, please give me some more detailed instructions. I'm not that familiar with these best practices.
If Thomas wants to do it, I will make the review and give some support for the changes in Svelte.

Yes, I think it will be easier if I fix the class names directly.

I create a fork of: https://github.com/jfeltkamp/cookiesjsr/tree/2.x/src

I'll try to solve it within the next two weeks!

1. Z-index problem Olivero Theme. Indeed we should provide a stable and good solution. But we should not modify the Olivero theme in our CSS. I think we should report an issue for Olivero and add a comment in our description section: E.g.

Known issues:
OLIVERO Theme - To avoid known display (z-index) problems, insert the COOKiES UI block at the end of the "Header" section.

We might should suggest a new region to Olivero "Outer Page" or something like this, located as a region at the bottom of the body-tag.

4. CookiesJSR Layer Flexbox: This could be a good improvement.

I'll try this out in in the library fork too.

Here is the Olivero issue: https://www.drupal.org/project/drupal/issues/3497813 ✨ Add further region "Outer Page" to place blocks directly inside the body Active

FYI according to the tests, the "bannerText" should be inside a span with the class "cookiesjsr-banner--text":

    $session->elementTextEquals('css', '#cookiesjsr > div.cookiesjsr--app > div.cookiesjsr-banner > div.cookiesjsr-banner--info > span.cookiesjsr-banner--text', $cookiesTexts->get('bannerText'));

But currently, the text sits directly inside "div.cookiesjsr-banner--info". Just a heads-up.

Done @jfeltkamp, please review: https://github.com/jfeltkamp/cookiesjsr/pull/42

BTW: Very well written, who actually needs Klaro :P

FYI according to the tests, the "bannerText" should be inside a span with the class "cookiesjsr-banner--text"

Guess the tests needs to be adjusted, I am pretty sure the class was renamed to 'cookiesjsr-banner__info' ('cookiesjsr-banner--info' before my changes).

@thomas.frobieter can you adjust the test here in a MR? Maybe otherwise @Grevil can help? Nice chance to take a look at the tests and how simple they work.

I don't currently have the time to delve into this topic (nor am I really interested). So if this needs to be fixed anytime soon, someone else should do this.

The class "cookiesjsr-banner--text" is not present in the new library, so its most likely "#cookiesjsr > div.cookiesjsr--app > div.cookiesjsr-banner > div.cookiesjsr-banner--info".

Ok, so we don't need the span anymore? Then I'll just adjust the tests accordingly and that's it :)

I can't tell what it used to be good for, but it's out and Joachim will scream if something doesn't fit ;)
So: ‘#cookiesjsr > div.cookiesjsr--app > div.cookiesjsr-banner > div.cookiesjsr-banner--info’

Made a review of Thomas' changes.
Everything still works fine, also the improvements in header and footer, what I like.

Created new version: 2.0.5

These guys using 1.x with extended custom changes will not be amused - But our motto is: Now or never! - So now :-)