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

[Product] added original price #3783

Merged
merged 7 commits into from
Dec 30, 2015
Merged

[Product] added original price #3783

merged 7 commits into from
Dec 30, 2015

Conversation

kwedrowicz
Copy link

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Fixed tickets
License MIT
Doc PR

Continuation of PR #3373


public function isReduced()
{
if ($this->originalPrice > $this->price) {
Copy link
Member

Choose a reason for hiding this comment

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

return $this->originalPrice > $this->price;?

Missing doblock.

<div class="product-box">
<a href="{{ path(product) }}"><h4>{{ product.name|truncate(18) }}</h4></a>
<a href="{{ path(product) }}">
<img class="img-rounded img-responsive" src="{{ product.image ? product.image.path|imagine_filter('sylius_medium') : 'http://placehold.it/262x255' }}" alt="{{ product.name }}" />
<span class="label label-primary">{{ sylius_calculate_price(product.masterVariant)|sylius_price }}</span>
<!--<span class="label label-primary">{{ sylius_calculate_price(product.masterVariant)|sylius_price }}</span>-->
Copy link
Member

Choose a reason for hiding this comment

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

Remove unused code.

@@ -32,3 +32,10 @@
</div>

{% endmacro %}

{% macro sylius_display_price_with_original(variant) %}

Choose a reason for hiding this comment

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

I would just call this sylius_display_price since the original is conditional

Copy link
Member

Choose a reason for hiding this comment

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

👍

@kwedrowicz
Copy link
Author

@pjedrzejewski @Zales0123 @lchrusciel @peteward I corrected everything you said. Anything more?

pjedrzejewski pushed a commit that referenced this pull request Dec 30, 2015
@pjedrzejewski pjedrzejewski merged commit 79b81d0 into Sylius:master Dec 30, 2015
@pjedrzejewski
Copy link
Member

Thank you Krzysztof! 👍 Nice work. :)

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.

None yet

5 participants