Skip to content
This repository has been archived by the owner on Aug 30, 2018. It is now read-only.

Update cart properties size check #318

Merged
merged 2 commits into from Jan 19, 2015
Merged

Update cart properties size check #318

merged 2 commits into from Jan 19, 2015

Conversation

cshold
Copy link
Contributor

@cshold cshold commented Jan 13, 2015

As per this Shopify forum post, {% if item.properties.size > 0 %} will never run so here it is removed. The for loop is more than capable of handling this on its own.

Fixes #317

cc @stevebosworth

@@ -989,7 +989,7 @@ hr {
}
}

.rte-header {
.rte--header {
margin-bottom: 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

This should probably be .rte__header since it's not really a modifier?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Didn't mean to include this here, but you're right. I'll update

@stevebosworth
Copy link
Contributor

Looks good 👍

@cshold
Copy link
Contributor Author

cshold commented Jan 13, 2015

Awaiting word whether the liquid will be fixed to make fix this unnecessary.

@cshold
Copy link
Contributor Author

cshold commented Jan 19, 2015

Rather than remove the check, I've now assigned a new variable using the | size filter rather than .size. As one of our devs mentioned, if size happened to be a line item property we'd run into issues, which is why using the filter makes a lot more sense.

@cshold cshold changed the title Removed failed cart properties liquid Update cart properties size check Jan 19, 2015
@stevebosworth
Copy link
Contributor

Cool, good to know about using the size property. 🚢

cshold added a commit that referenced this pull request Jan 19, 2015
Update cart properties size check
@cshold cshold merged commit 30c1048 into master Jan 19, 2015
@cshold cshold deleted the cart-logic branch January 19, 2015 16:02
rickydazla added a commit to rickydazla/Tricky3eme that referenced this pull request Jan 21, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

item.properties Bug
3 participants