🇺🇸United States @mherchel

While testing this out, I found that the primary button's focus state is not visible in the hero, because both the focus outline and the background are the same color (Drupal navy).

I fixed that while I'm in here. Should be good to go!

mherchel → created an issue.

This is ready for review in the private Gitlab.

mherchel → created an issue.

I did some extensive spot checking in Olivero, Claro, as well as the off-canvas dialog.

Everything looks as it should. Without some type of robust visual regression system, this is the best we're going to get.

I recommend getting this merged sooner than later, so we have time to identify any potential regressions (which I'd be surprised to see).

Also, this saves about 269kb on the initial page load. It'll save more if the <pre> tag was present to load Source Code Pro

This is ready!

Steps:

• Removed all non-woff2 files
• Cleaned up the global-fonts.css to reflect correct paths
• Swapped out the noto-sans variant of the font to be consistent with the other variants to only contain latin glyphs
• Swapped out the variable TTF for the Source Code Pro font. We weren't using any of the variable font features, and the file was unnecessarily large.
• added font-display: block to all the font declarations. This will tell the browser to wait for the webfonts for 3s. If it doesn't have it by then, it will load the fallbacks. If and when the webfonts download, they'll be swapped out.

God that is a fugly selector! Anyway, I'll take a look at this shortly!

I found bugs in Tugboat:

Bugs fixed (🤞)

Good catch!

FYI, test is red because evidently the performance test counts the number of CSS bytes (which is obviously increasing with this MR).

This will need to be changed at https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/navig..., although I don't want to do it just yet, as reviews might necessitate further changes.

New MR created with a "regular" CSS reset.

The "Donut" selector that @nod_ mentioned in Slack isn't necessary.

Because [data-drupal-admin-styles] exists multiple times on each page, we can't use a regular #id selector reset like we do we the off-canvas. Fortunately, there's a "quirk" with the CSS :is() pseudo-class, where its weight is equal to the highest weight selector in the list.

So, if we write :is(#extra-specificity, [data-drupal-admin-styles]), the ruleset will match [data-drupal-admin-styles], but have the specificity of the ID #extra-specificity. This is how we're proceeding.

This should be ready to go!

mherchel → changed the visibility of the branch 3511280-front-end-theme-styles to hidden.

I created a test case, where i test Noto Sans with the non-latin glyphs above.

The tl;dr is that the glyphs will gracefully fallback to system fonts if needed.

IMO, we should only include the latin glyphs, which would save ~1mb of critical assets. The glyphs will be rendered properly, and it's extremely doubtful that anyone will know they're in a different font.

You can see this a short screencast at https://youtu.be/LAPYmm7I8rw

I'm attaching my test files, if anyone wants to test locally.

I asked chatGPT

I have an international website where people from all over the world register their names. Their names might include non-latin glyphs.

I need to test out fonts to make sure the names are rendered properly.

Can you give me a representative list of somewhat common names that use non-latin glyphs?

And it gave me this. I'll use it to test.

Arabic

• محمد
• فاطمة
• علي
• یاسمین
• حسین

Cyrillic

• Алексей
• Наталья
• Дмитрий
• Ольга
• София

Chinese

• 王伟
• 李娜
• 张强
• 陈杰
• 刘洋

Japanese

• 佐藤健
• 田中美咲
• 山口翔
• たかし
• サクラ

Korean

• 김민준
• 이서연
• 박지훈
• 최예은
• 정우진

Devanagari (Hindi)

• आरव
• सिया
• अर्जुन
• प्रिया
• विवेक

Greek

• Γεώργιος
• Αθηνά
• Νίκος
• Ελένη
• Δημήτρης

Hebrew

• יוסי
• שרה
• דוד
• רונית
• משה

Thai

• สมชาย
• พรทิพย์
• สุริยา
• กิตติ
• นพพร

We do need a font with all the glyphs, since people’s names are in many scripts, localize.drupal.org has many languages, and everywhere the global community is communicating.

Yeah, but all of the other font weights (I assume) don't have the glyphs. If we load all or the font files, it could potentially add 1MB of render blocking assets to the page, which IMO is a serious problem.

Is there anyway to get a list of names that have non latin glyphs? Then we can test properly. It's entire possible that the glyphs would gracefully fallback to another font (or the browser will otherwise handle it).

When exporting the font, we have the following options:

• cyrillic
• cyrillic-ext
• devanagari
• greek
• greek-ext
• latin
• latin-ext
• vietnamese

This is ready for review. A couple notes:

• Both mobile and desktop search buttons now toggle aria-expanded
• After opening the search drawer (both mobile and desktop), the search input will now focus.
• Open / close animation is much smoother now. There was some weirdness before that was being solved with a transition delay on the search form.
• I added testing instructions to the IS

Had a long discussion about how to proceed with this at https://drupal.slack.com/archives/C7AB68LJV/p1751487654599299

Involved were navigation maintainers @nod_ and @finnsky (among others).

Per @nod_, we're going to move forward with the reset similar to how the offcanvas does it. We should also look at the fence created by https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/ckedi... and see if this is something we need or could use.

mherchel → created an issue.

mherchel → created an issue.

Good catch!

@biz123 - Let me know if you're still actively working on this. If not, I'm happy to help

I looked over the code, issue, and looked through the changes

A couple notes:

• This is a large MR
• Lots of order changes, which muddies things up
• Most of the work is in css/base/basic-elements.css and changing the global-text class to wysiwyg
• I don't see anything within the code that's going to break anything.
• Looking visually, I don't see any obvious regresssions

My initial thought is that because 1) This MR has been open for two months and there's a lots of reorganization, this should be merged, because it will hold up other changes.

We should also do a followup to convert the old-style CSS to use nesting.

Not yet setting this to RTBC, because not sure if @biz123 thinks its ready (it's not set to review).

nicxvan → credited mherchel → .

the_g_bomb → credited mherchel → .

New changes look good. The table library will be loaded when rich text fields are being displayed.

This looks good to me. I looked at the code, and it looks as expected. I also tested it out and checked out the interfaces with the most "interesting" CSS, to make sure they look as expected, and they do!

CR looks

to me!

Looking at the 11.x, the current top bar is is an <aside> with an aria-labelledby attribute that points to its visually-hidden heading, with the text "Administrative top bar"

From my point of view, I don't see any reason to change this (please correct me if I'm wrong).

There are currently two <nav> elements in the sidebar, which we will consolidate to one.

If all this looks correct, I'll update the IS. I also feel like we might need to have a discussion in Slack to make sure we're on the same page. The requirements of this issue keep changing, and we need to lock it down to make progress.

I hate to bikeshed this.

From @rkoller in #56:

For the sidebar i would use a navigation landmark and label it with primary, that way it would be crystal clear that this is the main navigation for the admin ui.

I worry that on the front-end of the site, the primary label would be confused with the primary navigation of the site. My thought is it could be named administrative toolbar Thoughts?

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

Yeah, I was also wondering about the 'nesting-rules': { edition: '2021' },. Per the docs it defaults the the behavior of previous PostCSS nesting (which is not actually what browsers implemented).

The new rules (2024-02) cause the browser to accurately reproduce specificity by wrapping parent selectors in an :is(). This mimics the actual behavior of browsers (which basically wrap parent selectors in an :is()).

I really doubt the 2024-02 rules will cause any regressions, but it's impossible to be 100% sure unless we have some type of robust visual regression testing going on, which we don't.

My suggestion is to stick with the 2021 rules for now. Early on after 11.2, lets update to the 2024-02 rules, which will more accurately reproduce the specificity that browser do. Then we'll have several months to find any potential regressions.

After that, we should really see what's holding us back from shipping native nesting. A quick look at https://caniuse.com/css-nesting, shows that our supported browsers support it.

OK. Should be good!

Fixing now

I like the naming in #35!

Should there be a reverse relationship too? An example of this would be to limit people to put accordion_items only into accordion_group components.

Good catch!

The issue was that the top bar was rendering, even if it didn't have content, which pushed down the popover.

I added a check for content within the twig file. Should be good to go. Thank you!

Yay! Test is passing. Note that I don't test out displace's functionality (that gets tested in its respective tests), I just test that the attribute is added properly, which is what the JS in this module does.

Yeah the current tests passed when I re-ran them.

Just pushed a simple nightwatch test. Lets see if this passes (I didn't test it locally 😬🤞)

Ready for review! Screenshot attached :D

mherchel → changed the visibility of the branch 3526266-navigation-top-bar to hidden.

mherchel → created an issue.

Fix attached.

mherchel → changed the visibility of the branch 3526180-regression-drupal.displace-not to active.

mherchel → changed the visibility of the branch 3526180-regression-drupal.displace-not to hidden.

mherchel → created an issue.

Somehow I missed this.

Tables can also be added via CKEditor, which is why we had it as a global library. You can see at https://git.drupalcode.org/project/drupal/-/blob/11.2.x/core/themes/oliv..., we're also targeting tables that appear in .text-content (which is formatted text).

wim leers → credited mherchel → .

mherchel → created an issue.

mherchel → created an issue.

mherchel → created an issue.

wim leers → credited mherchel → .

andyg5000 → credited mherchel → .

Heck yes! This totally fixes the issue for me. I'm attaching two videos:

1. Part 1: The video showing the issue existing
2. Part 2: The video showing patch installed, and the issue solved

Still leaving a NR, since we need a code review and I'm not super qualified for this level of PHP. But confirmed to fix the issue!

evidently the test case isn't as simple as I initially thought. I've run into this a lot, but need to see what the differentiator is that causes it.

Updating the steps to reproduce

Slack discussion at https://drupal.slack.com/archives/C079NQPQUEN/p1746998199858839

In it, backend framework manager @larowlan states (in regards to @deviantintegral approach in the issue summary)

that approach looks fine to me, ideally with the cache clearer injected

Bumping this to Major (note it may need to be critical). Most new themes will have SDCs, and this will affect anyone who install new themes via the UI.

smustgrave → credited mherchel → .

Any chance this can be backported to 10.x? It'd be nice to use this features and have themes be compatible with both D10 and D11. My understanding is that XB hopes to support D10 also.

This works perfectly!

I ran the command in ddev, and worked as expected.

The theme appeared under the appearance route as expected.

I played around with it and tried to break it, and it worked great!

Running this thing through its paces 🤞

➕💯➕💯➕💯➕💯

mherchel → created an issue. See original summary → .

Crap. Forgot to actually change the field!

Bumping to major.

With Drupal's focus on site templates, this becomes a blocker for that functionality.

yep!

g4mbini → credited mherchel → .

Just pushed a 2.0.5 release with this fix (and fixes tests!)

Crediting Matt Glaman. I owe him so many beers!

mherchel → created an issue. See original summary → .

Going to try to fix tests before I put out a release. Mid-week by latest

Merged! Will tag a release shortly!

Works great for me! Thank you

Whoohoo!! Thanks Matt and Lauri!!!

I'm hoping that this can be backported to 10.5.x. As it stands now if I want to make a theme that supports both D10 and D11, I still have to work around this bug by adding additional specificity onto my SDC CSS selectors. :D

I gave a quick look at the markup and CSS and added a couple suggestions. Everything is looking great at first glance although I haven't yet tested it (will probably do so next week).

Thanks for working on this!

I'm also crediting @caligan here, since she worked on it at the DrupalCon Atlanta sprints (although it looks like work was duplicated with @javi-er, which happens).

Re-tested. Confirmed new method outputs the same. Not including screenshots because they're exactly the same as in #59 and #62

Reviewing now!

Image showing issue is fixed with aggregation

Confirmed that this is fixed when aggregation is enabled. I tested this by modifying Olivero's CSS in two places:

1. In the teaser SDC component.
2. In the theme's base CSS.

The SDC styles take get injected after, which takes precedent, which is the correct behavior! YAY!!!

Here's a picture of Matt doing the work!! :D

Screenshots showing that the issue is fixed. I'm going to double check this works as expected when aggregation is enabled.

Draft CR at https://www.drupal.org/node/3515838 → . Still needs screenshots, which I will add as I test this out.

mherchel → changed the visibility of the branch 3374901-css-load-order to hidden.

mherchel → changed the visibility of the branch 11.x to hidden.

mherchel → changed the visibility of the branch 3374901-sdc-stylesheets-are to hidden.

Assigning credit (everyone gets credit)

Updating issue summary.

Updating title.

mherchel → created an issue.

I tested this by using the following snippet in the page.html.twig

              {% embed 'drupal_cms_olivero:accordion' with {
                title: 'This is the main heading',
                description: 'This is a long description.This is a long description.This is a long description.This is a long description. ',
              } only %}
                {% block items %}
                  {% embed 'drupal_cms_olivero:accordion-item' with {
                    title: 'Accordion item headingAccordion item headingAccordion item headingAccordion item headingAccordion item heading',
                  } only %}
                    {% block accordion_content %}
                    Lorem ipsum dolor sit amet consectetur adipisicing elit. Nam, veniam, sint facilis pariatur hic iste dignissimos rerum possimus, totam architecto qui quos molestiae incidunt voluptatibus fugiat omnis nihil rem eum!
                    {% endblock %}
                  {% endembed %}

                  {% embed 'drupal_cms_olivero:accordion-item' with {
                    title: 'Accordion item heading',
                  } only %}
                    {% block accordion_content %}
                    Lorem ipsum dolor sit amet consectetur adipisicing elit. Nam, veniam, sint facilis pariatur hic iste dignissimos rerum possimus, totam architecto qui quos molestiae incidunt voluptatibus fugiat omnis nihil rem eum!
                    {% endblock %}
                  {% endembed %}

                  {% embed 'drupal_cms_olivero:accordion-item' with {
                    title: 'Accordion item heading',
                  } only %}
                    {% block accordion_content %}
                    Lorem ipsum dolor sit amet consectetur adipisicing elit. Nam, veniam, sint facilis pariatur hic iste dignissimos rerum possimus, totam architecto qui quos molestiae incidunt voluptatibus fugiat omnis nihil rem eum!
                    {% endblock %}
                  {% endembed %}

                  {% embed 'drupal_cms_olivero:accordion-item' with {
                    title: 'Accordion item heading',
                  } only %}
                    {% block accordion_content %}
                    Lorem ipsum dolor sit amet consectetur adipisicing elit. Nam, veniam, sint facilis pariatur hic iste dignissimos rerum possimus, totam architecto qui quos molestiae incidunt voluptatibus fugiat omnis nihil rem eum!
                    {% endblock %}
                  {% endembed %}

                {% endblock %}
              {% endembed %}

This should be good to go!

Overall this is looking good. There's a couple issues:

1. The content area within the accordion items should be a slot, since it will accept arbitrary markup (and possibly other components)
2. We should be using CSS nesting
3. Need to use CSS logical properties for proper RTL support

I'm in the DrupalCon contrib room right now, so going to work on this.

wim leers → credited mherchel → .