Add `#gallery` to `Variant` and `Product` #2337

swcraig · 2017-10-30T23:20:23Z

Paperclip is deprecated and we need to remove it from stock Solidus.

In order to remove Paperclip properly we need to add extension points so that we can plug a different image uploading library (like Active Storage). Adding galleries to Product and Variant will give us more control over what we send to the view layer, and protect our core models from image implementation details.

To plug this interface a developer needs to create custom galleries for both Variant and Product and make sure that the #images method of those return a collection of images that respond to the new "presentational" image interface the frontend expects. This PR includes changes to Spree::Image which transitions it into a presentation role for Paperclip. Specifically, the presentational image interface that the view layer expects is:

id: int
url(:size): string
filename: string
viewable_id: int # the ID of the variant this is "attached" to

Before Paperclip can be extracted into its own extension properly we need this PR to be merged.

Much of the work around the the Gallery classes are heavily influenced by/verbatim the work Clarke did in #625.

cbrunsdon

I personally think this is the most reasonable and sane way to start pulling paperclip out of core, as discussed elsewhere, so I'm 👍

jhawthorn · 2017-11-02T00:57:43Z

core/app/models/spree/variant.rb

+    # An Enumerable of media that goes along with a variant
+    # Stores may not necessarily want to attach images to variants
+    # This provides a more robust interface
+    # NOTE: In the future this will return an empty array


Why would this be useful? What is a developer supposed to do with this information.

I assume this is for extension points, since we want to avoid class_evaling, maybe we should look at introducing a Spree::Config.variant_gallery_class and a Spree::Config.product_gallery_class.

I like that idea of configurable image attachment classes. If we want extensions to be able to hook into we want to provide a real integration point.

Also I don’t think gallery is the correct term. gallery_images would follow the “plural is a collection” convention of Rails better. We could even override the images getter. Btw. We need to deprecate the images = setter as well or delegate this to the images provider class.

tvdeyen

I second John here and think we should introduce a real integration point for extensions.

tvdeyen · 2017-11-02T06:24:52Z

core/app/models/spree/variant.rb

+    # An Enumerable of media that goes along with a variant
+    # Stores may not necessarily want to attach images to variants
+    # This provides a more robust interface
+    # NOTE: In the future this will return an empty array


I like that idea of configurable image attachment classes. If we want extensions to be able to hook into we want to provide a real integration point.

Also I don’t think gallery is the correct term. gallery_images would follow the “plural is a collection” convention of Rails better. We could even override the images getter. Btw. We need to deprecate the images = setter as well or delegate this to the images provider class.

houndci-bot · 2017-11-02T18:08:13Z

core/app/models/spree/gallery/variant_gallery.rb

@@ -0,0 +1,18 @@
+module Spree
+  module Gallery
+


Extra empty line detected at module body beginning.

houndci-bot · 2017-11-02T18:08:13Z

core/app/models/spree/gallery/product_gallery.rb

+module Spree
+  module Gallery
+    class ProductGallery < Gallery::Base
+


Extra empty line detected at class body beginning.

houndci-bot · 2017-11-06T23:57:19Z

core/spec/models/spree/gallery/variant_gallery_spec.rb

+  describe '#primary_image' do
+  end
+end
+


1 trailing blank lines detected.

houndci-bot · 2017-11-06T23:57:19Z

core/spec/models/spree/gallery/variant_gallery_spec.rb

+  describe '#primary_image' do
+  end
+end
+


1 trailing blank lines detected.

jhawthorn · 2017-11-07T22:00:53Z

core/app/models/spree/gallery/variant_gallery.rb

+      private
+
+      def product_images
+        @variant.product.try(:variant_images) || []


I don't think this needs to be a try. Is @variant ever nil?

jhawthorn · 2017-11-07T22:04:28Z

core/app/models/spree/gallery/base.rb

@@ -0,0 +1,23 @@
+module Spree


I'm not sure there's a need for this base class. It should be sufficient to duck-type have the two gallery classes "quack" similarly 🦆 🦆 🦆 🦆 🦆

I guess the reason I would want the base class is so that it's obvious the interface that should be implemented for galleries in general. But I understand your point and can make the changes.
🦆 🦆

tvdeyen

I also don’t see a need for the base class. The two example implementations should be enough.

Everything else is 👌🏻

swcraig · 2017-11-15T18:52:52Z

This is ready for another review. There have been a few changes since the last reviews. I have added Variant#gallery.best_image. This method will fallback to using the first associated product image if a variant itself has no images.

When a variant has no images itself, why do we allow developers to choose to fallback to the first associated product image in some cases but not others? Could I remove the #best_image method and move this into a config value (Spree::Config.fallback_to_product_images)?

tvdeyen

I don’t think we should put all the image display logic into the gallery objects. They could defer from partial to partial and this screams for adding even more methods into that classes. Secondary_image? And what’s a best image anyways ;)

TL;DR let’s just remove them and be more explicit in the views. This might be not dry, but provides much more flexibility on the view layer, that is way easier to override and customize than a class. Also caching is much easier this way.

tvdeyen · 2017-11-15T19:57:59Z

core/app/models/spree/gallery/variant_gallery.rb

+      private
+
+      def product_images
+        @variant.product.variant_images


I don’t see a need for this private method as long as we don’t access it from two different methods at least.

tvdeyen · 2017-11-15T20:00:05Z

core/app/models/spree/gallery/variant_gallery.rb

+      # @return [Spree::Image] best image for the variant gallery
+      def best_image
+        @variant.images.first || product_images.first || Spree::Image.new
+      end


Why not remove these image collector methods from here and move into the views. This might be more duplication, but is also more flexible. Although I think the naming is difficult and misleading. Let’s not make these decisions here, but in the views. They are easier to override in a per use case basis.

houndci-bot · 2017-11-21T00:08:26Z

core/spec/helpers/images_helper_spec.rb

@@ -0,0 +1,21 @@
+require 'rails_helper'
+
+ RSpec.describe Spree::ImagesHelper, type: :helper do


Inconsistent indentation detected.

houndci-bot · 2017-11-21T18:07:31Z

core/app/models/spree/product.rb

@@ -267,6 +267,8 @@ def total_on_hand
    # variants. If all else fails, will return a new image object.
    # @return [Spree::Image] the image to display
    def display_image
+      Spree::Deprecation.warn('Spree::Product#display_image is DEPRECATED. ' \ 
+                              'Choose an image from Spree::Product#gallery instead.')


unexpected token tSTRING

houndci-bot · 2017-11-21T18:07:31Z

core/app/models/spree/product.rb

@@ -267,6 +267,8 @@ def total_on_hand
    # variants. If all else fails, will return a new image object.
    # @return [Spree::Image] the image to display
    def display_image
+      Spree::Deprecation.warn('Spree::Product#display_image is DEPRECATED. ' \ 


bare backslash only allowed before newline

swcraig · 2017-11-21T18:23:02Z

I think this is good to go. I've added ImagesHelper#image_or_default for the admin/frontend views. This will fallback to a Spree::Image if an image cannot be found for a product/variant.

Thomas and I discussed over this, and we think this is not the right abstraction (uh)

mamhoff

Hm, so now we have an abstraction inbetween Spree::Variant and the views, but the views still need to know Paperclip's API in order to display the images.

I would prefer if the galleries would return a simple list of paths or URLs that the views can use.

Something like this:

module Spree
  class Variant
    class Gallery
      def initialize(variant)
        @variant = variant
      end

      def image_urls(options)
        @variant.images.map { |image| image.attachment(options) }
      end
    end
  end
end

This would require us to change partials, too, but we would really lose the Paperclip API in the view layer, which without a change as radical as that we would not.

houndci-bot · 2017-11-28T18:56:53Z

core/spec/models/spree/gallery/variant_gallery_spec.rb

+  end
+
+  it_behaves_like 'a gallery'
+


Extra empty line detected at block body end.

swcraig · 2017-11-28T23:22:10Z

Thank you for the feedback.

The galleries have an #image_urls method in addition to an the #images method. The JBuilder API endpoints. the product thumbnails partial on the frontend, and images/_image_row in the backend expect Spree::Images (that respond to Paperclip API calls). I can't see an easy way to rewrite these with only URLs while keeping the same behaviour.

The actual display logic no longer needs to know Paperclip's API (there is no more accessing of the #attachment of an image). A sort of unfortunate result is that the views need to specify the image size twice (in case there is no image and we need to fallback to use the CSS of class= placeholder <size>. Like here for example.

Thanks to @derrickp for his input on this.

The variant and product galleries return an Enumerable of images (things that must respond to #url, #alt, #viewable_id, and #id**). The display logic no longer knows about Paperclip's API. The only place that still knows about the Paperclip API is the relevant JBuilder image partial in solidus_api.

Instead of having complex fallback rules, both the variant and product galleries can return empty Enumerables. Our image partials in the frontend/backend have historically dealt with nil "images" by applying the CSS of class= placeholder <size>. Instead of defining a hacky Paperclip image default, we should have stores use this CSS to define what to display in the case that there are no images.

** If we are fine with removing this modal in the admin we can get rid of #id requirement. This is also the only place we use that modal.

I'm interested in hearing what we think of where this is going.

mamhoff

This is now starting to look really good in my view. I have two minor nits regarding the URL used in views, which with the PR changes from :mini to whatever is the default.

mamhoff · 2017-12-01T10:18:31Z

frontend/app/views/spree/shared/_image.html.erb

  <% if itemprop %>
-    <%= image_tag image.attachment(size), itemprop: itemprop %>


This does not pass the :mini parameter to the image. Maybe it should.

mamhoff · 2017-12-01T12:58:30Z

backend/app/views/spree/admin/shared/_image.html.erb

-<% if image && image.attachment? %>
-  <%= image_tag image.attachment(size) %>
+<% if image %>
+  <%= image_tag image %>


This is also missing the :mini option.

This partial is given an image url (the caller has already decided the size and retrieved the relevant url). I've updated the naming to be more clear.

Does that make sense?

tvdeyen

We should try to decouple from how an extension implements urls as much as possible. Currently the objects interacting with the gallery need to know to much implementation details. Preferably the gallery classes would only return images urls. Extensions would then implement gallery classes.

tvdeyen · 2017-12-01T13:15:32Z

core/app/models/spree/gallery/product_gallery.rb

+      #
+      # @return [Enumerable<Spree::Image>] all images in the gallery
+      def images
+        @images ||= (@product.images + @product.variant_images).uniq


There is no need to merge these two collections, because @product.variant_images already includes images of the master variant - what are the images returned by @product.images

tvdeyen · 2017-12-01T13:22:58Z

backend/app/views/spree/admin/stock_items/_stock_management.html.erb

@@ -28,7 +28,9 @@
        <td class="no-padding" rowspan="<%= row_count %>">
          <div class='variant-container'>
            <div class='variant-image'>
-              <%= render 'spree/admin/shared/image', image: variant.display_image(fallback: false), size: :small %>
+              <%= render 'spree/admin/shared/image',
+                image: variant.gallery.images.first.try(:url, :small),


The thing you're passing into the partial as image is actually an url. Shouldn't we rename the variable as well?

tvdeyen · 2017-12-01T13:25:50Z

backend/app/views/spree/admin/orders/_shipment_manifest.html.erb

@@ -4,7 +4,9 @@
      data-variant-id="<%= item.variant.id %>"
      >
    <td class="item-image align-center">
-      <%= render 'spree/admin/shared/image', image: item.variant.display_image, size: :mini %>
+      <%= render 'spree/admin/shared/image',
+        image: (item.variant.gallery.images.first || item.variant.product.gallery.images.first).try(:url, :mini),


It makes sense to have a first_image_url method in the gallery classes.

item.variant.gallery.first_image_url(size: :mini) || item.variant.product.gallery.first_image_url(size: :mini)

core/app/models/spree/gallery/product_gallery.rb

core/app/models/spree/gallery/variant_gallery.rb

tvdeyen · 2017-12-01T13:32:59Z

core/app/models/spree/image.rb

+    end
+
+    def url(size)
+      attachment.url(size)


We are not able to decouple from paperclip if we have this method inside of here. It would be better to put this url retrieving logic inside the gallery classes. Extensions like solidus_paperclip would then implement such gallery classes.

frontend/app/views/spree/products/_thumbnails.html.erb

swcraig · 2017-12-04T21:15:39Z

frontend/app/views/spree/shared/_image.html.erb

  <% if itemprop %>
-    <%= image_tag image.attachment(size), itemprop: itemprop %>
+    <%= image_tag image_url, itemprop: itemprop %>


Something to note here. We don't use an image's alt attribute in either frontend or backend. Would being able to access image.alt here be an argument for using a custom image class like I suggest in this PR comment? That image class could have a strict interface #url(size), #id (for that single-use modal in backend/image/_image_row), #viewable_id, and #alt.

Is there a reason why we haven't historically been rendering an image's alt attribute? @mamhoff @tvdeyen

Would being able to access image.alt here be an argument for using a custom image class

Yes! A separate image class (Picture maybe?) makes total sense to abstract the presenter API 👍

Is there a reason why we haven't historically been rendering an image's alt attribute?

Not sure. We should render it (at least in the frontend).

houndci-bot · 2017-12-05T21:06:03Z

core/spec/models/spree/gallery/variant_gallery_spec.rb

+  end
+
+  it_behaves_like 'a gallery'
+end


unexpected token kEND

core/spec/models/spree/gallery/variant_gallery_spec.rb

houndci-bot · 2017-12-05T21:06:03Z

core/spec/models/spree/gallery/variant_gallery_spec.rb

+  end
+
+  it_behaves_like 'a gallery'
+end


unexpected token kEND

core/spec/models/spree/gallery/variant_gallery_spec.rb

houndci-bot · 2017-12-05T21:26:09Z

core/app/models/spree/picture.rb

+    end
+
+    # Used by the API endpoints that expect Paperclip attributes
+    def image


Use attr_reader to define trivial reader methods.

swcraig · 2017-12-12T01:26:15Z

I have this to the point where we don't reference Paperclip specific calls anywhere in the views. We do continue to use Paperclip specific logic in the solidus_api controller for images, and in the actual form for uploading images. We can probably deal with these when someone goes to pull Paperclip into an extension.

swcraig · 2017-12-13T20:29:01Z

This is ready for another review.

I plan on transitioning Spree::Image to function as a "presenter" class for images on the frontend. It's sort of weird right now because it represents a Paperclip image, but instead of getting rid of it and using a new class (Spree::Picture) if/when someone rips out Paperclip we could just keep it and define the interface there.

I am pretty happy with how this looks; the current state of the PR would make it easy for someone to pull out Paperclip, the only places that would need special attention are the places I mentioned in a previous comment.

Very happy to discuss this either here or by DM on Slack if any of this isn't clear.

swcraig · 2018-02-05T18:08:14Z

Could we possibly take another look at this? With Rails 5.2/ActiveStorage close to release I think there should be some consideration to get Paperclip out.

core/spec/models/spree/gallery/variant_gallery_spec.rb

core/spec/models/spree/gallery/product_gallery_spec.rb

core/lib/spree/testing_support/shared_examples/gallery.rb

core/app/models/spree/gallery/variant_gallery.rb

core/app/models/spree/gallery/product_gallery.rb

core/app/models/spree/gallery/variant_gallery.rb

core/app/models/spree/gallery/product_gallery.rb

Paperclip is deprecated, we need to remove it from being stock Solidus. This becomes a problem because we have very specific Paperclip method calls in our view layer, and generally are too tightly coupled to everything Paperclip related. Having a pluggable gallery implementation for products and variants gives us better control of how we send things to the view layer and can hide implementation details of specific image uploading libraries. If a developer wants to plug a different image library they need to write a custom gallery implementation that returns a collection of images that respond to the same interface as `Spree::Image`. See PR 2337 for more context.

Image uploading extensions can define their own galleries (that quack like these ones) and use them in `Spree::Product` and `Spree::Variant`.

The view layer in Solidus uses too many many Paperclip specific calls. In order to remove Paperclip from core (since it is no longer being maintained) we need to move those calls out of the view layer and into here. The idea being that a different image provider need only to give the view layer images that respond to the same interface defined here. The image galleries (like Spree::Gallery::ProductGallery) should respond to `#images` with a collection of objects that respond to `#id`, `#filename`, `#url(size)`, and `#viewable_id`. Instead of removing Spree::Image and creating a new class to represent this image class for the view layer, we can just transition Spree::Image into that role.

This isn't being used anywhere in the project.

`Product` and `Variant` should not know the implementation details of their images. When they have that knowledge we end up coupling the image uploading library (like Paperclip) way too tightly to these core models. Developers are able to write their own custom gallery classes that respond to a single `#images` method that returns a collection of images that implement the interface defined in `Spree::Image`.

Doing this allows a developer to plug in different galleries for variants and products and not need to modify existing core models to change image providers. This change is motivated by the need to remove the deprecated Paperclip project from Solidus. See PR 2337 for more context.

Instead of calling a Paperclip specific method (`#attachment`) we instead want to use the more generic interface that we have now defined in `Spree::Image`. This way as long as our variant and product galleries return images that respond to a common interface we can swap out our image uploading libraries as we wish. The behaviour of this partial remains the same.

Instead of calling a Paperclip specific method (`#attachment`) we instead want to use the more generic interface that we have now defined in `Spree::Image`. This way as long as our variant and product galleries return images that respond to a common interface we can swap out our image uploading libraries as we wish.

Doing this allows a developer to plug in different galleries for variants and products and not need to modify existing core models to change image providers. This change is motivated by the need to remove the deprecated Paperclip project from Solidus. See PR 2337 for more context.

Products and variants should not need to worry about providing images to the view layer. They should instead push that responsibility to their galleries. Images should be picked from `#gallery.images` collection.

swcraig · 2018-06-18T22:41:46Z

With Paperclip officially being deprecated can we take a serious look at merging this? When this is merged it will make extracting Paperclip into an extension much easier. I also think this is a better general approach to dealing with images in core/the view layer.

@tvdeyen feedback would be appreciated.

tvdeyen

This is a good abstraction that's waiting too long now. Sorry for keeping you waiting for this.

Things I would like to see in future PRs:

Remove the has_many :images relation from Spree::Product and Spree::Variant
Remove Spree::Image
Implement a presenter class for #gallery.images (Spree::GalleryPicture | Spree::GalleryImage)

tvdeyen · 2018-10-12T22:37:59Z

🎉 🎁

This was referenced Oct 30, 2017

Add #gallery to Spree::Variant and Spree::Product #2323

Closed

[RFC] Move Paperclip into an extension #2324

Closed

cbrunsdon approved these changes Oct 31, 2017

View reviewed changes

jhawthorn reviewed Nov 2, 2017

View reviewed changes

tvdeyen requested changes Nov 2, 2017

View reviewed changes

houndci-bot reviewed Nov 2, 2017

View reviewed changes

swcraig changed the title ~~Add #gallery to Variant and Product~~ [WIPAdd #gallery to Variant and Product] Nov 2, 2017

swcraig changed the title ~~[WIPAdd #gallery to Variant and Product]~~ [WIP] Add #gallery to Variant and Product Nov 2, 2017

swcraig changed the title ~~[WIP] Add #gallery to Variant and Product~~ Add #gallery to Variant and Product Nov 6, 2017

houndci-bot reviewed Nov 6, 2017

View reviewed changes

core/spec/models/spree/gallery/variant_gallery_spec.rb Outdated

describe '#primary_image' do

end

end

Copy link

houndci-bot Nov 6, 2017

Choose a reason for hiding this comment

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

1 trailing blank lines detected.

houndci-bot reviewed Nov 6, 2017

View reviewed changes

core/spec/models/spree/gallery/variant_gallery_spec.rb Outdated

describe '#primary_image' do

end

end

Copy link

houndci-bot Nov 6, 2017

Choose a reason for hiding this comment

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

1 trailing blank lines detected.

jhawthorn reviewed Nov 7, 2017

View reviewed changes

tvdeyen reviewed Nov 8, 2017

View reviewed changes

mamhoff previously approved these changes Nov 8, 2017

View reviewed changes

tvdeyen reviewed Nov 15, 2017

View reviewed changes

houndci-bot reviewed Nov 21, 2017

View reviewed changes

swcraig changed the title ~~Add #gallery to Variant and Product~~ [WIP] Add #gallery to Variant and Product Nov 21, 2017

swcraig changed the title ~~[WIP] Add #gallery to Variant and Product~~ Add #gallery to Variant and Product Nov 21, 2017

houndci-bot reviewed Nov 21, 2017

View reviewed changes

mamhoff suggested changes Nov 28, 2017

View reviewed changes

houndci-bot reviewed Nov 28, 2017

View reviewed changes

mamhoff reviewed Dec 1, 2017

View reviewed changes

tvdeyen requested changes Dec 1, 2017

View reviewed changes

swcraig commented Dec 1, 2017

View reviewed changes

frontend/app/views/spree/products/_thumbnails.html.erb Show resolved Hide resolved

swcraig commented Dec 4, 2017

View reviewed changes

houndci-bot reviewed Dec 5, 2017

View reviewed changes

benjaminwil mentioned this pull request Dec 12, 2017

Products and variants documentation #2437

Merged

houndci-bot reviewed Jun 6, 2018

View reviewed changes

houndci-bot reviewed Jun 18, 2018

View reviewed changes

core/app/models/spree/gallery/variant_gallery.rb Show resolved Hide resolved

core/app/models/spree/gallery/product_gallery.rb Show resolved Hide resolved

swcraig and others added 12 commits June 18, 2018 15:32

Add integration points for image galleries

942f849

Image uploading extensions can define their own galleries (that quack like these ones) and use them in `Spree::Product` and `Spree::Variant`.

Deprecate Spree::Image#mini_url

72f6220

This isn't being used anywhere in the project.

Deprecate #display_image methods

6cd8d58

Products and variants should not need to worry about providing images to the view layer. They should instead push that responsibility to their galleries. Images should be picked from `#gallery.images` collection.

This was referenced Jun 18, 2018

Paperclip closing, time to move to ActiveStorage? #2725

Closed

Move Paperclip into an extension #2328

Closed

kennyadsl approved these changes Jun 27, 2018

View reviewed changes

tvdeyen approved these changes Oct 12, 2018

View reviewed changes

tvdeyen merged commit 5662eb4 into solidusio:master Oct 12, 2018

swcraig deleted the add-gallery-to-variant-and-product branch October 12, 2018 22:44

elia mentioned this pull request Nov 23, 2018

ActiveStorage support #2974

Closed

7 tasks

		@@ -0,0 +1,21 @@
		require 'rails_helper'

		RSpec.describe Spree::ImagesHelper, type: :helper do

		<% if itemprop %>
		<%= image_tag image.attachment(size), itemprop: itemprop %>

Add #gallery to Variant and Product #2337

Add #gallery to Variant and Product #2337

Conversation

swcraig commented Oct 30, 2017 • edited Loading

cbrunsdon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tvdeyen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jhawthorn Nov 7, 2017 • edited Loading

Choose a reason for hiding this comment

swcraig Nov 7, 2017 • edited Loading

Choose a reason for hiding this comment

tvdeyen left a comment

Choose a reason for hiding this comment

swcraig commented Nov 15, 2017 • edited Loading

tvdeyen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

swcraig commented Nov 21, 2017 • edited Loading

mamhoff left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

swcraig commented Nov 28, 2017 • edited Loading

mamhoff left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

swcraig Dec 1, 2017 • edited Loading

Choose a reason for hiding this comment

tvdeyen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

swcraig Dec 4, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

swcraig commented Dec 12, 2017 • edited Loading

swcraig commented Dec 13, 2017 • edited Loading

swcraig commented Feb 5, 2018

swcraig commented Jun 18, 2018

tvdeyen left a comment

Choose a reason for hiding this comment

tvdeyen commented Oct 12, 2018

Add `#gallery` to `Variant` and `Product` #2337

Add `#gallery` to `Variant` and `Product` #2337

swcraig commented Oct 30, 2017 •

edited

Loading

jhawthorn Nov 7, 2017 •

edited

Loading

swcraig Nov 7, 2017 •

edited

Loading

swcraig commented Nov 15, 2017 •

edited

Loading

swcraig commented Nov 21, 2017 •

edited

Loading

swcraig commented Nov 28, 2017 •

edited

Loading

swcraig Dec 1, 2017 •

edited

Loading

swcraig Dec 4, 2017 •

edited

Loading

swcraig commented Dec 12, 2017 •

edited

Loading

swcraig commented Dec 13, 2017 •

edited

Loading