🇺🇸United States @sea2709

@pameeela
I fixed the issue about node title not decoding html entities. I also update the entity links plugin settings

I removed the download configuration from the admin UI, code and test cases as well. Please let me know if we should add any texts to make it clear about this plugin, and how it works or how to extend more entity types.

@catch
I believe the MR is ready to review.

Thanks @pameeela for your feedback.

Will update based on your comments soon!

I found a workaround solution based on this article https://www.drupal.org/forum/support/post-installation/2018-02-28/how-to... →

Whenever a user clicks on the reset button in an ajax view, I trigger the event RefreshView on that view.

I confirm the patch works and fixed the missing translation on my end. Thanks!

@catch

I added more test cases for enabling multi entity types to provide entity link suggestions! I think the PR is ready for review :-)

IMHO, I agree that we should allow more characters in CSS classes like Tailwind CSS classes with colon, brackets, etc.
I'm concerned when we makes changes for the function cleanCssIdentifier, since this function is used to clean the classes generating by Drupal.

For example, I place a field block in layout builder, that field block has a class like block-field-block:paragraph:grid:field-column if we allow colon in cleanCssIdentifier

I wonder if generated classes should have colons, and it might impact an existing site which has many elements being styled based on the generated classes. We should think twice before applying changes for cleanCssIdentifier.

@catch

Two different entity types, you mean that we enable one more entity type, for example "Contact Form"? Or a test case covers 2 bundles of an entity type? I just checked this test file https://git.drupalcode.org/project/drupal/-/blob/26f5c7d8f4ab548079d2986... , this test case just covers only "page" bundle, I guess I should create a new test case with 2 bundles "article" and "page"

@catch:

I'm not sure if there is anything I need to work on this issue. But I finished things that you listed in #190 ✨ Drastically improve the linking experience in CKEditor 5 Needs work

By default, the suggestion plugin only suggests node entities. I'm not really familiar with unit tests, so I don't know if I need to write some unit tests for cases when modules implementing the hook_entity_bundle_info() to enable entity suggestions?

I created a new branch 3317769-link-suggestions-minimum-2 because I ran into a javascript functional test failed which was not related to the plugin this branch, so I thought the test was failed because the branch was behind the 11.x branch, so I merged 11.x branch into the current branch, and more jobs in the pipeline failed.

When you have a chance, can you review my work? If there is anything else I need to work, please let me know. Thanks!

@catch: I just pushed some changes mostly for cleaning up the EntityLinkSuggestion entity type, and making the ckeditor plugin work. My next step is to fix unit tests. When you have time, can you take a look at my changes? Just want make sure that I'm on the right direction. Thanks!

I'd like to work on this one if there's no one working on it at this point. I spent a couple of hours to dive into the changes @catch did on the new branch and got the ideas.

My first step is to make the update editor configuration work after enabling the entity links plugin. By default, node entity type is allowed to provide link suggestions when the plugin is enabled. In the plugin settings section, we should be able to configure which entity types and their bundles providing link suggestions.

Thanks @pdureau for the updates.

Look forward to errors free when using functions from SDC core! In the meantime, I will use component function :-)

Looks like if I use include function in twig file, I still run into a fatal error. I saw your comment on https://www.drupal.org/project/ui_patterns/issues/3481860 💬 Components require attributes prop defined Active about using component function instead of include and embed twig function?

I checked out the branch 3317769-link-suggestions-minimum, then I went to Basic HTML configuration and enabled "Entity Links" filter, then I clicked Save configuration. Then I got the fatal error

The website encountered an unexpected error. Try again later.

AssertionError: assert(NestedArray::keyExists($subform, $parts)) in assert() (line 857 of core/modules/ckeditor5/src/Plugin/Editor/CKEditor5.php).

Does anyone run into this issue?

I spent some time to debug the issue, and was able to reproduce this bug on a fresh D11 install.

Drupal 11
UI Patterns 2.0.x-dev
Uninstall UI Patterns Devel off
UI Suite DaisyUI 4.0.0-alpha2

I updated the template file web/themes/contrib/ui_suite_daisyui/templates/menu/menu--account.html.twig as

{% for item in items %}
  {{ include('ui_suite_daisyui:button', {
    label: item.title,
    url: item.url.toString()
  }, with_context = false) }}
{% endfor %}

I see that when I render a component in a twig template file, that component doesn't go through the #pre_render process, so attributes prop isn't processed. As a result, the component validation is failed since the attribute prop is a string, but in the schema, attribute prop is an object with the prop type as ui-patterns://attributes

Looks like the error is still presented on my local. I'll have sometime tomorrow, I'll dig into it!

@Kristen Pol:
Here is the document which compares how the sections and components on the homepage are set up on two themes https://docs.google.com/document/d/1OPM_MSm5W53OyNmADL-sfEEurzG8nplk90rT... . Please feel free to edit and comment on it!

Overall, Bluecheese theme is using paragraphs to set up sections for homepage. Some paragraphs are using SDC components, some are using paragraph templates to render fields. I expected to see SDC components are using more heavily on Bluecheese theme :-)

WIP
https://docs.google.com/document/d/1OPM_MSm5W53OyNmADL-sfEEurzG8nplk90rT...

@quietone : I'm using Drupal 10.3.1

I run into this issue where I have a view listing all locations based on a proximity search returned from Solr. The issue happens when there is a distance info in the location row to indicate how far from the location filter to the current location in each row. When a user do a search, the search query will be presented in the URL. So in my case, views row cache should have the cache context 'url.query_args'.
This is the code from StylePluginBase.php

$data = [
            '#pre_render' => [[$this, 'elementPreRenderRow']],
            '#row' => $row,
            '#cache' => [
              'keys' => $cache_plugin->getRowCacheKeys($row),
              'tags' => $cache_plugin->getRowCacheTags($row),
              'max-age' => $max_age,
            ],
            '#cache_properties' => $field_ids,
          ];
          $renderer->addCacheableDependency($data, $this->view->storage);

The patch on #7 🐛 Views row cache do not respects display contexts Active suggested adding the cache context from the display to the rows cache. I think it's a good approach, but I think I will create so many cache renderer records based on the some common contexts from the display like "languages", "url", "url.query_args", etc. for each individual row in a view.

Because of that, I think adding more cache context for views cache row should be specific for views. Most of the cases, we don't need adding these contexts.

I noticed that the we add cache dependency for $data based on the $this->view->storage. So I came up with a solution to implement the hook views_pre_render, in this hook I will add appropriate cache contexts that needed for the rows cache. In details of my case, it looks like this:

function mymodule_views_pre_render(ViewExecutable $view) {
  if ($view->id() === 'someview' && $view->current_display === 'somedisplay') {
    // View row cache doesn't have the cache context query_args, since query_args contains info
    // about the proximity search, and different proximity query search has different distance info
    $view->storage->addCacheContexts(['url.query_args']);
  }
}

I did a test on my on-going project and the performance is improved. The rendering time for the view in which I use a Component per Item (UI Patterns) as a formatter for a View field is improved significantly. I don't see there is much change in performance in other views that are not using the Component Per Item formatter.

Thanks for working on this!

Thanks @just_like_good_vibes,

I'll have some time tonight, I will checkout the dev version and see how it works on my project, will let you know how the performance is improved!

@just_like_good_vibes : Thanks for working on this. Do you think performance is an issue on other features like Layout, Block, Views when using SDC with UI Patterns?

Not sure if it's helpful. In my case, I noticed that the component SDC-button I created hasn't been processed by the function "processAttributesProp" in ComponentElementAlter.php , while the other components have. I only see only button component encounters this issue. In my case, I don't think it's because of views, will share more information when I figure out something!

@just_like_good_vibes: Thanks for looking into this.
I'm using 2.0-beta3 version.

If I clear cache and refresh the page, the rendering time is around 3.2 seconds. Then I refresh page again, this time the rendering time is around 2.7 seconds. I think from the second refresh attempt, the rendering time is reduced as 2.7 seconds.

I'm using ddev with default D11 site, just install a couple of modules for testing.

sea2709 → created an issue.

g4mbini → credited sea2709 → .

griffynh → credited sea2709 → .

I encounter this issue as well. I notice that in some cases, instead of putting a real space, the editor puts a  

The patch #27 🐛 Issue with HTML ` ` not being correctly filtered out from URLs Needs work works on my project. I'm a little bit concerned about if this is the root cause of this issue. I think the issue is from the editor more than the filtering process.

you could also try to define broad CSS variables with default values or use CSS layers so that they are easy to override.

Yeah, this is what I meant, we need to have a guidelines about how to build outside / dependent components to make them work well with themes later on.

One more thing came across my mind is when we have a set of base components and apply these components for different projects. So there might be a chance that we want to add more properties, more slots for components. From what I understand, we don't have this ability at this moment, the way we do is to replace a component by another component in case we want to extend it.

I see that many components using variables from the theme, especially theme's colors, font styles, etc.. Just wondering, if components are something separate from the theme, will we need to have some guidelines or standards about how to define variables or how to pass variables from theme to components. Just want to share my thoughts!

Thanks @pdureau,

It's helping! This code works for my case on beta2, with and without only!

{% embed 'careerforce_skin:sdc-button' with { attributes: attributes.toArray(), text: attributes.value|raw, variant: variant} only %}

Thank you for your help! I guess I need some time to digest your response, I'm a little bit exhausted at this moment, but I'm glad that it works :-)

Thanks for looking into this.

I confirm your first test works on my end too. I did some tricks to overcome to the issue you mentioned 😅 , anyway, I saw a fix for that issue and it worked. I tried again to put a label, but it doesn't show up!

I confirm that the fix to skip validation works. I encountered the same issue, but there was no error was log in the Drupal logs, after some debugging, I found that the error was from the file web/core/modules/views/src/Plugin/views/style/StylePluginBase.php in the renderFields function, at line 661

I wonder if there is a way to log this kind of errors in the Drupal logs?

Thanks for your response. Not sure if my implementation is correct. The idea is I would like the submit buttons are rendered by using a button SDC component, so that's why I created a twig template file input__submit.html.twig and rendered the button component here.

This piece of code went well on UI Patterns 2.0.0-beta1, I only encounter the invalid component exception after I upgraded to beta2.

{% embed 'careerforce_skin:sdc-button' with { text: attributes.value|raw, variant: variant} %}
  {% block children %}
    {{ children }}
  {% endblock %}
{% endembed %}

I did some debugging and this is what I found

On beta-1, during the component validation process, the props variable has 2 properties variant and text

On beta-2, during the component validation process, the props variable has 3 properties variant, text and attributes

I think attributes should be an object or an array, not a string. I guess maybe somewhere on UI Patterns 2-beta2, the attributes prop is converted to a string

sea2709 → created an issue.

Thanks everyone for reviewing and making changes on my work 🤩🤩🤩 Such a relief when this one is shipped 😀

@jessebaker, @bnjmnm
I made some changes in the code to calculate the size of the component wrapper in iframe to fix the issue when the drupalicon component is rendered as a square.
I tested the preview component feature with Starshot demo design system components, and this is how it looks
https://drive.google.com/file/d/13MIMsw0fxyf1fsXD21wEMMhCm0wtBzSn/view?u...

The slider component preview in my test is not quite right, not sure if it's the nature of this component, or should we need some treatments for it?

During the development, I found a bug related to Radix Tooltip where the tooltip content is rendered twice, this bug is reported at https://github.com/radix-ui/primitives/issues/3034

I move the status to Needs Review, even though Slider component preview isn't rendered well. Happy to have other thoughts? :-)

This is the patch that I'm using on 1.18, just a small change!

I'm using the latest version of Paragraph 1.18 , I don't see this issue, but I see some warnings while using layout builder to editing a layout and I think we can improve this merge to avoid the warnings.

Assigned it back to me to check points that @bnjmnm mentioned!

sea2709 → created an issue.

@jessebaker: Thanks for resolving the Cypress issue and your suggestion changes, I applied them and it worked well :-) Learn new things today :-)

@jessebaker / @Wim Leers :

The MR https://git.drupalcode.org/project/experience_builder/-/merge_requests/3... is ready to review. The job Cypress E2E is failed at the test case "previews components on hover" https://git.drupalcode.org/issue/experience_builder-3469856/-/jobs/2737246, I need someone to support me fixing this test case since I cannot set up Cypress on my local, it's hard for me to make it right!

This is the video about how this feature works on my local https://drive.google.com/file/d/1dKujRCNeg4XvvoyXM6BItv8CKvh09o9V/view?u...

Sorry for taking it so long, I was kind of struggling on how to handle the iframe without much flickering and resizing it appropriately :-)

sea2709 → changed the visibility of the branch 0.x to active.

sea2709 → changed the visibility of the branch 3469856-3 to active.

sea2709 → changed the visibility of the branch 0.x to hidden.

sea2709 → changed the visibility of the branch 3469856-2 to hidden.

sea2709 → changed the visibility of the branch 3469856-3 to hidden.

sea2709 → changed the visibility of the branch 3469856-3 to active.

I confirm the MR fixed the issue when the title is showing twice on my end!

I noticed the latest patch is applied in latest version. From my understanding, the patch allows anonymous users to redirect to a user page which has the user id parameter the same with the logged in user. I have concern about the case, for example, an admin user would like to redirect to a edit profile page of an authenticated user after the admin logs in, so the URL should be https://example.com/user/login?destination=/user/345/edit , it seems this patch forces the admin the land on his or her own user page.

This is the patch that I'm using, it inherits from #27 🐛 CSS class validation too strict Needs work , I just updated the regex as
/[^\x{0026}\x{002D}\x{002E}\x{002F}\x{003A}\x{0030}-\x{005A}\x{005F}\x{005B}\x{005D}\x{0061}-\x{007A}\x{00A1}-\x{FFFF}]/u

So I allow these characters
{0026} &
{002D} -
{002E} .
{002F} /
{003A} :
{005F} _
{005B} [
{005D} ]
numbers
letters A-Z a-z

I ran into that issue as well. I upgraded from version 2.9 to 2.10 . But the issue went away after I cleared cache. I think it's just a cache issue, when in version 2.9, the IconsetFinderService requires one argument in the constructor, and in version 2.10, it requires two arguments. The social_media_links.services.yml is also updated from 2.9 - 2.10

sea2709 → changed the visibility of the branch 3469856-3 to hidden.

sea2709 → changed the visibility of the branch 3469856-heading-preview-component-style-not-correct to hidden.

Thanks for pointing it out :-) I dig into CKEditor 5 plugin development a couple of months ago, and it was crazy! :D

@jessebaker: Thanks for your feedback :-) Will improve the UX based on your inputs!

I like how Gutenberg editor in Wordpress implements this feature https://wordpress.org/gutenberg/ and learning its implementation, still not figured out yet, but will dig into it more.

Thanks @mogtofu33 for working on this! I'm sorry for taking a little bit long to response!

I tested alpha3 release on a fresh D11, I did see the insert icon button in the editor toolbar.

I also did a test on my current project, the latest dev fixed the issue. I'm just curious, why the alpha version doesn't work on my current project?

https://git.drupalcode.org/issue/experience_builder-3469856/-/pipelines/...

I just updated the Cypress E2E scripts for previewing components, it's working now, yeah!!! It took a while since I can't set up Cypress on my local!
I see the job UI eslint fails, I think it failed after I merged branch 0.x into my branch, so maybe there is some in progress UI eslint work on 0.x branch.

@Wim Leers: When you have a chance, can you take a look at this MR https://git.drupalcode.org/project/experience_builder/-/merge_requests/241 ?
This is how it works on my local https://drive.google.com/file/d/1hr2WXAg8V-xX2oC91i7oDHO78dYZHUO0/view?u...

This MR is using iframe and attach a src attribute to iframe for component preview. At first, I was about to use srcdoc for the iframe by building a preview HTML for each component, but then I saw this issue https://www.drupal.org/project/experience_builder/issues/3471070 🐛 [Performance regression] Loading the components takes >5s Active , I changed the implementation to build a component preview route.
I noticed that the MR's pipeline https://git.drupalcode.org/issue/experience_builder-3469856/-/pipelines/... is failed at the job cypress E2E, I searched through source code test cases we implemented, and found test cases that are checking CSS classes for ShadowDOM, since I'm no longer using ShadowDOM then the test cases fail. I'm not familiar with doing Cypress E2E test cases, but can give it a try if you think using iframe is a good approach.

Thanks :-)

Working on failed jobs in the pipeline

I did a test on components using Shoelace library, I use both Shadow DOM and iframe, all the inputted HTML are the same for both cases.

This is the HTML structure inside the Shadow tree

And this is the HTML structure inside the iframe

I think the iframe is more reliable when some components need some scripts to render.

@Wim Leers
I updated to use iframe component preview. This is the video I recorded to see how it works on my local
https://drive.google.com/file/d/1shf5KCvelJNG90R6VeCrO4KfGIQeof19/view

The preview popup shows up not really smoothly. Since it is an iframe, beside the component CSS file, it loads other theme libraries, so the rendering is kind of lagging. I put a script to resize the iframe height after it is loaded completely, originally, the iframe size is 1200px * 1600px, most of components are much shorter than this size, so I updated the height, not sure if it is needed.

I see that if we use iframe, we can get these things done:
1. Components can use CSS variables that are defined in the theme
2. Some components require executing scripts, like some sample components from Shoelace that we are using.

But the downside of it is to load many resource files then it takes some time to render.

I'm not concerned about the case when the iframe renders a mobile view instead of a desktop view since the preview container is small. We set the width for the iframe as 1200px, and then we scale it down, so there is no reason that the mobile media query takes effect.

sea2709 → created an issue.

sea2709 → created an issue.

If that doesn't/can't work, then we'll need to start using

s for the component previews too.

@Kristen: The component preview is scaled down 50%, which makes many previews look so tiny. The preview wrapper is 600px width, and it is scaled down 50%, so the width is 300px.

@Wim Leers: Not sure if we already discussed about the preview size. I see we do some calculation for the scale ratio, but based on the code logic, it's always 50%

@Wim Leers:
I see that when we are using edit mode, we don't load the base libraries from the theme. Some of our components are using variables defined in the theme. There are 2 approaches acrossed my mind:

1. The base theme libraries should be loaded when previewing components. I tried this way, but ran into a problem when the CSS custom properties defined at the :root level, and they are not available in Shadow DOM, for example, in Olivero theme, they are

:root {
--sp: 18px;
...
}

This approach will work if we add :host, otherwise these variables cannot be applied in Shadow DOM.
I'm not sure if at the component level, when we use variables, we should define a default value or not!

2. The base theme libraries should be loaded when we are in XB editing mode, so all CSS custom properties defined on :root are available in shadow DOM, I updated my MR following this way.
So there are theme libraries loaded when we load XB

This is how I loaded the theme libraries

$base_page_build = $this->bareHtmlPageRenderer->renderBarePage([], '', 'page');
    $libaries = $base_page_build->getAttachments()['library'];
    $libaries[] = 'experience_builder/xb-ui';
    return (new HtmlResponse(self::HTML))->setAttachments([
      'library' => $libaries,
     ...

I don't have a strong opinion for this case, need some suggestions, and I'm willing to change my implementation for a better approach :-)

@Wim Leers
I noticed the heading component style was like that

[data-component-id="experience_builder:heading"] {
  &.h1 {
    letter-spacing: -0.01em;
    font-size: 1.75rem;
    line-height: var(--sp2);
  }

Based on the twig files, h1, h2, h3, etc should be tags not a class. And in case, we have heading tags, they should go first before the attributes in CSS styling.

I do see CSS variables not getting their values, let me check it!

MR is created. The issue is not because of Shadow DOM, issue is in the heading component styling itself. I updated the component style. For the preview block, I remove the white text, and add a white background.

@Kristen: I removed unnecessary dependencies from composer.json of starshot_demo_design theme , I updated the package name as well (hopefully it's appropriate). MR is ready for review https://git.drupalcode.org/project/demo_design_system/-/merge_requests/25

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

The documentation is shorter :-)

There is one thing we might need to pay more attention, in `composer.json` of the demo_design_system theme https://git.drupalcode.org/project/demo_design_system/-/blob/1.0.x/compo..., it requires some modules

• Components
• Field Group
• Focal Point
• etc.

These modules are included in Drupal CMS, but not in D11. I think missing these modules might break some features in the theme, and I don't see the inserting dummy data feature in the theme settings, maybe because of missing some modules.

g4mbini → credited sea2709 → .

@akhil babu: I'm glad it works. I'm not sure about the authentication you ran into, but it looks like it just happened at the very first time.

@Kristen: For #2, I created a new issue and a MR to fix the issue as well. https://www.drupal.org/project/demo_design_system/issues/3469462#comment... 🐛 Cannot run storybook for starshot_demo subtheme Needs review

@Kristen: I created a MR to fix this issue. When you have a chance, can you review it? Thanks!

sea2709 → created an issue.

@akhil babu:

The first warning "No composer.lock file present. Updating dependencies to latest instead of installing from lock file", I think this one is normal, we don't have composer.lock in Drupal CMS repository, so developers can get the most up to date modules versions when they set up their local. When ddev start is run, composer install is kicked off, it's just a warning that composer will generate a lock file.

For the authentication error, I'm curious if you can see the recipes "notify_new_comments" in your project source code, it should be at "web/recipes/notify_new_comments". I think you can do a test to remove the folder "web/recipes/notify_new_comments" and then do "ddev composer install" again to see if this recipe is created back. If it shows up, then we're good :-)

Hi @Kristen,

I followed the documentation to set up my local. I was able to set up the site successfully, I just have a couple of comments

1. At the step "Provision content", it might be helpful to add a step to set the default theme appropriately (CivicTheme or Starshot Demo) before creating dummy content and structure.

2. I was not able to run storybook for Starshot Demo theme, not sure if this issue is reported or not, but there is a typo at line 18 in the file https://git.drupalcode.org/project/demo_design_system/-/blob/1.0.x/stars...
{% embed '@base/starshot-container/starshot-container.twig' wit {constrain: true} %}
it should be {% embed '@base/starshot-container/starshot-container.twig' with {constrain: true} %}

3. I was able to check experience builder by following the documentation steps. I think there have been some changes in XB recently that we need to do more steps. Based on the XB documentation https://git.drupalcode.org/project/experience_builder/-/blob/0.x/CONTRIB..., XB only works with article content type at this moment. And there was no article content type on my dev site, so I installed the standard profile "ddev drush si standard", and then enabled experience_builder. And the next step was to run "npm i" and "npm run build" in the folder web/modules/contrib/experience_builder/ui". I didn't need to run this step before, but on the latest XB version, I needed to add this step!

Thanks @jurgenhaas for your tip. Yeah, I didn't think about a token with bracket as value.

When I dug into the code, I noticed in the Actions.php (service class for Drupal core actions in ECA), it adds to the action plugin configuration an option "replace_tokens". I'm curious, how this setting is being used and can we expand this option to other plugin actions?

I also noticed that in the action Render: Add Class, the machine name field is a token support field as well. I would like to set the machine name as "wrapper][form][submit", and the name ends up with wrapper][submit. I'm not sure if it makes sense that machine name doesn't support token, or there should be an option to enable token support or not!

sea2709 → created an issue.

sea2709 → created an issue.

pameeela → credited sea2709 → .

Just a heads up, I did some testings to remove attributes property from the YML files, and it's still working, I can pass attributes to components. So I guess, attributes is a built-in property in a component, and we don't need to define it! Please correct me if I'm wrong!

Hi @Kristen,

I think we might need to review how we should set up attributes property in SDC components. From my understanding, attributes property should have the type "Drupal\Core\Template\Attribute".

When I checked the component map (02-molecules/map), I noticed that in this component template, we include 2 other twig files for rendering iframe and button. In my opinion, when we convert these components into SDC, we should render components instead off including twig files.

I just want to demonstrate how I think we should use SDC components, when you have a chance, can you take a look at this commit https://git.drupalcode.org/issue/demo_design_system-3468125/-/commit/288... ? Not sure if it's a correct implementation (it breaks the storybook), but I think we might consider about attributes property and how to render components inside components.

sea2709 → changed the visibility of the branch 3468125-test-sdc-conversion to active.

sea2709 → changed the visibility of the branch 3468125-test-sdc-conversion to hidden.

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