Show carousel of images when multiple images are available by chahmedejaz · Pull Request #14337 · openfoodfoundation/openfoodnetwork

chahmedejaz · 2026-05-27T06:57:04Z

⚠️ Please use clockify #82 - Multiple images when working on this ⚠️

What? Why?

Closes Show carousel of images when multiple images are available #13981

This PR introduces support for displaying multiple product images through a reusable carousel component.

As part of the broader multiple-images initiative, this change implements the carousel experience in:

Shop product details modal (shopper view)
Admin product preview

The carousel is only displayed when a product has multiple images. Products with a single image continue to show the existing single-image experience.

To support this functionality, this PR:

Adds support for accessing multiple images on products.
Updates the admin product images page to list and manage all product images.
Introduces reusable carousel and thumbnail carousel components with:
- Previous/next navigation
- Thumbnail navigation
- Accessibility support
- Optional image captions
Adds carousel support to the admin product preview.
Adds carousel support to the frontshop product modal.
Exposes product images through the product API serializer.
Provides fallback behaviour for products without images.
Adds component, helper, serializer and frontend test coverage.

This work allows us to build and validate the carousel experience ahead of the future backoffice work for managing multiple images.

What should we test?

Single image products

Open a product with a single image in the shop.
Open the product details modal.
Verify only the product image is shown.
Verify no carousel navigation or thumbnails are displayed.

Multiple image products (shop)

Open a product with multiple images.
Open the product details modal.
Verify a carousel is displayed.
Verify all product images are available.
Verify previous/next navigation works.
Verify thumbnail navigation works.
Verify images are displayed in upload order (oldest to newest).

Multiple image products (admin)

Open Admin → Product Preview for a product with multiple images.
Verify a carousel is displayed.
Verify all product images are available.
Verify previous/next navigation works.
Verify thumbnail navigation works.
Verify images are displayed in upload order (oldest to newest).

Image management

Open Admin → Product Images.
Verify all product images are listed.
Verify image edit functionality still works.
Verify image deletion still works.

Regression testing

Verify products without images continue to display the placeholder image.
Verify existing single-image product workflows continue to function as before.

Release notes

Changelog Category (reviewers may add a label for the release notes):

User facing changes
API changes (V0, V1, DFC or Webhook)
Technical changes only
Feature toggled

Dependencies

Need the following PR to be merged along with this one:

Prevent admins to be able to view carousel in shops #14364

Documentation updates

None.

…ail navigation and use it in admin product preview

…rontshop

chahmedejaz · 2026-05-27T07:13:22Z

Hey @mariocarabotta - The carousel for multiple images on the product has been deployed to FR Staging. Please review.
I created the data for this product:
https://staging.coopcircuits.fr/admin/products/71817/images

But of course, you may create your own and play around. Thanks.

P.S. Should we introduce a feature flag for this feature, or deploy it all at once?

mariocarabotta · 2026-05-27T11:30:52Z

nice work!! some visual changes below (sorry pretty rough notes, tweaked in the browser).
I need to think about how the images behave when they don't fit the container. I'll have a look at the swiper options, things like resize to fill or similar. also I might make some further visual tweaks later, but hopefully I can push them directly. i've got a local environment set up now.

I will come back to you re feature flag, just checking with Rachel.

active image container (.ofn-thumbnail-carousel__image)
background-color: #fff;
border-radius: 8px;

arrow circle
background: rgb(255 255 255);
height: 1.5rem;
width: 1.5rem;

arrows
color: #444;
font-weight: 400;
left arrow only padding: 0 2px 2px 1px;
right arrow only padding: 0 0px 2px 1px;

caption
background: rgb(255 255 255);
REMOVE right: 0;
margin-left: 16px;
border-radius: 8px 8px 0px 0px;
font-family: Roboto, Arial, sans-serif;
font-weight: 600;
font-size: 0.9rem;
padding: 8px 12px 4px 12px;
color: #444;

ofn-thumbnail-carousel__thumbs ng-scope
REMOVE justify-content: center;

RachL · 2026-05-27T12:04:15Z

@mariocarabotta @chahmedejaz I don't see an advantage to use a feature flag here. However then let's move this PR through testing with testers as well, so we check if there aren't any edge case.

We probably need to update the acceptance criteria:

there is an item to confirm: And the order is based on the upload time (TBC)
we need to test the scenario of a product having multiple images before the feature is added and the case where new features are added after. Or does it not change anything?

Is there a max. of pictures we can handle?

mariocarabotta · 2026-05-28T05:25:11Z

@mariocarabotta @chahmedejaz I don't see an advantage to use a feature flag here. However then let's move this PR through testing with testers as well, so we check if there aren't any edge case.

this issue will need to be released together with this one, so that people don't see any carousel in production, but we are still able to see and test it in staging. we'll also need to give a heads up to the testers so that they have some test accounts in staging with multiple images. once we hide the button to add, they won't be able to include additional images to a product.

We probably need to update the acceptance criteria:

there is an item to confirm: And the order is based on the upload time (TBC)

updated to reflect how it currently behaves (from the oldest upload to the most recent). I think it's ok like this but please let me know if you think otherwise.

we need to test the scenario of a product having multiple images before the feature is added and the case where new features are added after. Or does it not change anything?

mmm it's a bit tricky because what we are going to be testing now is a product with multiple product images, but what we are going to enable for users in production eventually is a product with one product image and one image per variant. In my mind this issue is really building the carousel frontend and testing that it can show multiple images, as a UI element.
Maybe the issue where we enable to upload variant images might be the best place to test the scenario you describe?

Is there a max. of pictures we can handle?

I have been back and forth on specifying this. The max of images would be the same as the max of variants that a product can have. is there a limit to how many variants we can have per product?

In terms of how it renders a considerable number of images in the carousel, we'll need to fix it. Here's an example. I would prefer if images would just wrap, but I am also aware that some producers have 20 variants per product. But maybe they would not upload a photo for each one. I am leaning towards wrapping rather than a side scrolling which is not particularly accessible. Thoughts?

RachL · 2026-05-28T10:37:13Z

thanks Mario.

this issue will need to be released together withhttps://github.com//issues/14157

oh I didn't know about this dependency. It sounds to me more complex then than introducing a feature toggle now and releasing carousel only when the multiple image upload is available. Let's see what @chahmedejaz thinks.

If we introduce a feature toggle now; we can leave the carousel hidden and we avoid the work on #14157

In my mind this issue is really building the carousel frontend and testing that it can show multiple images, as a UI element.

in that case, and again if we have a feature toggle, the easiest here is to merge without testing manually (the feature is hidden, therefore no problem). And wait for a complete test when the image upload is ready.

Aligned on wrapping (but i don't think your example link is working?)

chahmedejaz · 2026-05-28T12:11:52Z

Thanks for the feedback, @mariocarabotta - I’ll address this and follow up.

@RachL, @mariocarabotta For this case, I think we have two options:

Introduce a feature flag for this behavior, or
Include the work from the other related issue in this PR so we can release the full carousel-related functionality, including prevention, together.

I’m leaning toward the second option since the additional work for the related issue seems relatively small, and it would allow us to ship the carousel changes in a more complete state.

mariocarabotta · 2026-05-29T11:18:49Z

given @chahmedejaz comment, how about we go for his suggested option 2?
I will prepare some more test products in staging australia and will continue to test the UI next week.
We can then let testers do proper end to end testing when this one is ready for them.

RachL · 2026-05-29T12:17:12Z

Let's go for option 2 then! Let's just keep in mind that sometimes the follow-up PR takes more time, and maintaining the previous one can be a pain.
I don't think this will be the case here, so happy to not use the feature toggle. Just mentioning this scenario for future cases: feature toggle allow us to divide the work in several PR. easy to review and merged incrementally.

…images

chahmedejaz · 2026-06-03T18:39:23Z

Adding a note here as well:

As discussed in the delivery circle yesterday, your design feedback is addressed @mariocarabotta, and it's ready for review.

chahmedejaz · 2026-06-11T22:10:50Z

Thanks for catching this, @mariocarabotta! I missed it as well since I didn't have any products with long descriptions. 😅
The issue should now be fixed and has been deployed to AU staging. It's ready for another review.
If you still notice the problem, could you share a short video or screen recording? That would help me investigate further.
Here's the screen recording from my mobile testing:

Carousel.demo.mp4

mariocarabotta · 2026-06-15T00:17:58Z

looking great @chahmedejaz, now it's fixed!
I don't need to do any further testing, it's looking very good from my side. I would still let testers do some further review though.

amazing work and thank you for your patience

rioug · 2026-06-16T23:44:30Z

+  position: absolute;
+  top: 50%;
+  transform: translateY(-50%);
+  z-index: 2;


I think it's would be good to use a variables here, and defined it in a central place with other z-index value like what we started doing for the admin CSS :

openfoodnetwork/app/webpacker/css/admin/globals/variables.scss

Lines 185 to 192 in 11e3b90

// z-index

//--------------------------------------------------------------

$tos-banner-z-index: 1001;

$flash-message-z-index: 1000;

$tag-drop-down-z-index: 999;

$ts-dropdown-z-index: 101;

$save-bar-z-index: 100;

rioug · 2026-06-16T23:49:56Z

It looks like you copied the CSS from the frontend, ideally we would share the same CSS so any changes to the frontend flows to the product preview. Could you have a look at sharing the CSS ?

rioug · 2026-06-16T23:51:07Z


 @import 'modal_component/modal_component';
+@import 'carousel_component/carousel_component';
+@import 'thumbnail_carousel_component/thumbnail_carousel_component';


I don't see any CSS for thumbnail_carousel_component' is this needed ?

The CSS file exists at app/components/thumbnail_carousel_component/thumbnail_carousel_component.scss and is imported in both darkswarm/all.scss and admin/all.scss.

rioug · 2026-06-16T23:52:08Z

@@ -0,0 +1,30 @@
+# frozen_string_literal: true
+
+RSpec.describe CarouselComponent, type: :component do


rioug · 2026-06-16T23:58:26Z

  $scope.productPropertySelectors = FilterSelectorsService.createSelectors()

+  $scope.currentProductImageIndex = 0
+  $scope._productCarouselImages = null


Why the underscore in the name here ?

The leading underscore is intentional. It's being used to indicate that this is an internal/private variable rather than a public one. The naming is purely for annotation and readability purposes.

rioug · 2026-06-17T00:19:36Z

+  margin-left: 16px;
+  border-radius: 8px 8px 0px 0px;
+  font-family: Roboto, Arial, sans-serif;
+  font-weight: 600;
+  font-size: 0.9rem;
+  padding: 8px 12px 4px 12px;
+  color: #444;


I think in general it would be good to use SCSS variables when applicable,could you could review the final CSS and add variables ?

rioug · 2026-06-17T00:32:04Z

+    .ofn-thumbnail-carousel
+      .swiper-wrapper{"ng-style" => "{'transform': 'translateX(-' + (currentProductImageIndex * 100) + '%)', 'transition': 'transform 220ms cubic-bezier(0.22, 0.61, 0.36, 1)'}"}
+        .swiper-slide{"ng-repeat" => "image in productCarouselImages() track by $index"}
+          %img.ofn-thumbnail-carousel__image{"ng-src" => "{{image.url}}", "ng-attr-alt" => "{{image.alt}}"}
+          %p.ofn-thumbnail-carousel__caption{"ng-if" => "image.caption", "ng-bind" => "image.caption"}
+
+        .swiper-slide{"ng-if" => "productCarouselImages().length === 0"}
+          %img.ofn-thumbnail-carousel__image.placeholder{ src: Spree::Image.default_image_url(:large), "alt" => "Product image placeholder" }
+
+      %button.ofn-thumbnail-carousel__button.ofn-thumbnail-carousel__button--prev{
+        type: "button",
+        "ng-if" => "productCarouselImages().length > 1",
+        "ng-click" => "previousProductImage()",
+        "aria-label" => "Previous slide"
+      }
+        %span.ofn-thumbnail-carousel__button-icon{ "aria-hidden" => "true" } &#10094;
+
+      %button.ofn-thumbnail-carousel__button.ofn-thumbnail-carousel__button--next{
+        type: "button",
+        "ng-if" => "productCarouselImages().length > 1",
+        "ng-click" => "nextProductImage()",
+        "aria-label" => "Next slide"
+      }
+        %span.ofn-thumbnail-carousel__button-icon{ "aria-hidden" => "true" } &#10095;
+
+      .ofn-thumbnail-carousel__thumbs{"ng-if" => "productCarouselImages().length > 1"}
+        %button.ofn-thumbnail-carousel__thumb{
+          type: "button",
+          "ng-repeat" => "image in productCarouselImages() track by $index",
+          "ng-click" => "selectProductImage($index)",
+          "ng-class" => "{'is-active': $index === currentProductImageIndex}",
+          "ng-attr-aria-label" => "Go to slide {{$index + 1}}",
+          "ng-attr-aria-pressed" => "{{$index === currentProductImageIndex}}"
+        }
+          %img.ofn-thumbnail-carousel__thumb-image{"ng-src" => "{{image.thumb_url || image.url}}", "ng-attr-alt" => "{{image.alt}}"}


Looking at this and some of the other later commit, it looks like you had to re implement the Carousel functionality in AngularJS. I'd prefer if we could void that, because now we would have two places to look at any time we need to make changes to the carousel. And from what I can see you had to re implement some of the swipping functionality we get out of the box the Stimulus carousel.
I would much rather we rewrite the modal using StimulusJS and use the same carousel everywhere. I am not sure that's easily feasible, but I think it's worth investigating.
This PR sort of introduce the StimulusJS product modal ( I just reused the one we use for the product preview in the backoffice. ). The modal is included the html, but we could probably do something similar to what is done in the backoffice, load the modal via an AJAX call.
@openfoodfoundation/reviewers what do you think ?

The modal is included the html, but we could probably do something similar to what is done in the backoffice, load the modal via an AJAX call.

That sounds good. We probably don't want to load all the product modals into the shop list. That would be bad for performance.

…y and consistency.

chahmedejaz · 2026-06-22T01:35:36Z

Hi @roug - Thanks for the feedback. I've addressed it in the following commits. It's ready for review. Thanks

rioug

Thanks @chahmedejaz , it's much better like this. Maybe we can slightly improve one of spec, but it's otherwise good to go !

rioug · 2026-06-22T03:32:29Z

+    if (!container) return;
+
+    this.abortPendingRequest();
+    this.abortController = new AbortController();


TIL about AbortController !

rioug · 2026-06-22T03:34:08Z

+  async load(url) {
+    if (!url) return;
+
+    const container = document.getElementById("shop-product-modal-container");


I think ideally this would be target associated with the controller, but it's probably too tricky with our existing HMTL.

rioug · 2026-06-22T03:45:16Z

+  .row
+    .columns.small-12.medium-6.large-6.product-header
+      %h3= @product.name
+      - if @supplier
+        %span
+          %em= t("products_from")
+          %span= @supplier.name
+
+      %br
+
+      .filter-shopfront.property-selectors.inline-block
+        %ul
+          - @product.properties_including_inherited.each do |property|
+            - next if property[:name].blank?
+            %li
+              %span= property[:name]
+
+      - if @product.description.present?
+        .product-description{ "data-controller" => "add-blank-to-link" }
+          %p.text-small
+            - # description is sanitized in Spree::Product#description method
+            = @product.description.html_safe
+
+    .columns.small-12.medium-6.large-6.product-img
+      = render ThumbnailCarouselComponent.new(images: @carousel_images, show_captions: true, class: (@product.images.any? ? nil : "placeholder"))


It would have been better to reuse the existing template from the backoffice, like what I did here : 67316ea
That said, it's going to create conflict if we both do the same change : ) Let's leave if as it is, and I'll sort it out while I work on the Product Grid view.

rioug · 2026-06-22T03:48:19Z


  def open_product_modal(product)
-    page.find("a", text: product.name).click
+    page.find("#product-#{product.id} [data-controller~='shop-product-modal']",


This would be better if it wasn't tied to the data-controller name, can't you use the product link like before ?

mkllnk

I have to admit that I looked at this in a rush. Generally it seems okay and Mario verified that it worked. It's a lot more code than I hoped though. I don't have any suggestions for improvements. I guess, I was just hoping that using an existing component would be much easier.

chahmedejaz added 3 commits May 23, 2026 23:00

13981 - support multiple images in the product

16bec03

13981 - Implement carousel components for multiple images with thumbn…

24141a0

…ail navigation and use it in admin product preview

Add carousel support for multiple product images with navigation on f…

0234657

…rontshop

github-project-automation Bot added this to OFN Delivery board May 27, 2026

github-project-automation Bot moved this to All the things 💤 in OFN Delivery board May 27, 2026

chahmedejaz moved this from All the things 💤 to In Progress ⚙ in OFN Delivery board May 27, 2026

github-actions Bot assigned chahmedejaz May 27, 2026

chahmedejaz added pr-staged-fr staging.coopcircuits.fr user facing changes Thes pull requests affect the user experience labels May 27, 2026

AMEA-LYON removed the pr-staged-fr staging.coopcircuits.fr label Jun 1, 2026

chahmedejaz added 3 commits June 3, 2026 01:31

Address design feedback

58924e8

fix lint issues

83cf9e8

Enhance product carousel to conditionally show captions for multiple …

42f1ea8

…images

chahmedejaz added technical changes only These pull requests do not contain user facing changes and are grouped in release notes and removed user facing changes Thes pull requests affect the user experience labels Jun 2, 2026

fix typo

cdfb016

chahmedejaz marked this pull request as ready for review June 3, 2026 06:47

chahmedejaz added the feedback-needed label Jun 3, 2026

chahmedejaz mentioned this pull request Jun 3, 2026

Prevent admins to be able to view carousel in shops #14364

Merged

4 tasks

sigmundpetersen moved this from In Progress ⚙ to Code review 🔎 in OFN Delivery board Jun 3, 2026

chahmedejaz moved this from Code review 🔎 to In Progress ⚙ in OFN Delivery board Jun 3, 2026

chahmedejaz added the pr-staged-au staging.openfoodnetwork.org.au label Jun 11, 2026

chahmedejaz removed pr-staged-au staging.openfoodnetwork.org.au feedback-needed labels Jun 16, 2026

chahmedejaz marked this pull request as ready for review June 16, 2026 23:21

chahmedejaz moved this from In Progress ⚙ to Code review 🔎 in OFN Delivery board Jun 16, 2026

rioug added user facing changes Thes pull requests affect the user experience and removed dependencies technical changes only These pull requests do not contain user facing changes and are grouped in release notes labels Jun 16, 2026

rioug self-requested a review June 17, 2026 00:03

rioug reviewed Jun 17, 2026

View reviewed changes

mariocarabotta mentioned this pull request Jun 19, 2026

[Grid view] Display product as a grid #14345

Open

4 tasks

Refactor carousel styles to use variables for improved maintainabilit…

2cfbecd

…y and consistency.

chahmedejaz force-pushed the task/13981-carousel-for-multiple-images branch 2 times, most recently from 0dc2f72 to 9a9ef3d Compare June 21, 2026 23:02

rioug moved this from Code review 🔎 to In Progress ⚙ in OFN Delivery board Jun 21, 2026

chahmedejaz force-pushed the task/13981-carousel-for-multiple-images branch 3 times, most recently from e483f6b to bf9ab90 Compare June 22, 2026 01:01

Migrate product modal from AngularJS to Stimulus

f6dccf8

chahmedejaz force-pushed the task/13981-carousel-for-multiple-images branch from bf9ab90 to f6dccf8 Compare June 22, 2026 01:29

chahmedejaz moved this from In Progress ⚙ to Code review 🔎 in OFN Delivery board Jun 22, 2026

rioug approved these changes Jun 22, 2026

View reviewed changes

chahmedejaz requested a review from mkllnk June 24, 2026 01:51

mkllnk approved these changes Jun 29, 2026

View reviewed changes

mkllnk moved this from Code review 🔎 to Test Ready 🧪 in OFN Delivery board Jun 29, 2026

chahmedejaz mentioned this pull request Jul 3, 2026

Prevent admins to be able to view carousel in shops #14495

Draft

4 tasks

mariocarabotta added the priority We focus on this issue right now label Jul 8, 2026

	// z-index
	//--------------------------------------------------------------
	$tos-banner-z-index: 1001;
	$flash-message-z-index: 1000;
	$tag-drop-down-z-index: 999;

	$ts-dropdown-z-index: 101;
	$save-bar-z-index: 100;

		@@ -0,0 +1,30 @@
		# frozen_string_literal: true

		RSpec.describe CarouselComponent, type: :component do

Uh oh!

Uh oh!

Conversation

chahmedejaz commented May 27, 2026 • edited by rioug Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What? Why?

What should we test?

Single image products

Multiple image products (shop)

Multiple image products (admin)

Image management

Regression testing

Release notes

Dependencies

Documentation updates

Uh oh!

chahmedejaz commented May 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mariocarabotta commented May 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RachL commented May 27, 2026

Uh oh!

mariocarabotta commented May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RachL commented May 28, 2026

Uh oh!

chahmedejaz commented May 28, 2026

Uh oh!

mariocarabotta commented May 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RachL commented May 29, 2026

Uh oh!

chahmedejaz commented Jun 3, 2026

Uh oh!

chahmedejaz commented Jun 11, 2026

Uh oh!

mariocarabotta commented Jun 15, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chahmedejaz commented Jun 22, 2026

Uh oh!

rioug left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mkllnk left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

chahmedejaz commented May 27, 2026 •

edited by rioug

Loading

chahmedejaz commented May 27, 2026 •

edited

Loading

mariocarabotta commented May 27, 2026 •

edited

Loading

mariocarabotta commented May 28, 2026 •

edited

Loading

mariocarabotta commented May 29, 2026 •

edited

Loading