From 80578e75f05da0f1a89a9f444c7fc57d165acf4e Mon Sep 17 00:00:00 2001 From: David Battersby Date: Thu, 13 Jul 2023 20:36:22 +0800 Subject: [PATCH] FIX: add tracked property for items in lightbox carousel (#22592) The new lightbox was missing the tracked property for items when it was launched earlier as experimental feature flag. This PR should fix issues experienced when the user clicks between multiple galleries causing the carousel images not to be updated as they were previously not tracked. --- .../discourse/app/components/d-lightbox.js | 4 ++ .../tests/acceptance/d-lightbox-test.js | 41 +++++++++++++++---- 2 files changed, 38 insertions(+), 7 deletions(-) diff --git a/app/assets/javascripts/discourse/app/components/d-lightbox.js b/app/assets/javascripts/discourse/app/components/d-lightbox.js index 87d3e6abe11..15c7cc0d106 100644 --- a/app/assets/javascripts/discourse/app/components/d-lightbox.js +++ b/app/assets/javascripts/discourse/app/components/d-lightbox.js @@ -27,6 +27,7 @@ import { tracked } from "@glimmer/tracking"; export default class DLightbox extends Component { @service appEvents; + @tracked items = []; @tracked isVisible = false; @tracked isLoading = false; @tracked currentIndex = 0; @@ -40,6 +41,9 @@ export default class DLightbox extends Component { @tracked hasCarousel = false; @tracked hasExpandedTitle = false; + options = {}; + callbacks = {}; + willClose = false; elementId = LIGHTBOX_ELEMENT_ID; titleElementId = TITLE_ELEMENT_ID; animationDuration = ANIMATION_DURATION; diff --git a/app/assets/javascripts/discourse/tests/acceptance/d-lightbox-test.js b/app/assets/javascripts/discourse/tests/acceptance/d-lightbox-test.js index 0da932577ef..7ff8b685c67 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/d-lightbox-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/d-lightbox-test.js @@ -51,7 +51,14 @@ ${generateLightboxMarkup(LIGHTBOX_IMAGE_FIXTURES.second)}`; function setupPretender(server, helper, markup) { const topicResponse = cloneJSON(topicFixtures["/t/280/1.json"]); - topicResponse.post_stream.posts[0].cooked += markup; + let markupFromArray = Array.isArray(markup); + let responseTotal = markupFromArray ? markup.length : 1; + + for (let i = 0; i < responseTotal; i++) { + topicResponse.post_stream.posts[i].cooked += markupFromArray + ? markup[i] + : markup; + } server.get("/t/280.json", () => helper.response(topicResponse)); server.get("/t/280/:post_number.json", () => helper.response(topicResponse)); @@ -632,11 +639,10 @@ acceptance("Experimental Lightbox - carousel", function (needs) { needs.settings({ enable_experimental_lightbox: true }); needs.pretender((server, helper) => - setupPretender( - server, - helper, - multipleLargeImagesMarkup + multipleLargeImagesMarkup - ) + setupPretender(server, helper, [ + multipleLargeImagesMarkup + multipleLargeImagesMarkup, + multipleLargeImagesMarkup, + ]) ); test("navigation", async function (assert) { @@ -705,7 +711,7 @@ acceptance("Experimental Lightbox - carousel", function (needs) { const lightboxes = [...queryAll(SELECTORS.DEFAULT_ITEM_SELECTOR)]; - const lastThreeLightboxes = lightboxes.slice(-3); + const lastThreeLightboxes = lightboxes.slice(-6); lastThreeLightboxes.forEach((lightbox) => { lightbox.remove(); @@ -731,6 +737,27 @@ acceptance("Experimental Lightbox - carousel", function (needs) { await click(SELECTORS.CLOSE_BUTTON); assert.dom(SELECTORS.LIGHTBOX_CONTENT).doesNotExist(); }); + + test("images update when changing galleries", async function (assert) { + await visit("/t/internationalization-localization/280"); + + // click on image from first gallery + await click("#post_1 " + SELECTORS.DEFAULT_ITEM_SELECTOR + ":nth-child(1)"); + + // toggle carousel open + await click(SELECTORS.CAROUSEL_BUTTON); + + // number of images in carousel matches first gallery + assert.dom(SELECTORS.CAROUSEL_ITEM).exists({ count: 6 }); + + await click(SELECTORS.CLOSE_BUTTON); + + // click on image from second gallery + await click("#post_2 " + SELECTORS.DEFAULT_ITEM_SELECTOR + ":nth-child(1)"); + + // number of images in carousel matches second gallery + assert.dom(SELECTORS.CAROUSEL_ITEM).exists({ count: 3 }); + }); }); acceptance("Experimental Lightbox - mobile", function (needs) {