Can Picafort
Account created on 11 July 2009, over 15 years ago
#

Merge Requests

More

Recent comments

e0ipso Can Picafort

I had to resolve merge conflicts on the README.md

e0ipso Can Picafort

Thank you everyone for your contribution!

e0ipso Can Picafort

Thanks for this contribution!

I think this change is not 100% safe. Maybe we should add an additional check that there is a sibling package.json for extra certainty.

e0ipso Can Picafort

Can you elaborate on what is the issue here? The issue summary is too brief.

e0ipso Can Picafort

e0ipso made their first commit to this issue’s fork.

e0ipso Can Picafort

I am wondering why Drupal\Storybook\FileUrlGenerator is not doing its job here. Can anyone here debug through this?

e0ipso Can Picafort

There is a section in the README about ddev, perhaps this could go in there. I think this is new in SB8.

e0ipso Can Picafort

You are not supposed to write JSON files with stories, please check the documentation for more info.

e0ipso Can Picafort

Adding a patch for the MR as of #9, so we can add it to composer patches.

e0ipso Can Picafort

e0ipso made their first commit to this issue’s fork.

e0ipso Can Picafort

e0ipso made their first commit to this issue’s fork.

e0ipso Can Picafort

e0ipso made their first commit to this issue’s fork.

e0ipso Can Picafort

I think the tokens road is the my preferred, but I could go both ways.

e0ipso Can Picafort

I know that I could create a theme or module and them have my components there, but it kind of feels like an unnecesary / annoying requirement.

When we were on the drawing board, this was considered. However it was discarded because Drupal does not have a way to deal with name collisions in composer packages. It does however have one for modules. This way we were able to reliably determine the component prefix.

The important question here is: why does it feel like an unnecesary / annoying requirement?

I'd like to clarify this part of the IS before moving forward with this.

e0ipso Can Picafort

Pagination on the cases studies carousel breaks a bit in desktop on the second slide:

e0ipso Can Picafort

Content wise, there were three things that jumped out to me.

The most severe one is that we are involving ourselves with Nestle in Drupal's biggest redesign and marketing push. I know this is no news, but Nestle is one of the most evil companies in the world. Concerns about Nestle include slavery, child labor, deforestation, preventing access to non-bottled water in impoverished countries, ... Please take a look at Wikipedia's page dedicated to this for sources backing all these proven claims. I feel ultra strongly about this. I am convinced this goes against Drupal values, the community values, and individual values. I don't want us to be the focus of controversies like ADF and Project 2025 (which are US centric), but on actual slavery, childe labor, etc.

My second concern is "50% of Fortune 500 companies use Drupal". My reaction, as a developer and contributor, was oh... am I supposed to volunteer my time for the super-rich? I guess I should think about it. I am unsure if this is the expected reaction. Perhaps we should highlight how these contribute back to the common good.

My third concern is that I was expecting Drupal recognized as a public good being front and center in the messaging, but is seems that is not there at all (unless I missed it). I think this is the kind of message that will attract companies we want to volunteer for (and that tend to actually contribute back).

e0ipso Can Picafort

Added an example of a required slot.

e0ipso Can Picafort

I made some minor modifications to the patch.

e0ipso Can Picafort

Adding docs around the library names.

e0ipso Can Picafort

Thanks @apaderno. Is there any way to update the Created by field to use my user instead?

e0ipso Can Picafort

Thanks @apaderno!

Is there any way to remove this reference?

e0ipso Can Picafort

Hello.

It's been more than 14 days now. Did the maintainers respond to the queries?

e0ipso Can Picafort

One small suggestion, do not ever edit an accepted record. Only make cosmetic changes to it. If you need to change it, deprecate it and write a new one. Write why you are deprecating it on the new one. Links back and forth should be present in both ADRs.

WRT #12 we have seen much better results when the ADRs are in the repo. In our experience, putting them as close as possible to the code helps developers maintain them.

e0ipso Can Picafort

I think people can probably figure out that "Single Directory Components" in a slide deck refers to a thing that is officially called "Single-Directory Components".

Agreed. This is exactly what I meant when I said I think this is a minor issue, so I side with the inconsistency., but more eloquently put.

e0ipso Can Picafort

If you are curious about my confusion in #8, I was trying to write a test for 4, before I wrote one for 2:

4. Have embed within extends and forward blocks from the parent to the children.

However, that is not valid Twig. And that is what a I was realizing in #8.

The commit above adds test coverage for extends, and we should be good now.

e0ipso Can Picafort

Please ignore my comment in #8 🤦. I was conflating two different issues here.

To recap this issue proves that we can:

  1. Have embed within embeds.
  2. Have embed within extends.
  3. Have embed within embeds and forward blocks from the parent to the children.
e0ipso Can Picafort

This is good for me. I didn't know about this grammatical rule, and it didn't come up earlier in any of the reviews.

My only question is about documentation. The status quo is that code and documentation are consistent bur wrong. Changing this in code is nice and easy, but now we have inconsistent documentation (official Drupal.org docs, video keynotes, DrupalCon presentations, DrupalCamp presentations, contrib project pages, slack messages, etc.)

I am OK with the change, and I will try to use the new for moving forward, if we agree on inconsistent docs. For the record, I think this is a minor issue, so I side with the inconsistency.

e0ipso Can Picafort

To make it clearer: I would not consider the intention to commit code in the next 30 days a reason sufficient not to transfer the project ownership, considering that the project node does not contain any information about the project.

Could this be a reason to speed up the process?

e0ipso Can Picafort

Moved to the project ownership issue queue after 2 weeks without a response (even if this qualifies as an "empty" project).

e0ipso Can Picafort

@alexpott while working on an example with extends I realized that the trick to forward block contents will not work on extended templates. This is not related to SDC, but a Twig limitation.

{# extends-example.twig #}
{% extends 'sdc_test:just-a-slot' %}
{% set slot_value %}
  {% block nested_slot %}{% endblock %}
{% endset %}

{% block the_slot %}Foo {{ slot_value }}{% endblock %}

In the example above there is no scope for extend-example.twig to grab the contents of the block nested_slot.

Since we are extending another template it does not make sense to introduce a new slot/block, you just use the blocks defined by the parent instead. Therefore I think now, that I was wrong in #2 to suggest that we need to support extends.

Back to RTBC based on the above.

e0ipso Can Picafort

Thanks! I added this to the project page.

e0ipso Can Picafort

For the record, I reached out to Mustafa Hajmohammed (mustafa.hajmohammed) three more times after the comment above without a reply, so far.

I contacted the user:

  1. 28/05/2024
  2. 29/05/2024
  3. 31/05/2024
e0ipso Can Picafort

I have been doing this for a while in Drupal using https://github.com/e0ipso/schema-forms-php

However, I think that the current approach is geared towards using field widgets instead. In other words, instead of mapping the schema to the form elements, it tries to map the schema to the typed data in field types. Once you have the field type, you can load the widget for it.

e0ipso Can Picafort

Test only patch is failing with the expected error: https://git.drupalcode.org/issue/drupal-3446933/-/jobs/1730262#L45

Twig\Error\SyntaxError: An exception has been thrown during the compilation of a template ("We found an unexpected slot that is not declared: [the_slot]. Declare them in "my-banner.component.yml".").

The pipeline with the fix is green.

This is ready to review.

e0ipso Can Picafort

At this point, it feels that this issue is only about a name convention. Variant is a regular prop: we can do that today. The template is managed manually: we can do that today.

The only remaining value is to ensure that people uses variants with the same name, and thus it becomes introspectable metadata that we can use down the line (add a selector in XB?).

Should we update the IS?

e0ipso Can Picafort

In the beginning, all of these actions are achieved through a low-code experience using a code editor within the browser.

I have to admit I can't get over my initial reaction about a "code editor within the browser", it gives me pause. What is the rationale behind this requirement? I would prefer, if possible, to not do this and jump straight to the no-code solution, without this stepping stone.

The design system may also have no-code configuration options (e.g. color schemes, fonts, spacing, etc) to enable non-technical builders to create experiences tailored to their brand and content strategy.

Yes!

Exposing a component to the Experience Builder should be as easy as it is with competing products.

Any examples of what it is like with competing products?

Thanks for this writeup @lauriii. My favorite part is that technical-builders and low-code builders are on the same track, even if using different tools. I think that having significantly different theme building experience would have eventually sidelined one or the other. I know we are focusing on the ambitious site builder persona, but at the end of the day there are lots and lots of technical builders building Drupal sites that we need to keep prioritizing. I see this vision supporting that.

e0ipso Can Picafort

From experience, I believe most of the use of variants will be in class names, as BEM modifier or equivalents in other naming methodologies:

AFAICT you can accomplish that with a regular prop today, no need for this ticket to land.

this feature is optional (I don't have to create different template every time I create a new variant)

Couldn't you load templates manually from the main template based on the variable? E.g. {% include "banner--" ~ variant ~ ".html.twig" %".

Just pointing out that this two statements may appear to be in conflict, but I believe they are not. We need to ensure the template file exists before doing the template swap.

the variant prop will be injected into the template

I think this may belong to the componentMetadata object that gets injected. Thoughts?

I am still unconvinced about making the user request the variant using a seemingly regular prop. It feels like bad UX. I only see two inputs for embedding a component: the template name (my_module:my-component) and the template context (with {...}), so we don't have many options.

If we are to introduce props with render logic, perhaps we could introduce an unlikely reserved prop for those. Then users would write:

{% embed 'my_theme:cool-component' with {
  unlikelyReservedNameUpForBikeshedding: { variant: 'my-variant-name' },
  prop1: 'foo',
  prop2: 'bar',
} %}

In the future we could add more stuff in there (aside from variants), should we need to do so.

e0ipso Can Picafort

Mustafa Hajmohammed (mustafa.hajmohammed) just replied to the email sent on 24/05/2024 agreeing to the transfer. He will take action shortly.

e0ipso Can Picafort

Thanks for the vote of confidence! I can confirm that I have read the sections linked by @xjm in #12, and I agree to follow the principles in them.

e0ipso Can Picafort

I want to hear @pdureau's take on this. Let's see if I can lure him in to add his thoughts before we decide on anything.

e0ipso Can Picafort

This summary above is part of my conversations with @penyaskito on the topic. Please, consider granting credit to him.

e0ipso Can Picafort

I believe BC will complicate things here, now that SDC is stable. But I am getting ahead of myself.

I think we have to:

Define how to call the variant from Twig

This is the template ID. If we want to keep ourselves to native include / embed / extend we need to create a new naming convention. Potential ideas:

include('<provider>:<component-name>#<variant>', ...)
include('<provider>:<component-name>.<variant>', ...)
include('<provider>:<component-name>--<variant>', ...)
include('<provider>:<component-name>|<variant>', ...)

Define the naming convention for the template file name

Here we should probably mirror the decision above.

<provider>/components/<component-name>/<component-name>--<variant>.twig
<provider>/components/<component-name>/<component-name>.<variant>.twig
<provider>/components/<component-name>/<component-name>|<variant>.twig

Connect the template ID with the filename

The ComponentLoader will receive a template ID, and will be in charge of finding the template file name, as defined above.

For the render array way of embedding things, the ComponentElement will take a new optional #variant (as proposed in the IS), and will compose the necessary template ID. Everything else is the same as using Twig to embed the component.

The component plugin Drupal\Core\Plugin\Component

At this moment we access the template for the component via a public property $component->template. However now the template to be used for the component will depend on the variant. We need to introduce $component->getTemplate($variant) (which could throw a new MissingComponentVariantException). This is a problem for backwards compatibility.

Proposals to tackle this issue should include a plan for backwards compatibility.

e0ipso Can Picafort

Embedding nested components is done the same way you would do it in a regular twig template. Perhaps this example can help: https://git.drupalcode.org/project/sdc_examples/-/blob/1.x/components/si...

e0ipso Can Picafort

Thanks @mortona2k and @freelock! Please, ping me in Slack when you want my attention back to this issue.

e0ipso Can Picafort

This may also be true for field mappings correct? I mean, rendering a field that contains an integer to pass it to a component will yield "3" and not 3 as we want.

Updating the Issue Summary to reflect this.

e0ipso Can Picafort

e0ipso made their first commit to this issue’s fork.

e0ipso Can Picafort

I think that before we can move forward with this, we need to clarify what will be the role of Experience Builder in this space. I cannot imagine a first-in-class page building experience that doesn't implement all of those features.

e0ipso Can Picafort

My take is that it is seldom useful to contribute a single component. I am sure the use case exists, but it will not be the norm for sure. I think that a "bundle" of components (a school of components? a gaggle of components?) will be the norm. Like in other technologies.

I agree that this feature is very useful. We thought about it a bit when designing SDC. Our idea was that a this bundle should be a module. You can have a module that only contains components. Components are discoverable within a module, and have rich metadata (like thumbnails, README, schemas, descriptions, etc). It is not too far fetched to imagine that you can search for an accordion component in project browser, and you find the Simple Components Extras Plus module.

Using modules has certain key advantages:

  • They already have documentation on how to author them, how to download them, how to enable them.
  • They already have the technical tools to author them, download them, enable them.
  • They have the concept of versioning built in.
  • They have the concept of a dependency chain built in.
  • A component cannot currently exist in isolation since you put them in the page by: my-module-or-theme:my-component
  • Modules can also house dependencies for the components, like new Twig filters.
  • Modules can also contain tests. And our CI tools also know how to run those tests.
  • We have tools to upgrade modules (and the components within)
  • ...

This seems to go inline with some of the comments in this issue.

There is also the fact that we'll download components, and we will need to tweak them later on. For this to be possible a component will need to have their schema in the component definition.
e0ipso Can Picafort

Thanks for helping move the conversation forward @dafeder!

I am adding a note to the IS to note that Composer also depends on this package. This might add some additional confidence in adding this as a runtime dependency for Drupal.

e0ipso Can Picafort

🤣 this made me laugh @mherchel!

Yes, I am interested in being added as a maintainer.

e0ipso Can Picafort

Pierre, like I already told you, thanks for being part of this community, and for putting up with me 😜




e0ipso Can Picafort

I think that the field group should appear in the mapping options (as stated in the IS), and the fields within that fieldgroup should not. Thoughts?

e0ipso Can Picafort

I see some of the stories are plain text for the slot content. I would love to see them with HTML markup to ensure they are not improperly escaped.

e0ipso Can Picafort

Another potential example to look at is the Carbon Design System, which is Open Source.

Is the intent to have a list of components we may want to support? Or are we looking for an actual set of components? If we are, we will likely need the design system for them (or we can extrapolate it if we are building the components for an existing theme like Olivero).

However, there is a chance that what we are looking for theme independent components. If that is the case, we might want to discuss first how to best implement those.

e0ipso Can Picafort

I wonder if we could solve this by just adding use DependencySerializationTrait; to the class. Any chance you can try that?

e0ipso Can Picafort

Ah! Thanks for the clarification @Wim Leers. I was taking it as a "there is no need to dwell too much in this now, since we'll be able so solve it with a build step".

To pile on that spirit that @effulgentsia brings on #33, I'll say that using industry standards will bring us more and more of these tools for free. Not only to generate the schemas, but also to read and leverage them. Staying true to the JSON-Schema standard will also allow us to auto-generate forms for the component props, synthetic examples, etc. Carving out (more) exceptions like drupal-image will make these tools fail without specific handling.

e0ipso Can Picafort

While I am an advocate of TS (I even pushed an ADR at Lullabot to the effect), I do not think that we should prescribe it in order to write re-usable schemas. This is specially true since we are not writing JSX components in Drupal (yet).

In any case the import method is a bit similar. In one case we defer to the native module loader and a URI, and in the other we use a custom code loader and a URI.

Production build 0.71.5 2024