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

'No images found' if each variant has it's own #2228

Closed
geermc4 opened this Issue Nov 20, 2012 · 11 comments

Comments

Projects
None yet
2 participants
@geermc4

geermc4 commented Nov 20, 2012

If each variant has it's own image and no image is set to 'All', the Images section of the product says there aren't any, if you add one with 'All' then they show up. They always show up on the customer side.

Tested this on a new project with just the default Gemfile and yes on all questions by the installer.

So it's spree 1.3, rails 3.2.9

@geermc4

This comment has been minimized.

Show comment
Hide comment
@geermc4

geermc4 Nov 20, 2012

Guessing https://github.com/spree/spree/blob/master/core/app/views/spree/admin/images/index.html.erb#L11 is the reason why.

Any particular reason why variant images shouldn't show up with the product?

geermc4 commented Nov 20, 2012

Guessing https://github.com/spree/spree/blob/master/core/app/views/spree/admin/images/index.html.erb#L11 is the reason why.

Any particular reason why variant images shouldn't show up with the product?

@geermc4

This comment has been minimized.

Show comment
Hide comment
@geermc4

geermc4 Nov 21, 2012

Also, if there is no image set to All, the product listing doesn't show any image, maybe we could use the first available on the variants on these cases?

geermc4 commented Nov 21, 2012

Also, if there is no image set to All, the product listing doesn't show any image, maybe we could use the first available on the variants on these cases?

@geermc4

This comment has been minimized.

Show comment
Hide comment
@geermc4

geermc4 Jan 22, 2013

This seems to work, should i just submit a patch with that line changed?

Deface::Override.new(:virtual_path => "spree/admin/images/index",
                 :name         => "fix_product_images_in_admin",
                 :replace      => "code[erb-silent]:contains('unless @product.images.any?')",
                 :text         => "<% unless @product.images.any? or @product.variants.collect(&:images).any? %>")

geermc4 commented Jan 22, 2013

This seems to work, should i just submit a patch with that line changed?

Deface::Override.new(:virtual_path => "spree/admin/images/index",
                 :name         => "fix_product_images_in_admin",
                 :replace      => "code[erb-silent]:contains('unless @product.images.any?')",
                 :text         => "<% unless @product.images.any? or @product.variants.collect(&:images).any? %>")
@geermc4

This comment has been minimized.

Show comment
Hide comment
@geermc4

geermc4 Jan 22, 2013

Actually that would just fix that section, on the list of products it still shows the default 'no image available'

geermc4 commented Jan 22, 2013

Actually that would just fix that section, on the list of products it still shows the default 'no image available'

@radar

This comment has been minimized.

Show comment
Hide comment
@radar

radar Jan 29, 2013

Member

I've just been able to verify this issue today. Looking into it.

Member

radar commented Jan 29, 2013

I've just been able to verify this issue today. Looking into it.

@radar

This comment has been minimized.

Show comment
Hide comment
@radar

radar Jan 29, 2013

Member

I've got a fix for this on my local master and 1-3-stable branches. I will need to get back to my "office" today to run the tests, and then once I validate that nothing's broken there, I'll push it up to spree/spree for you to use :)

Member

radar commented Jan 29, 2013

I've got a fix for this on my local master and 1-3-stable branches. I will need to get back to my "office" today to run the tests, and then once I validate that nothing's broken there, I'll push it up to spree/spree for you to use :)

@geermc4

This comment has been minimized.

Show comment
Hide comment
@geermc4

geermc4 Jan 30, 2013

Ha, cool, and I was just about to post another override to fix the part i was missing from my previous post, though yours looks more complete than my Deface hacks so i'll use yours, thanks @radar 👍

geermc4 commented Jan 30, 2013

Ha, cool, and I was just about to post another override to fix the part i was missing from my previous post, though yours looks more complete than my Deface hacks so i'll use yours, thanks @radar 👍

@radar radar closed this in fb3960a Feb 4, 2013

@geermc4

This comment has been minimized.

Show comment
Hide comment
@geermc4

geermc4 Feb 25, 2013

This is still a issue on the frontend.

index
Screen Shot 2013-02-25 at 11 48 50 AM

show
Screen Shot 2013-02-25 at 11 49 08 AM

Is this too hacky for a PR?

diff --git a/frontend/app/views/spree/shared/_products.html.erb b/frontend/app/views/spree/shared/_products.html.erb
index 5c5831f..653c229 100644
--- a/frontend/app/views/spree/shared/_products.html.erb
+++ b/frontend/app/views/spree/shared/_products.html.erb
@@ -14,7 +14,14 @@
     <% if product.on_display? %>
       <li id="product_<%= product.id %>" class="columns three <%= cycle("alpha", "secondary", "", "omega secondary", :name => "classes") %>" data-hook="products_list_item" itemscope itemtype="http://schema.org/P
         <div class="product-image">
-          <%= link_to small_image(product, :itemprop => "image"), product, :itemprop => 'url' %>
+          <%
+            if product.images.blank? then
+              p = product.variants.joins(:images).first
+            else
+              p = product
+            end
+          %>
+          <%= link_to small_image(p, :itemprop => "image"), product, :itemprop => 'url' %>

geermc4 commented Feb 25, 2013

This is still a issue on the frontend.

index
Screen Shot 2013-02-25 at 11 48 50 AM

show
Screen Shot 2013-02-25 at 11 49 08 AM

Is this too hacky for a PR?

diff --git a/frontend/app/views/spree/shared/_products.html.erb b/frontend/app/views/spree/shared/_products.html.erb
index 5c5831f..653c229 100644
--- a/frontend/app/views/spree/shared/_products.html.erb
+++ b/frontend/app/views/spree/shared/_products.html.erb
@@ -14,7 +14,14 @@
     <% if product.on_display? %>
       <li id="product_<%= product.id %>" class="columns three <%= cycle("alpha", "secondary", "", "omega secondary", :name => "classes") %>" data-hook="products_list_item" itemscope itemtype="http://schema.org/P
         <div class="product-image">
-          <%= link_to small_image(product, :itemprop => "image"), product, :itemprop => 'url' %>
+          <%
+            if product.images.blank? then
+              p = product.variants.joins(:images).first
+            else
+              p = product
+            end
+          %>
+          <%= link_to small_image(p, :itemprop => "image"), product, :itemprop => 'url' %>
@geermc4

This comment has been minimized.

Show comment
Hide comment
@geermc4

geermc4 Feb 26, 2013

Also, /cart doesn't show the image either.

geermc4 commented Feb 26, 2013

Also, /cart doesn't show the image either.

@geermc4

This comment has been minimized.

Show comment
Hide comment
@geermc4

geermc4 Feb 26, 2013

Nevermind the /cart part, I found why, but the products one is a issue

geermc4 commented Feb 26, 2013

Nevermind the /cart part, I found why, but the products one is a issue

@radar

This comment has been minimized.

Show comment
Hide comment
@radar

radar Feb 26, 2013

Member

Yes, that is too hacky for a PR. There's got to be a better way to do that.

Please open a new PR when you figure it out.

Member

radar commented Feb 26, 2013

Yes, that is too hacky for a PR. There's got to be a better way to do that.

Please open a new PR when you figure it out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment