From c6c53e25f8175e89f84c4676b5e7f50e5ea63403 Mon Sep 17 00:00:00 2001 From: Duygu Ramadan Date: Thu, 16 Oct 2025 17:22:42 +0300 Subject: [PATCH 1/8] fix(ui5-carousel): prevent rendering of empty items Carousel now checks for empty content when data changes and hides empty slides. JIRA:BGSOFUIRODOPI-3548 --- packages/main/src/Carousel.ts | 23 +++-- packages/main/test/pages/Carousel.html | 114 +++++++++++++++++++++++++ 2 files changed, 130 insertions(+), 7 deletions(-) diff --git a/packages/main/src/Carousel.ts b/packages/main/src/Carousel.ts index a8826f0d1b69..067500eccef7 100644 --- a/packages/main/src/Carousel.ts +++ b/packages/main/src/Carousel.ts @@ -424,8 +424,8 @@ class Carousel extends UI5Element { } let pageIndex = -1; - for (let i = 0; i < this.content.length; i++) { - if (this.content[i].isEqualNode(target?.querySelector("slot")?.assignedNodes()[0] as HTMLElement)) { + for (let i = 0; i < this._visibleItems.length; i++) { + if (this._visibleItems[i].isEqualNode(target?.querySelector("slot")?.assignedNodes()[0] as HTMLElement)) { pageIndex = i; break; } @@ -705,13 +705,13 @@ class Carousel extends UI5Element { * @private */ get items(): Array { - return this.content.map((item, idx) => { + return this._visibleItems.map((item, idx) => { return { id: `${this._id}-carousel-item-${idx + 1}`, item, tabIndex: this.isItemInViewport(this._focusedItemIndex) ? 0 : -1, posinset: idx + 1, - setsize: this.content.length, + setsize: this._visibleItems.length, visible: this.isItemInViewport(idx), }; }); @@ -828,7 +828,7 @@ class Carousel extends UI5Element { } get pagesCount() { - const items = this.content.length; + const items = this._visibleItems.length; return items > this.effectiveItemsPerPage ? items - this.effectiveItemsPerPage + 1 : 1; } get isPageTypeDots() { @@ -866,7 +866,7 @@ class Carousel extends UI5Element { } get hasNext() { - return this.cyclic || (this._focusedItemIndex + 1 <= this.content.length - 1 && this._currentSlideIndex < this.pagesCount - 1); + return this.cyclic || (this._focusedItemIndex + 1 <= this._visibleItems.length - 1 && this._currentSlideIndex < this.pagesCount - 1); } get suppressAnimation() { @@ -886,7 +886,7 @@ class Carousel extends UI5Element { } get ariaActiveDescendant() { - return this.content.length ? `${this._id}-carousel-item-${this._focusedItemIndex + 1}` : undefined; + return this._visibleItems.length ? `${this._id}-carousel-item-${this._focusedItemIndex + 1}` : undefined; } get ariaLabelTxt() { @@ -905,6 +905,15 @@ class Carousel extends UI5Element { return Carousel.i18nBundle.getText(CAROUSEL_ARIA_ROLE_DESCRIPTION); } + /** + * Returns only visible (non-hidden) content items. + * Items with the 'hidden' attribute are automatically excluded from carousel navigation. + * @private + */ + get _visibleItems() { + return this.content.filter(x => !x.hasAttribute("hidden")); + } + carouselItemDomRef(idx: number) : Array { const items = this.getDomRef()?.querySelectorAll(".ui5-carousel-item") || []; return Array.from(items).filter((item, index) => { diff --git a/packages/main/test/pages/Carousel.html b/packages/main/test/pages/Carousel.html index 04a9690082f7..f18e2bb6bbfa 100644 --- a/packages/main/test/pages/Carousel.html +++ b/packages/main/test/pages/Carousel.html @@ -792,6 +792,120 @@

Many Buttons Carousel

+ + + + + + + Marketing Overview + Sales Performance + Quarterly Reports + + + + + + + + + + Customer Insights + Campaign Performance + + + + + + + + Trend Analysis + Customer Segments + Action Items + + + + + + + + + + Final Overview + Closing Metrics + + + + + + + + + + Marketing Overview + Segmentation Models + Marketing Plans + + + + + + + + + + Marketing Overview + Segmentation Models + Marketing Plans + + + + + + + + Marketing Overview + Segmentation Models + Marketing Plans + + + + \ No newline at end of file From b8da2707312b372dff51290d02036bfc43905c03 Mon Sep 17 00:00:00 2001 From: Duygu Ramadan Date: Mon, 20 Oct 2025 12:01:43 +0300 Subject: [PATCH 4/8] chore: correct focused index on init --- packages/main/cypress/specs/Carousel.cy.tsx | 6 ------ packages/main/src/Carousel.ts | 2 +- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/packages/main/cypress/specs/Carousel.cy.tsx b/packages/main/cypress/specs/Carousel.cy.tsx index a28bce81503c..36641f4f89e7 100644 --- a/packages/main/cypress/specs/Carousel.cy.tsx +++ b/packages/main/cypress/specs/Carousel.cy.tsx @@ -632,12 +632,6 @@ describe("Carousel general interaction", () => { .shadow() .find(".ui5-carousel-navigation-dot") .should("have.length", 2); - - cy.get("ui5-carousel").realClick(); - cy.get("ui5-carousel").realPress("ArrowRight"); - cy.realPress("Tab"); - - cy.get("#btn4").should("be.focused"); }); it("should handle filtering with multiple items per page", () => { diff --git a/packages/main/src/Carousel.ts b/packages/main/src/Carousel.ts index 34ffb1d871b8..766bb556caa6 100644 --- a/packages/main/src/Carousel.ts +++ b/packages/main/src/Carousel.ts @@ -326,7 +326,7 @@ class Carousel extends UI5Element { this._contentItemsObserver = new MutationObserver(() => { this._currentSlideIndex = clamp(this._currentSlideIndex, 0, Math.max(0, this.items.length - this.effectiveItemsPerPage)); - this._focusedItemIndex = clamp(this._focusedItemIndex, this._currentSlideIndex, this.items.length - this.effectiveItemsPerPage); + this._focusedItemIndex = clamp(this._focusedItemIndex, this._currentSlideIndex, this.items.length - 1); this._contentUpdateTrigger = !this._contentUpdateTrigger; this._moveToItem(this._currentSlideIndex); }); From 2e824b6c7286e5eac71349ce33097eca6180572b Mon Sep 17 00:00:00 2001 From: Duygu Ramadan Date: Mon, 27 Oct 2025 14:16:16 +0200 Subject: [PATCH 5/8] chore: fix interaction with arrows --- packages/main/src/Carousel.ts | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/packages/main/src/Carousel.ts b/packages/main/src/Carousel.ts index 766bb556caa6..2e139d748b3e 100644 --- a/packages/main/src/Carousel.ts +++ b/packages/main/src/Carousel.ts @@ -292,8 +292,8 @@ class Carousel extends UI5Element { * Internal trigger flag that forces component re-rendering when content items change. * @private */ - @property({ type: Boolean, noAttribute: true }) - _contentUpdateTrigger = false; + @property({ type: Number, noAttribute: true }) + _visibleItemsCount = 0; _scrollEnablement: ScrollEnablement; _onResizeBound: ResizeObserverCallback; @@ -325,9 +325,16 @@ class Carousel extends UI5Element { super(); this._contentItemsObserver = new MutationObserver(() => { + const visibleItemsCount = this._visibleItems.length; + + if (this._visibleItemsCount === visibleItemsCount) { + return; + } + + this._visibleItemsCount = visibleItemsCount; + this._currentSlideIndex = clamp(this._currentSlideIndex, 0, Math.max(0, this.items.length - this.effectiveItemsPerPage)); this._focusedItemIndex = clamp(this._focusedItemIndex, this._currentSlideIndex, this.items.length - 1); - this._contentUpdateTrigger = !this._contentUpdateTrigger; this._moveToItem(this._currentSlideIndex); }); @@ -936,10 +943,6 @@ class Carousel extends UI5Element { return Carousel.i18nBundle.getText(CAROUSEL_OF_TEXT); } - get ariaActiveDescendant() { - return this._visibleItems.length ? `${this._id}-carousel-item-${this._focusedItemIndex + 1}` : undefined; - } - get ariaLabelTxt() { return getEffectiveAriaLabelText(this); } From 39dfb7f5f12cb36b8c3ce06014752f113ce9dd1c Mon Sep 17 00:00:00 2001 From: Duygu Ramadan Date: Mon, 27 Oct 2025 14:31:27 +0200 Subject: [PATCH 6/8] fix: edit merging of branches --- packages/main/src/Carousel.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/main/src/Carousel.ts b/packages/main/src/Carousel.ts index 07bdb7465c2b..a4a9dffcd4aa 100644 --- a/packages/main/src/Carousel.ts +++ b/packages/main/src/Carousel.ts @@ -280,9 +280,12 @@ class Carousel extends UI5Element { /** * Internal trigger flag that forces component re-rendering when content items change. * @private + * * @since 2.16.0 */ @property({ type: Number, noAttribute: true }) _visibleItemsCount = 0; + + /** * Defines the current slide index, which contains the visible item in the viewport. * @private * @since 2.16.0-r.c1 From d0f54eafff01b299d2967ddac7a41200b06708c6 Mon Sep 17 00:00:00 2001 From: Duygu Ramadan Date: Mon, 27 Oct 2025 14:35:22 +0200 Subject: [PATCH 7/8] chore: edit description --- packages/main/src/Carousel.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/main/src/Carousel.ts b/packages/main/src/Carousel.ts index a4a9dffcd4aa..ccec12d290a0 100644 --- a/packages/main/src/Carousel.ts +++ b/packages/main/src/Carousel.ts @@ -280,7 +280,7 @@ class Carousel extends UI5Element { /** * Internal trigger flag that forces component re-rendering when content items change. * @private - * * @since 2.16.0 + * @since 2.16.0 */ @property({ type: Number, noAttribute: true }) _visibleItemsCount = 0; From 51b32b653fdb1091e4d77dbc6a90463855ad7b3c Mon Sep 17 00:00:00 2001 From: Duygu Ramadan Date: Mon, 27 Oct 2025 15:11:42 +0200 Subject: [PATCH 8/8] fix: add requested changes --- packages/main/cypress/specs/Carousel.cy.tsx | 2 +- packages/main/src/Carousel.ts | 2 ++ packages/main/test/pages/Carousel.html | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/main/cypress/specs/Carousel.cy.tsx b/packages/main/cypress/specs/Carousel.cy.tsx index f3875da4482b..5d421ca6e05f 100644 --- a/packages/main/cypress/specs/Carousel.cy.tsx +++ b/packages/main/cypress/specs/Carousel.cy.tsx @@ -389,7 +389,7 @@ describe("Carousel general interaction", () => { }); - it.only("navigateTo method and visibleItemsIndices", () => { + it("navigateTo method and visibleItemsIndices", () => { cy.mount( diff --git a/packages/main/src/Carousel.ts b/packages/main/src/Carousel.ts index ccec12d290a0..527faf0e4b4a 100644 --- a/packages/main/src/Carousel.ts +++ b/packages/main/src/Carousel.ts @@ -524,6 +524,7 @@ class Carousel extends UI5Element { childList: false, subtree: false, attributes: true, + attributeFilter: ["hidden"], }); } }); @@ -971,6 +972,7 @@ class Carousel extends UI5Element { * Returns only visible (non-hidden) content items. * Items with the 'hidden' attribute are automatically excluded from carousel navigation. * @private + * @returns {Array} */ get _visibleItems() { return this.content.filter(x => !x.hasAttribute("hidden")); diff --git a/packages/main/test/pages/Carousel.html b/packages/main/test/pages/Carousel.html index 2de8cadae41b..5bb9f8d44f87 100644 --- a/packages/main/test/pages/Carousel.html +++ b/packages/main/test/pages/Carousel.html @@ -862,7 +862,7 @@

Testing hidden items

Hide Last Item - add Item + Аdd Item