Convert Olivero's tags to use single directory components

Converted Oliveros tags to single directory component

CC failure in the MR.

Tried testing though and after applying the MR and clearing cache I get this error

Twig\Error\LoaderError: Template "olivero:tags" is not defined in "core/themes/olivero/templates/field/field--node--field-tags.html.twig" at line 42. in Twig\Loader\ChainLoader->getCacheKey() (line 99 of vendor/twig/twig/src/Loader/ChainLoader.php).

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

After a quick review, this is looking really good!

The only quick nit is that you can simplify your include function.

instead of doing a

{{ include('olivero:tags', {
  attributes: attributes,
  label_hidden: label_hidden,

you can simplify to

{{ include('olivero:tags', {
  attributes,
  label_hidden,

Also, I'm guessing that the tests are failing because SDC is not being enabled by default. So tests won't pass at this point

This is looking just about perfect! The only major change is to assemble the classes and title_classes arrays within the field--node--field-tags.html.twig file.

Note that tests will not pass on this because SDC is not a part of standard profile. Once this gets to a good place we'll postpone it until SDC becomes stable.

Changes look good (with my limited knowledge of SDC). I'm assuming the failures are due to SDC not being in standard. Postponing issue until that happens.

Gonna work on it

I started a new approach because I believe that styles in components should not be tied to Drupal. This makes them more flexible.

So it shouldn't be
.field .node-- and other things

Just pure simple and clear css

Also fixed flexboxes and added support for label above

Please check!

The styles are looking as expected. Only thing i can figure out is the flex direction is column now. Attaching screenshot below

Yeah. I've added above/inline label display support.

@finnsky, This makes sense and the above/inline field display is working as expected.

Inline:

Above:

smustgrave → changed the visibility of the branch 3365389-convert-oliveros-tags to hidden.

smustgrave → changed the visibility of the branch 3365389-sdc-olivero-tags to hidden.

If we are adding additional functionality believe the summary needs to be update to include this is more then just converting to SDC.

Also this changes the current design with the label no longer being inline, which may need sub-maintainer sign off for that design change.

Thanks.

more then just converting to SDC

we should never just converting ;)

SDC give us chance to do it better.

Normally 100% agree but would say in this case think getting to SDC first and in follow up doing enhancement would be best, just my opinion, or both seem to get delayed.

Hi,

Looking at: https://git.drupalcode.org/project/drupal/-/merge_requests/8860/diffs

    items:
      title: Tags Items
      type: array
      item:
        type: object
        properties:
          content:
            title: Tags Item
            type: object
          attributes:
            title: Item Attributes
            type: Drupal\Core\Template\Attribute

1. No need to have "Tags Items" as title, we know we are in a Tag component. "Items" is enough.

2. This is not JSON schema valid. You need items instead of item:

      type: array
      items:
        type: object

Source: https://json-schema.org/understanding-json-schema/reference/array

3. In your JSON object, content property is an object itself. Why? An object of what? Which properties? It is printed in the template, but a mapping is not printable in Twig (except when it is a Drupal renderable, but those are expected in slots). Would a simple string fit instead?

4. If you make content property a string, it would be the opportunity to adopt an existing Drupal data structure, like the menu structure, so it will be easier to plug it to Drupal API, by replacing content by title.

Thanks @pdureau !

Fixed feedbacks. Please review

Looks great.

label prop

I would recommend to make it a slot instead of a prop to be able to send any renderable into it. But I know slots are a bit underappreciated in SDC those days, and this is not a big deal anyway.

tags prop

    tags:
      title: Tags
      type: array
      items:
        type: object
        properties:
          content:
            title: Tag
            type: string

Only the first one is a blocker in my opinion.

1. ⚠️ Now, we have an array of objects, where the object has only one property: content. Where did attributes go? Forgotten I guess, because visible in the template: <li{{ tag.attributes.addClass('tags__item') }}>{{ tag.content }}</li>

2. As said previously, even without ✨ Add unified Links data type for menus, pagers, breadcrumbs... Active , you can make integration with Drupal API (menus, links, breadcrumbs..) easier by using text or title instead of content.

Getting tonns of errors like

[Tue Oct 22 20:50:22 2024] Uncaught PHP Exception Drupal\Core\Render\Component\Exception\InvalidComponentException: "[props.properties.tags.items.properties.attributes.type] Does not have a value in the enumeration ["array","boolean","integer","null","number","object","string"]/n[props.properties.tags.items.properties.attributes.type] String value found, but an array is required/n[props.properties.tags.items.properties.attributes.type] Failed to match at least one schema/n[props.properties.tags.items] Object value found, but an array is required/n[props.properties.tags.items] Failed to match at least one schema" at /Users/ivan/Sites/experiments/navi-core/core/lib/Drupal/Core/Theme/Component/ComponentValidator.php line 121
[Tue Oct 22 20:50:22 2024] [::1]:49327 Closing

when defining attributes in component. via object or that weird Drupal related type