-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Implement hooks for checkout #16636
Implement hooks for checkout #16636
Conversation
Bunnyshell Preview Environment deletedAvailable commands:
|
{% block title %}{{ 'sylius.ui.address'|trans }} | {{ parent() }}{% endblock %} | ||
{% block content_title %}{{ 'sylius.ui.address'|trans }}{% endblock %} | ||
{% set form = hookable_metadata.context.form %} | ||
{% set order = hookable_metadata.context.order %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you need the order
here?
{% set taxIncluded = order.getTaxIncludedTotal() %} | ||
{% set taxExcluded = order.getTaxExcludedTotal() %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
{% set taxIncluded = order.getTaxIncludedTotal() %} | |
{% set taxExcluded = order.getTaxExcludedTotal() %} | |
{% set tax_included = order.getTaxIncludedTotal() %} | |
{% set tax_excluded = order.getTaxExcludedTotal() %} |
{% import "@SyliusShop/shared/macro/money.html.twig" as money %} | ||
|
||
{% set order = hookable_metadata.context.order %} | ||
{% set itemsSubtotal = order.getItemsSubtotal() %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
{% set itemsSubtotal = order.getItemsSubtotal() %} | |
{% set items_subtotal = order.getItemsSubtotal() %} |
|
||
{% set item = hookable_metadata.context.item %} | ||
|
||
<td id="{{ item.variant.product.slug }}" {{ sylius_test_html_attribute('item-subtotal', item.variant.product.slug) }}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<td id="{{ item.variant.product.slug }}" {{ sylius_test_html_attribute('item-subtotal', item.variant.product.slug) }}> | |
<td {{ sylius_test_html_attribute('item-subtotal', item.variant.product.code) }}> |
@@ -0,0 +1,7 @@ | |||
{% set item = hookable_metadata.context.item %} | |||
|
|||
<td {{ sylius_test_html_attribute('item-subtotal-quantity', item.variant.product.name) }}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<td {{ sylius_test_html_attribute('item-subtotal-quantity', item.variant.product.name) }}> | |
<td {{ sylius_test_html_attribute('item-subtotal-quantity', item.variant.product.code) }}> |
Didn't check how it's used in behat, but I'm wondering if we shouldn't base the attribute on variant instead of the product
{% set order = hookable_metadata.context.order %} | ||
|
||
<table class="table table-borderless mb-3"> | ||
{% hook 'total_costs' with { order } %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
{% hook 'total_costs' with { order } %} | |
{% hook 'totals' with { order } %} |
Same with the file name, using costs
when we also have promotions inside seems weird.
Having it just as totals
is more ambiguous.
|
||
<tr> | ||
<td>{{ 'sylius.ui.items_total'|trans }}:</td> | ||
<td class="text-end">{{ money.convertAndFormat(itemsSubtotal) }}</td> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<td class="text-end">{{ money.convertAndFormat(itemsSubtotal) }}</td> | |
<td class="text-end">{{ money.convertAndFormat(order.itemsSubtotal) }}</td> |
To be more inline with the files below
6350f3c
to
2c0a938
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these checkout events still needed?
{% include '@SyliusShop/checkout/address/form/addresses/shipping_address_required.html.twig' %} | ||
{% else %} | ||
{% include '@SyliusShop/checkout/address/form/addresses/billing_address_required.html.twig' %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These includes should be converted into Twig Hooks, and ideally all address fields should be divided into separate templates so that this can be customized as easily as possible
2c0a938
to
fd60922
Compare
fd60922
to
089a90f
Compare
Thank you, @kulczy! |
No description provided.