mherchel ā created an issue.
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!
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.
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.
mherchel ā created an issue.
mherchel ā created an issue.
mherchel ā created an issue.
This is something that most SDC authors would make use of, and it would result in better UX's for end users.
mherchel ā created an issue.
mherchel ā created an issue.
mherchel ā created an issue.
mherchel ā created an issue.
(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.
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:
- I'm setting template suggestions for various blocks based on what regions they're in.
- So the primary menu block placed in the header_third region calls menu--region-header-third.html.twig
- 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)
- 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
- 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...
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.
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.
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.
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.
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.
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.
I want to add that I run into this issue all the time, and also fix it by adding 'null'
as a type.
mherchel ā created an issue.
Screenshot here! Let me know if this should be a separate issue:
mherchel ā created an issue.
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?
mherchel ā created an issue. See original summary ā .
mherchel ā created an issue.
mherchel ā created an issue.
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).
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 ā ļø
mherchel ā created an issue.
I would like to submit this for the "Module previously known as Experience Builder"
OK. I set the summary to trim at 800 characters. Should be good to go!
mherchel ā created an issue.
Updating CDN path. Removing D7 reference
Big +1 to this!
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.
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.
mherchel ā created an issue.
Testing instructions
mherchel ā created an issue.
Updating title.
Testing instructions
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?
I like it! Makes it a bit more clearer to the dev that this is very non-standard.
Pushed!
g4mbini ā credited mherchel ā .
Opened followup issues:
- š Refactor āGet startedā menu at desktop Postponed
- š Menus items should close on focus out (both at desktop and mobile) Postponed
- š Menu not accessible in forced colors mode Postponed
- š Mobile menu should have overlay when open Postponed
- š Menus should close on ESC Postponed
mherchel ā created an issue.
mherchel ā created an issue.
mherchel ā created an issue.
mherchel ā created an issue.
mherchel ā created an issue.
Thanks so much Putra! I'm sure we'll run into each other again soon!
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
Just pushed styles for "Get started" at mobile widths. This will also need to be a <button>
element instead of <nolink>
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
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!
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.
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
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.
Conflicts fixed!
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.
mherchel ā changed the visibility of the branch 3536333-schedules-daysfilter-toolbar to hidden.
Whoa! Good catch. Should be easily resolvable. Will look in a bit. :)
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.
mherchel ā created an issue.
mherchel ā made their first commit to this issueās fork.
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.
volkswagenchick ā credited mherchel ā .
This looks great.
Tested with geolocation module.
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?
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.