Skip to content

OEL-4322: Migrate Alert and Description list components.#528

Open
piotrsmykaj wants to merge 1 commit intoepic/OEL-3864from
OEL-4322
Open

OEL-4322: Migrate Alert and Description list components.#528
piotrsmykaj wants to merge 1 commit intoepic/OEL-3864from
OEL-4322

Conversation

@piotrsmykaj
Copy link
Contributor

No description provided.

@piotrsmykaj piotrsmykaj changed the base branch from 1.x to epic/OEL-3864 January 6, 2026 09:11
@piotrsmykaj piotrsmykaj force-pushed the OEL-4322 branch 2 times, most recently from f941bc5 to c5df227 Compare January 20, 2026 16:47
@piotrsmykaj piotrsmykaj changed the base branch from epic/OEL-3864 to 1.x January 20, 2026 16:51
@piotrsmykaj piotrsmykaj changed the base branch from 1.x to epic/OEL-3864 January 20, 2026 16:51
{% endfor %}

{% set _items = _items|merge([item|merge({'term': _terms})]) %}
{% endfor %}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be simplified by using |map(..)?
(on two levels)
The AI gives me this:

{% set _items = items|map(item =>
  item|merge({
    terms: (item.term is iterable ? item.term : [item.term])|map(term => {
      _term = term is iterable ? term : {'label': term}
    } => _term.icon is defined and _term.icon
      ? _term|merge({
          icon: {
            size: 'xs',
            path: _icon_path
          }|merge(_term.icon is iterable ? _term.icon : { name: _term.icon })
        })
      : _term
    )
  })
) %}

I am not saying this will work as-is, just as a starting point to explore.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could also create twig functions or filters if that helps. Either generic helpers, or more specific functions aimed at specific templates.
But not sure it would be worthwhile in this case.

}
}
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see we are removing the preprocess, but the pattern itself is still there.

{# 'items': items,#}
{# 'attributes': attributes,#}
{# })#}
{#}}#}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this necessary?
As a result, no menu will be shown?

We should find a different solution where we don't have to edit individual templates like this.

}

$data['#fields']['image'] = ImageValueObject::fromArray($data['#fields']['image']);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usually I like this early return pattern.
In this case I am not fully on board, because:
The method name massageCardPatterns() suggests that any alteration of the data for this pattern would happen in this method. For now it is only about items, but what if we want to also alter something else? Going by the method name, it would go into the same method.

  private static function massageCardPattern(array $data): array {
    if (isset($data['#fields']['image'])) {
      $data['#fields']['image'] = ImageValueObject::fromArray($data['#fields']['image']);
    }
    if (isset($data['label'])) {
      $data['label'] = translate($data['label']);
    }
    return $data;
  }

It's not a hard blocker, I am just sharing my thoughts.

if (!isset($data['#fields']['items'])) {
return $data;
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here the same arguments apply to the "early return", but I kind of like it because we don't have to touch the rest of the method for indentation :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants