Gainesville, FL, US
Account created on 21 February 2007, over 18 years ago
  • Senior Front-end Developer at AgileanaĀ  …
#

Merge Requests

More

Recent comments

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US
šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US
šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

mherchel → created an issue.

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

Can confirm this fixes the issue for me! I had to manually apply the patch and then run ddev xb-ui-build. But works as expected! šŸ™Œ

Leaving at NR, since I don't have enough context to do a code review.

Thank you!

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

We can do it differently, we can have a SDC container which has one slot for all columns.
and SDC child column. And no need to ask the editor how many columns he needs.

Good question. The component that we have also asks how they arrange the slots. For example, we might have a 3 column layout that arranges them 25% 25% 50%, etc.

Plus we might want to place more than one other component within a slot.

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

Couple reasons:

- Developer experience. Sometimes I want to inspect the page to make sure the component is rendering properly under the hood. The preview has pointer-events: none, so its tougher to inspect (although maybe I should file an issue for that).
- Just because I want to see the actual page that my customers will see.

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

mherchel → created an issue.

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

mherchel → created an issue.

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US
šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

mherchel → created an issue.

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

This is something that most SDC authors would make use of, and it would result in better UX's for end users.

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

mherchel → created an issue.

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

mherchel → created an issue.

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

mherchel → created an issue.

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

mherchel → created an issue.

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

(was pointed to this issue by @penyaskito at https://drupal.slack.com/archives/C072JMEPUS1/p1759664391938989?thread_t...)

I have a use case for this:

I want to create an custom block type to display remote video and expose it to Canvas. Obviously core Media supports remote video out of the box, so this should be easy. However, I cannot get this inline block into Canvas because

Block plugin settings must opt into strict validation. Use the FullyValidatable constraint. See https://www.drupal.org/node/3404425 →

Enabling this would 1) solve my specific use case, and 2) open up a world of possibilities with other use cases.

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

What are those major implications? Is it purely code / architectural or is there implications to the end-user?

Had a discussion involving this with @effulgentsia at https://drupal.slack.com/archives/C072JMEPUS1/p1759503554301209

This isn't a CSS issue for me -- our issue is that template suggestions are not getting picked up.

From the conversation:

  1. I'm setting template suggestions for various blocks based on what regions they're in.
    1. So the primary menu block placed in the header_third region calls menu--region-header-third.html.twig
    2. This is important because the when placed in the header, the menu gets a special markup (so it can function as a mobile menu etc)
    3. But if the menu were to appear in a sidebar on the content of the page, we display the default template, which is a vertical sidebar menu
  2. But when placing the menu via Canvas, that template suggestion is not being set.

To set the template suggestion, we're doing something similar to what we've done in Olivero at https://git.drupalcode.org/project/drupal/-/blob/11.x/core/themes/oliver...

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

The has a canvas-page class, and the content region has a region--content class, so can you achieve this use case by using .canvas-page .region--content as your selector or prefix?

Yep I can. I'm doing something similar as a work-around right now.

Agree that the most important thing is making it consistent.

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

Answering Wim's question in #4

Also: this is still reproducible in yesterday's beta1, right?

Not sure. I'm on @phenaproxima's latest xb-demo, but IIRC that hasn't yet been updated as of right now.

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

The issue is that one malformed component is making the entire component list unavailable.

Exactly! The only way I was able to track this down is because I had just introduced this component. When I initially filed this issue several weeks ago, I was unsuccessful in tracking down the offending component.

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

Ran into this today right after I [tried] to create an image component.

Note that even if this component is malformed (it likely is) the error response is the bug here.

Placing this component so that it gets loaded totally breaks the editor. If this editor is malformed, it should not break anything. Instead it should show up on the disabled components list.

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

One of the main issues is that I'd hope that the markup of the page would be consistent (as possible). Removing this field template only on the editor iframe introduces inconsistencies that other people might run into.

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

Had a discussion with @penyaskito at https://drupal.slack.com/archives/C072JMEPUS1/p1759435412275339, and he asked me leave a comment here.

I have a use case where I override the field template for the canvas field field--canvas-page--components.html.twig. The use case to add a CSS class that sets default spacing on all direct children below it, and overriding the template works perfectly.

However this template isn't called within the iframe preview within the canvas editor.


šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

I want to add that I run into this issue all the time, and also fix it by adding 'null' as a type.

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

mherchel → created an issue.

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

Screenshot here! Let me know if this should be a separate issue:

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

mherchel → created an issue.

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

I'm also seeing an issue where [empty] groups are still visible even if the source theme is uninstalled.

Is this the same issue, or do you want a new one?

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US
šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

mherchel → created an issue. See original summary → .

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US
šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

mherchel → created an issue.

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

mherchel → created an issue.

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

No worries.

Before I do a rebase, can I get a review on the methodology, etc, from the maintainers/committers?

This will touch so many files, I'll be rebasing forever as it just sits here (it's already been waiting forever).

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

Can you confirm that you used only the UI to achieve this epic failure? 🫠

Yes. This is UI only.

Or did you get a confirm dialog that you clicked through?

Clicked through what? I simply uninstalled the theme through the /admin/appearance as normal. Then I went to the homepage and ā˜ ļø

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US
šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

mherchel → created an issue.

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

I would like to submit this for the "Module previously known as Experience Builder"

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

OK. I set the summary to trim at 800 characters. Should be good to go!

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

mherchel → created an issue.

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

Updating CDN path. Removing D7 reference

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

Big +1 to this!

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

To be clear: a manual way to opt out: via the UI.

It'd be super nice if we could include config in our themes that dictates which components are disabled. I wonder if this already works.

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

From an Olivero and CSS maintainer position, a big +1.

It's something that I don't believe will create a ton of issues. If issues are identified, they should be fixed. Plus, it makes Drupal's accessibility look behind the times if we say we only officially support 2.1.

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

mherchel → created an issue.

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

Testing instructions

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US
šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

Updating title.

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

Testing instructions

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US
šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US
šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US
šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

What about MR I don't really like `hack` word.
`#extra-specificity-trick` probably will sounds better? :)

Ya'll figure that out in Slack, and we can change the selector name at the end.

Does everything else look good?

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

I like it! Makes it a bit more clearer to the dev that this is very non-standard.

Pushed!

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

mherchel → created an issue.

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

mherchel → created an issue.

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

mherchel → created an issue.

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

mherchel → created an issue.

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

Thanks so much Putra! I'm sure we'll run into each other again soon!

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

Putra was instrumental in early Olivero work. In fact, it was her idea to get it off the ground and *really* do it. I remember the airport discussion with her (post DrupalCon Seattle) when we discussed it. It seems so long ago yet so close.

Here we are pre-FLDC 2020 sprinting. I remember I had a really bad head cold, but I was still stoked to keep working on it

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

Just pushed styles for "Get started" at mobile widths. This will also need to be a <button> element instead of <nolink>

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

Added testing steps (original doc here)

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US
šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

Removed the commented out stuff from preprocess along with unused code.

Many of the @todo's are gone, but some still remain and will likely be either followup issues, or issues to fix when the underlining issues (could be either browser issues or core issues) are resolved.

Once I have time, I'll get a matrix somewhere so we can comprehensively test, but in the meantime, I'd love as many eyes on this as possible so setting back to NR

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

That seems like a regression that should be easy to fix before we deploy. It's probably the one visual clue most users will have indicating that they are logged in vs not.

This is now working!

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

The header is ready for review! This is a comprehensive refactor that does the following:

  • One monolith component was split into eight(!!!) components, which is much more manageable
  • Greatly increases accessibility
    • Changes the top level item element type from span to button, which is semantically correct
    • Top level menu items implement aria-expanded. This was previously implemented on the spans, but that attribute is invalid for that role
    • Mobile menu disclosure button is now a span
    • Allows users with assistive tech to skip over menus if needed (currently submenus open and force users to tab through)
    • Proper labels on all buttons
    • Proper visible focus states
  • Visual changes
    • At wide widths, dropdowns are the full width of the header (per design)
    • Submenus animate on open/close (note all animations respect prefers-reduced-motion media query)
      • This works on all browsers (with great effort)
    • Only one dropdown is visible at a time

Still to do

  • Right now the ā€œSupport Drupalā€ special styling is controlled via checking for the text string in the twig template. This is a bit brittle
  • The right hand ā€œGet startedā€ menu is not yet refactored, and still has a lot of accessibility issues.
  • Submenus don’t respond to the ESC key. It’s a nice touch if they close when that’s hit.
  • The designs for the mobile menu have a background overlay. I have some questions about this before we implement it (fixed? What if people scroll then hit it, etc)
  • The user menu icon won’t change to the avatar of the logged in user
  • Still needs various accessibility fixes for forced-colors (Windows high contrast)
  • All font sizes (and most everything) are in PX. We should have a list of variables that are defined in REMs

Summary

Overall this is a huge improvement from the current version, and is the culmination of a lot of work from both Jayme and I.

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

What can we do to make those references working outside of Drupal (for example: in storybook)?

Or if we have an image component that we want to be able to use with and without Experience Builder.

Is it possible to have a fallback value? Maybe something like

$ref: 
  - module://experience_builder#image
  - theme://mytheme#image
šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

I'm not going to keep re-rolling this everytime some CSS changes. Let's get a review in before fixing the tests and recompiling the CSS.

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

Conflicts fixed!

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

Fixed!

The cause was that Intersection Observer was looking to see if the navbar (which includes the filters) was intersecting the viewport, and if so, stickying it. This cause the navbar to not intersect, which leads to an infinite loop and flickering.

For short viewports, the navbar was intersecting the bottom of the viewport, which I did not take into account.

To fix, I passed in an option to extend the bottom intersection point by 10000px, which is an arbitrary large number that will never be reached.

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

mherchel → changed the visibility of the branch 3536333-schedules-daysfilter-toolbar to hidden.

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

Whoa! Good catch. Should be easily resolvable. Will look in a bit. :)

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

This is ready for review:

A bunch of notes:

  • The original author was using GSAP only to manage the stickiness of the schedule filters. This is way overkill.
  • I refactored the GSAP out, and replaced it with Intersection Observer, which is a native API. My guess is this wasn't widely available when this JS was written.
  • Removing the GSAP saves 100k of JS
  • Functionality should be the same, but without the weird bug in the video above.

Will add testing instructions to the IS.

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

mherchel → created an issue.

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

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

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

is in … does that mean this can be closed? šŸ˜„

Yeah, this should be closed. i'm assigning credit to everyone (I don't see any unhelpful comments), but I'll leave it to the maintainers to close.

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

This looks great.

Tested with geolocation module.

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

Feedback:

  • Would love to see some simple explainer text stating what Drupal Forge is and how it makes money.
  • What does "Express Launch" launch?
  • "Free for a limited time" ... does that mean that we can pay to keep it up longer? Is that the business module?
šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

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!

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

This is ready for review in the private Gitlab.

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

mherchel → created an issue.

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

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).

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

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

šŸ‡ŗšŸ‡øUnited States mherchel Gainesville, FL, US

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.

Production build 0.71.5 2024