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

SYL-4167: UI cleanup #457

Merged
merged 9 commits into from
Dec 6, 2024
Merged

SYL-4167: UI cleanup #457

merged 9 commits into from
Dec 6, 2024

Conversation

bartek-sek
Copy link
Contributor

credit_memo
credit_memos
refunds

@mpysiak mpysiak force-pushed the SYL-4167/ui-cleanup branch from d954407 to c79d94f Compare December 4, 2024 06:07
@mpysiak mpysiak force-pushed the SYL-4167/ui-cleanup branch from c79d94f to d6ccd67 Compare December 4, 2024 07:03
@GSadee GSadee added UX/UI Issues and PRs aimed at improving User eXperience and User Interface. DX Issues and PRs aimed at improving Developer eXperience. labels Dec 5, 2024

'sylius_admin.order.show.content.sections.credit_memos.credit_memos.item.actions':
'sylius_admin.order.show.content.sections.credit_memos.credit_memos.items.actions':
Copy link
Member

Choose a reason for hiding this comment

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

credit_memos part is duplicated

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This hook is used in partial that will be refactored in the next task, so I left it without changes.

<td class="text-center">{% include '@SyliusAdmin/shared/grid/field/channel.html.twig' with {'data': credit_memo.channel} %}</td>
<td class="text-end"><strong>{{ money.format(credit_memo.total, credit_memo.currencyCode) }}</strong></td>
<td class="w-1">
{% hook 'sylius_admin.order.show.content.sections.credit_memos.credit_memos.items.actions' %}
Copy link
Member

Choose a reason for hiding this comment

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

Why are we triggering this hook here? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the template that will be refactored in the next task, so I left it without changes

Copy link
Member

Choose a reason for hiding this comment

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

Ok, but this hook has to be changed in the next iteration 🖖🏻

<div class="ui labeled input">
<div class="ui label">{{ order.currencyCode|sylius_currency_symbol }}</div>
<div class="input-group">
<div class="input-group-text">{{ order.currencyCode|sylius_currency_symbol }}</div>
{% set inputName = "sylius_refund_shipments["~shipping_adjustment.id~"][amount]" %}
Copy link
Member

Choose a reason for hiding this comment

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

Use snake_case here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the template that will be refactored in the next task, so I left it without changes

{% set item = hookable_metadata.context.item %}

<td>
{{ '%0.2f'|format(item.netValue/100) }}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
{{ '%0.2f'|format(item.netValue/100) }}
{{ (item.netValue / 100)|number_format(2) }}

@GSadee GSadee merged commit cb70ba4 into Sylius:2.0 Dec 6, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DX Issues and PRs aimed at improving Developer eXperience. UX/UI Issues and PRs aimed at improving User eXperience and User Interface.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants