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

Removed duplicate search layout code #374

Merged
merged 3 commits into from
Mar 20, 2015
Merged

Removed duplicate search layout code #374

merged 3 commits into from
Mar 20, 2015

Conversation

cshold
Copy link
Contributor

@cshold cshold commented Mar 20, 2015

There was a decent chunk of duplicate code on the search template. This reduces that by moving the if search performed and if no results conditionals only affect the title of the page.

cc/ @mpiotrowicz @stevebosworth @carolineschnapp

@carolineschnapp
Copy link
Contributor

A {% if item.price %} is nested inside an {% if item.featured_image %} so the logic seems flawed. If you have a featured_image for sure you have a price.

@cshold
Copy link
Contributor Author

cshold commented Mar 20, 2015

i think if just looks like that because of the indentation. item.price is not inside item.featured_image.

@carolineschnapp
Copy link
Contributor

Oh right I am so sorry: http://take.ms/WYFoe Would you like me to test all this in a sandbox? I can do that if you give me one more hour. Thanks for using a better check for the presence of a product result. We have dropped so many requirements for a product to be a product :) No need for type / vendor / description...

@cshold
Copy link
Contributor Author

cshold commented Mar 20, 2015

Yea that'd be great. I'm working on another theme's search results now and mimicking what's going on here. There may be another commit or two so no rush.

@carolineschnapp
Copy link
Contributor

When showing the description or content, there's often an indentation:

Alt text

I think that issue may be with our Liquid filters, OR RTE. I ran into this issue before and can't remember for the life of me what I did to get rid of the space. You, I, will also see that when showing a stripped, truncated article content. No amount of | remove: ' ' or | trimgets rid of that white space at the beginning.

@cshold
Copy link
Contributor Author

cshold commented Mar 20, 2015

Must be an RTE thing. Not happening on my shop https://carson-dev-shop.myshopify.com/search?q=page

@carolineschnapp
Copy link
Contributor

Must be, yes. I will dig from its grave a Github issue I once created for this.

Moving on!

cshold added a commit that referenced this pull request Mar 20, 2015
Removed duplicate search layout code
@cshold cshold merged commit e58a598 into master Mar 20, 2015
@cshold cshold deleted the search-layout branch March 20, 2015 17:34
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.

2 participants