Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[EWPP-1360] EWPP-1363: Style Sources field in full view mode. #903

Merged
merged 2 commits into from
Aug 5, 2021

Conversation

22Alexandra
Copy link
Contributor

EWPP-1363

Description

Style Sources field in full view mode.

Change log

  • Changed: Sources field in full view mode.

Comment on lines +9 to +36
<h2 class="ecl-u-type-heading-2">{{ 'Sources'|t }}</h2>
<div class="ecl-list">
{% for item in items %}
<article class="ecl-content-item ecl-u-d-s-flex ecl-u-pb-m">
<div class="ecl-u-flex-grow-1">
<div class="ecl-content-item__title ecl-u-type-heading-5 ecl-u-mb-xs ecl-u-mt-none">
{%- if item.content['#url'] %}
{% include '@ecl-twig/link' with {
link: {
type: 'standalone',
icon_position: 'after',
label: item.content['#title'],
path: item.content['#url']
},
icon: {
name: 'external',
size: 's',
path: ecl_icon_path
}
} %}
{% else %}
{{ item.content['#title'] }}
{%- endif -%}
</div>
</div>
</article>
{% endfor %}
</div>
Copy link
Member

Choose a reason for hiding this comment

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

Looks like it would be more suitable to reuse the list_item pattern like we did for related_links field.
See \oe_theme_preprocess_field function inside oe_theme.theme.

Copy link
Member

@sergepavle sergepavle Aug 3, 2021

Choose a reason for hiding this comment

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

Unfortunately, we can't, for now, use the list_item pattern according to fact that we can't control the link's icon. I think it would be the more preferable solution anyway to use a pattern but with additional parameter for controlling the link's type (for adding external icon if needed).
I suppose it can be impelmented in follow-up ticket.

tests/Functional/ContentNewsRenderTest.php Show resolved Hide resolved
sergepavle
sergepavle previously approved these changes Aug 3, 2021
Base automatically changed from EWPP-1362 to EPIC-EWPP-1360-News-v3 August 5, 2021 07:41
nagyad
nagyad previously approved these changes Aug 5, 2021
@22Alexandra 22Alexandra merged commit c008eab into EPIC-EWPP-1360-News-v3 Aug 5, 2021
@22Alexandra 22Alexandra deleted the EWPP-1363 branch August 5, 2021 13:48
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.

3 participants