Create new SDC component for Umami (umami common footer block)

I have attached the patch with new component for menu footer. please review

Verified on 11.x that the block renders the same as before.

1. +++ b/core/profiles/demo_umami/components/menu-footer/menu-footer.component.yml
   @@ -0,0 +1,45 @@
   +# Everything in this file is optional. Still, the file needs to exist. Adding
   

   Should we remove comments here? Seems like a lot to include all of this in every component.

2. +++ b/core/profiles/demo_umami/components/menu-footer/menu-footer.twig
   @@ -0,0 +1,25 @@
   +    'block-' ~ configuration.provider|clean_class,
   

   Shouldn't configuration be documented in the schema? Wondering if this should be triggering schema validation 🤔

3. +++ b/core/profiles/demo_umami/themes/umami/templates/components/navigation/block--system-menu-block--footer.html.twig
   @@ -1,25 +1,9 @@
   +    title_attributes: title_attributes,
   

   Should we use only here to avoid passing down context?

Addressed feedback in #4, attached interdiff for same. please review

+{{ dump(configuration)}}

Removed dump, attached interdiff. please review

Seems points in #4 have been addressed.

My main concern is about coupling between the component and the host template. This is an issue that happens when refactoring existing code, and will likely be less so when writing components from scratch.

1. +++ b/core/profiles/demo_umami/components/menu-footer/menu-footer.component.yml
   @@ -0,0 +1,48 @@
   +$schema: https://git.drupalcode.org/project/sdc/-/raw/1.x/src/metadata.schema.json
   

   $schema: https://git.drupalcode.org/project/drupal/-/raw/10.1.x/core/modules/sdc/...

2. +++ b/core/profiles/demo_umami/components/menu-footer/menu-footer.component.yml
   @@ -0,0 +1,48 @@
   +name: Umami footer
   

   It took me a moment to find this, so I am posting it.

   https://www.drupal.org/docs/develop/user-interface-standards/interface-t... →

   It seems this is the correct capitalization (sentence like), according to our style guide.

3. +++ b/core/profiles/demo_umami/components/menu-footer/menu-footer.component.yml
   @@ -0,0 +1,48 @@
   +    configuration:
   +      type: array
   +      label: configuration
   +    plugin_id:
   +      type: string
   +      label: Plugin Id
   

   Accordig to the use of this below, this is not an array but an object.

   Also, I believe we should simplify this and skip this. This couples the component to a block, which is not desirable. Moreover, this is only used to build up a set of classes.

   IMHO we should have a prop that allows the host template pass additional classes if we need them. Hence the block will send the provider, but something else will not.

   Same principle applies to the plugin_id.

4. +++ b/core/profiles/demo_umami/components/menu-footer/menu-footer.component.yml
   @@ -0,0 +1,48 @@
   +    type: string
   

   This is an invalid key. Slots do not provide an schema, only some basic metadata.

   Maybe replace with:

     content:
       title: Content
   

5. +++ b/core/profiles/demo_umami/components/menu-footer/menu-footer.component.yml
   similarity index 100%
   rename from core/profiles/demo_umami/themes/umami/css/components/navigation/menu-footer/menu-footer.css
   
   rename from core/profiles/demo_umami/themes/umami/css/components/navigation/menu-footer/menu-footer.css
   rename to core/profiles/demo_umami/components/menu-footer/menu-footer.css
   

   The menu-footer.css should be free of references to host template classes.

6. +++ b/core/profiles/demo_umami/components/menu-footer/menu-footer.twig
   @@ -0,0 +1,25 @@
   +    'block',
   +    'block-' ~ configuration.provider|clean_class,
   +    'block-' ~ plugin_id|clean_class,
   +    'menu-footer-wrapper',
   

   The internal component template should be independent from where it's placed. IMO we should not reference Drupal constructs like "block", or "menu".

   This same comment may apply elsewhere in the patch.

7. +++ b/core/profiles/demo_umami/themes/umami/templates/components/navigation/block--system-menu-block--footer.html.twig
   @@ -1,25 +1,12 @@
   +{% embed 'demo_umami:menu-footer' with {
   +    attributes: attributes,
   +    title_prefix: title_prefix,
   +    title_suffix: title_suffix,
   +    label: label,
   +    title_attributes: title_attributes,
   +    content: content,
   +    configuration: configuration,
   +    plugin_id: plugin_id,
   +} only %}
   +
   +{% endembed %}
   

   Here we are not passing the content slot as a twig block, instead we are passing it as an undocumented prop.

   Also, we should use the simplified syntax when the label and the name of the variable are the same.

     foo: foo,
   

   becomes

     foo,
   

I have addressed above feedback, only point 6 is left. Attached interdiff for same

For point 6 in #9 didn't test yet.

What i see now. We have
- 2 footer blocks which contains title and content
- 2 equal menus (footer menu and recipe tags)

Will try with this approach

Pushed footer block. Gonna check menu in separated commit.

CC failure.

Also if ✨ Create new SDC component for Footer promo Closed: duplicate is being combined will need an issue summary update.

Added `blockfooter` to CSpell dictionary.

There's already blockrecipe in dictionary so it's ok to update it with new block

smustgrave → credited mrinalini9 → .

Closed ✨ Create new SDC component for Footer promo Closed: duplicate as a duplicate.

Tagging for issue summary update now.

Before

After

So only difference I see is that the image is smaller but actually seems better. Think this is good.

Posted comment on the MR

@lauriii this good to be marked back to RTBC?

Going to remark but if you get a chance @lauriii

Moving to needs review to resolve the discussion in the MR

Believe random familiar. Rerunning tests.

As I thought was random

Before

After

May need submaintainer sign off but the image changed in sign, see before/after screenshots. But personally I think it looks better and actually shows the full image, so marking it.

Rebased.

+1 that the image looks better now.

lauriii → closed merge request !4695

Committed 0852886 and pushed to 11.x. Also cherry-picked to 10.2.x. Thanks!

Automatically closed - issue fixed for 2 weeks with no activity.