From 9741fab3d1af627739a0c25b2aa2eedc0031a909 Mon Sep 17 00:00:00 2001 From: Gergana Georgieva Date: Wed, 3 Sep 2025 17:19:52 +0300 Subject: [PATCH 01/17] feat(ui5-carousel): focus is on the items --- packages/main/cypress/specs/Carousel.cy.tsx | 28 --- packages/main/src/Carousel.ts | 184 +++++++++++++++----- packages/main/src/CarouselTemplate.tsx | 11 +- packages/main/src/themes/Carousel.css | 4 +- packages/main/test/pages/Carousel.html | 14 +- 5 files changed, 153 insertions(+), 88 deletions(-) diff --git a/packages/main/cypress/specs/Carousel.cy.tsx b/packages/main/cypress/specs/Carousel.cy.tsx index 7a548c2b96a6..b540e040041f 100644 --- a/packages/main/cypress/specs/Carousel.cy.tsx +++ b/packages/main/cypress/specs/Carousel.cy.tsx @@ -403,34 +403,6 @@ describe("Carousel general interaction", () => { }); - it("navigateTo method and visibleItemsIndices", () => { - cy.mount( - - - - - - - - - - - - ); - - cy.get("#carousel9") - .invoke("prop", "visibleItemsIndices") - .should("deep.equal", [0, 1]); - - cy.get("#carousel9").then(($carousel) => { - $carousel[0].navigateTo(1); - }); - - cy.get("#carousel9") - .invoke("prop", "visibleItemsIndices") - .should("deep.equal", [1, 2]); - }); - it("F7 keyboard navigation", () => { cy.mount( diff --git a/packages/main/src/Carousel.ts b/packages/main/src/Carousel.ts index 99886d58bf44..01d66d2fa640 100644 --- a/packages/main/src/Carousel.ts +++ b/packages/main/src/Carousel.ts @@ -51,7 +51,7 @@ type ItemsInfo = { tabIndex: number, posinset: number, setsize: number, - selected: boolean, + visible: boolean, _individualSlot?: string, } @@ -277,6 +277,10 @@ class Carousel extends UI5Element { _resizing: boolean; _lastFocusedElements: Array; _orderOfLastFocusedPages: Array; + _visibleItemsIndexes: Array; + _calculatedX: number = 0; + _itemIndicator: number = 0; + _currentSlideIndex: number = 0; /** * Defines the content of the component. @@ -304,6 +308,7 @@ class Carousel extends UI5Element { this._lastFocusedElements = []; this._orderOfLastFocusedPages = []; + this._visibleItemsIndexes = []; } onBeforeRendering() { @@ -324,6 +329,9 @@ class Carousel extends UI5Element { if (isDesktop()) { this.setAttribute("desktop", ""); } + this._width = this.offsetWidth; + this._updateVisibleItems(this._selectedIndex); + this._currentSlideIndex = Math.min(this._selectedIndex, this.pagesCount - 1); } onExitDOM() { @@ -345,6 +353,7 @@ class Carousel extends UI5Element { // Change transitively effectiveItemsPerPage by modifying _width this._width = this.offsetWidth; this._itemWidth = Math.floor(this._width / this.effectiveItemsPerPage); + this._updateVisibleItems(this._selectedIndex); // Items per page did not change or the current, // therefore page index does not need to be re-adjusted @@ -352,8 +361,8 @@ class Carousel extends UI5Element { return; } - if (this._selectedIndex > this.pagesCount - 1) { - this._selectedIndex = this.pagesCount - 1; + if (this._selectedIndex > this.items.length - 1) { + this._selectedIndex = this.items.length - 1; this.fireDecoratorEvent("navigate", { selectedIndex: this._selectedIndex }); } } @@ -370,24 +379,16 @@ class Carousel extends UI5Element { } } - async _onkeydown(e: KeyboardEvent) { + _onkeydown(e: KeyboardEvent) { if (isF7(e)) { this._handleF7Key(e); return; } - if (e.target !== this.getDomRef()) { - return; - } - if (isLeft(e) || isDown(e)) { this.navigateLeft(); - await renderFinished(); - this.getDomRef()!.focus(); } else if (isRight(e) || isUp(e)) { this.navigateRight(); - await renderFinished(); - this.getDomRef()!.focus(); } } @@ -400,7 +401,7 @@ class Carousel extends UI5Element { let pageIndex = -1; for (let i = 0; i < this.content.length; i++) { - if (this.content[i].contains(target)) { + if (this.content[i].isEqualNode(target?.querySelector("slot")?.assignedNodes()[0] as HTMLElement)) { pageIndex = i; break; } @@ -410,6 +411,7 @@ class Carousel extends UI5Element { return; } + this._selectedIndex = pageIndex; // Save reference of the last focused element for each page this._lastFocusedElements[pageIndex] = target; @@ -439,6 +441,7 @@ class Carousel extends UI5Element { if (e.target === this.getDomRef() && lastFocusedElement) { lastFocusedElement.focus(); } else { + //@TODO where shoul the focus go? this.getDomRef()!.focus(); } } @@ -465,14 +468,21 @@ class Carousel extends UI5Element { const previousSelectedIndex = this._selectedIndex; if (this._selectedIndex - 1 < 0) { + // TODO clean up logic, no cyclic for morre than pne page if (this.cyclic) { - this._selectedIndex = this.pagesCount - 1; + // make better + if (this._selectedIndex === 0 && this.effectiveItemsPerPage > 1) { + this._selectedIndex = 0; + } else { + this._selectedIndex = this.items.length - 1; + } } } else { --this._selectedIndex; } if (previousSelectedIndex !== this._selectedIndex) { + this.skipToItem(this._selectedIndex, -1); this.fireDecoratorEvent("navigate", { selectedIndex: this._selectedIndex }); } } @@ -482,9 +492,14 @@ class Carousel extends UI5Element { const previousSelectedIndex = this._selectedIndex; - if (this._selectedIndex + 1 > this.pagesCount - 1) { + if (this._selectedIndex + 1 > this.items.length - 1) { + // TODO clean up logic, no cyclic for mоre than оne page if (this.cyclic) { - this._selectedIndex = 0; + if (this._selectedIndex === this.items.length - 1 && this.effectiveItemsPerPage > 1) { + this._selectedIndex = this.items.length - 1; + } else { + this._selectedIndex = 0; + } } else { return; } @@ -492,11 +507,68 @@ class Carousel extends UI5Element { ++this._selectedIndex; } + //TOOD fix fast clicking of arrow left breaks animation if (previousSelectedIndex !== this._selectedIndex) { + this.skipToItem(this._selectedIndex, 1); this.fireDecoratorEvent("navigate", { selectedIndex: this._selectedIndex }); } } + _calculateItemSlideIndex(currentSlideIndex: number, itemStep: number) { + const itemsPerPage = this.effectiveItemsPerPage; + + let slideIndex; + + if (itemsPerPage > 1) { + if (currentSlideIndex === 0 && itemStep < 0) { + return 0; + } + + if (currentSlideIndex >= this.pagesCount && itemStep > 0) { + return this.pagesCount - 1; + } + + slideIndex = currentSlideIndex + itemStep; + } else { + slideIndex = itemStep > 0 ? currentSlideIndex + 1 : currentSlideIndex - 1; + if (this.cyclic) { + if (currentSlideIndex === 0 && itemStep < 0) { + return this.pagesCount - 1; + } + + if (currentSlideIndex === this.items.length - 1 && itemStep > 0) { + return 0; + } + } + } + return slideIndex; + } + + _moveToItem(slideIndex: number) { + if (this.items.length === 0) { + return; + } + + const itemsInViewportToShow = this.effectiveItemsPerPage, + itemsCount = this.items.length, + cyclic = this.cyclic; + + if (cyclic && itemsInViewportToShow !== 1 && (slideIndex < 0 || slideIndex > itemsCount - 1)) { + return; + } + + if (slideIndex + itemsInViewportToShow > itemsCount - 1) { + slideIndex = itemsCount - itemsInViewportToShow; + } + + this._updateVisibleItems(slideIndex); + this._currentSlideIndex = slideIndex; + } + + focusItem() { + this.carouselItemDomRef(this._selectedIndex)[0].focus(); + } + _navButtonClick(e: UI5CustomEvent) { const button = e.target as Button; if (button.hasAttribute("data-ui5-arrow-forward")) { @@ -504,8 +576,6 @@ class Carousel extends UI5Element { } else { this.navigateLeft(); } - - this.focus(); } /** @@ -514,29 +584,41 @@ class Carousel extends UI5Element { * @since 1.0.0-rc.15 * @public */ - navigateTo(itemIndex: number) : void { + navigateTo(itemIndex: number) { + //TODO test this and fix this._resizing = false; + //TODO check if _SelectedIndex is grater or smaller than itemIndex for skipToItemOffset this._selectedIndex = itemIndex; + this._currentSlideIndex = itemIndex; + + this.skipToItem(this._selectedIndex, 1); } + async skipToItem(focusIndex: number, offset: number) { + if (!this.isItemInViewport(focusIndex)) { + const slideIndex = this._calculateItemSlideIndex(this._currentSlideIndex, offset); + console.log(" SLIDE INDEX ", slideIndex); + this._moveToItem(slideIndex); + } + await renderFinished(); + this.focusItem(); + } /** * Assuming that all items have the same width * @private */ get items(): Array { return this.content.map((item, idx) => { - const visible = this.isItemInViewport(idx); return { id: `${this._id}-carousel-item-${idx + 1}`, item, - tabIndex: visible ? 0 : -1, + tabIndex: this._selectedIndex === idx ? 0 : -1, posinset: idx + 1, setsize: this.content.length, - selected: visible, + visible: this.isItemInViewport(idx), }; }); } - get effectiveItemsPerPage(): number { const itemsPerPageArray = this.itemsPerPage.split(" "); let itemsPerPageSizeS = 1, @@ -576,11 +658,33 @@ class Carousel extends UI5Element { } isItemInViewport(index: number): boolean { - return index >= this._selectedIndex && index <= this._selectedIndex + this.effectiveItemsPerPage - 1; + return this._visibleItemsIndexes.includes(index); + } + + _updateVisibleItems(index:number) { + let newItemIndex = index; + const effectiveItemsPerPage: number = this.effectiveItemsPerPage; + const items = this.items; + + if (!items.length) { + return; + } + + if (newItemIndex > items.length - effectiveItemsPerPage) { + newItemIndex = items.length - effectiveItemsPerPage; + } + const lastItemIndex = newItemIndex + effectiveItemsPerPage; + + this._visibleItemsIndexes = []; + + for (let i = newItemIndex; i < lastItemIndex; i++) { + this._visibleItemsIndexes.push(i); + } + console.log(" VISIBLE ", this._visibleItemsIndexes); } isIndexInRange(index: number): boolean { - return index >= 0 && index <= this.pagesCount - 1; + return index >= 0 && index <= this.items.length - 1; } /** @@ -631,7 +735,6 @@ class Carousel extends UI5Element { const items = this.content.length; return items > this.effectiveItemsPerPage ? items - this.effectiveItemsPerPage + 1 : 1; } - get isPageTypeDots() { if (this.pageIndicatorType === CarouselPageIndicatorType.Numeric) { return false; @@ -646,7 +749,7 @@ class Carousel extends UI5Element { for (let index = 0; index < pages; index++) { dots.push({ - active: index === this._selectedIndex, + active: index === this._currentSlideIndex, ariaLabel: Carousel.i18nBundle.getText(CAROUSEL_DOT_TEXT, index + 1, pages), }); } @@ -663,11 +766,11 @@ class Carousel extends UI5Element { } get hasPrev() { - return this.cyclic || this._selectedIndex - 1 >= 0; + return (this.cyclic && this._selectedIndex - 1 >= 0) || this.effectiveItemsPerPage === 1; } get hasNext() { - return this.cyclic || this._selectedIndex + 1 <= this.pagesCount - 1; + return (this.cyclic && this._selectedIndex + 1 <= this.content.length - 1) || this.effectiveItemsPerPage === 1; } get suppressAnimation() { @@ -679,7 +782,7 @@ class Carousel extends UI5Element { } get selectedIndexToShow() { - return this._isRTL ? this.pagesCount - (this.pagesCount - this._selectedIndex) + 1 : this._selectedIndex + 1; + return this._isRTL ? this.items.length - (this.items.length - this._selectedIndex) + 1 : this._selectedIndex + 1; } get ofText() { @@ -706,22 +809,11 @@ class Carousel extends UI5Element { return Carousel.i18nBundle.getText(CAROUSEL_ARIA_ROLE_DESCRIPTION); } - /** - * The indices of the currently visible items of the component. - * @public - * @since 1.0.0-rc.15 - * @default [] - */ - get visibleItemsIndices() : Array { - const visibleItemsIndices: Array = []; - - this.items.forEach((item, index) => { - if (this.isItemInViewport(index)) { - visibleItemsIndices.push(index); - } - }); - - return visibleItemsIndices; + carouselItemDomRef(idx: number) : Array { + const items = this.getDomRef()?.querySelectorAll(".ui5-carousel-item") || []; + return Array.from(items).filter((item, index) => { + return index === idx; + }) as Array; } } diff --git a/packages/main/src/CarouselTemplate.tsx b/packages/main/src/CarouselTemplate.tsx index 714a7eeddbc5..a4e160deb3ef 100644 --- a/packages/main/src/CarouselTemplate.tsx +++ b/packages/main/src/CarouselTemplate.tsx @@ -10,7 +10,6 @@ export default function CarouselTemplate(this: Carousel) { "ui5-carousel-root": true, [`ui5-carousel-background-${this._backgroundDesign}`]: true, }} - tabindex={0} role="list" aria-label={this.ariaLabelTxt} aria-roledescription={this._roleDescription} @@ -21,20 +20,22 @@ export default function CarouselTemplate(this: Carousel) { onMouseOver={this._onmouseover} >
-
+
{this.items.map(itemInfo =>
@@ -105,5 +106,5 @@ function navIndicator(this: Carousel) {
{this.selectedIndexToShow} {this.ofText} {this.pagesCount}
; + >{this._currentSlideIndex + 1} {this.ofText} {this.pagesCount}
; } diff --git a/packages/main/src/themes/Carousel.css b/packages/main/src/themes/Carousel.css index 9f87a8d65aa8..ec9b5fdcd90d 100644 --- a/packages/main/src/themes/Carousel.css +++ b/packages/main/src/themes/Carousel.css @@ -8,8 +8,8 @@ height: 100%; } -:host([desktop]) .ui5-carousel-root:focus, -.ui5-carousel-root:focus-visible { +:host([desktop]) .ui5-carousel-item:focus, +.ui5-carousel-item:focus-visible { outline: var(--sapContent_FocusWidth) var(--sapContent_FocusStyle) var(--sapContent_FocusColor); } diff --git a/packages/main/test/pages/Carousel.html b/packages/main/test/pages/Carousel.html index 9bfb1a10a37e..41f31caccf7b 100644 --- a/packages/main/test/pages/Carousel.html +++ b/packages/main/test/pages/Carousel.html @@ -155,7 +155,7 @@ - + Template Based Segmentation @@ -164,7 +164,7 @@ - @@ -177,7 +177,7 @@ - + @@ -187,7 +187,7 @@ - + Template Based Segmentation @@ -196,7 +196,7 @@ - @@ -209,7 +209,7 @@ - + @@ -219,7 +219,7 @@ - + Template Based Segmentation From 6febbf2ed8c477b3a51eed399390aa57866a8543 Mon Sep 17 00:00:00 2001 From: Konstantin Kondov Date: Thu, 25 Sep 2025 13:57:11 +0300 Subject: [PATCH 02/17] feat(ui5-carousel): updates accessibility and adds keyboard handling Sets focus is on the carousel items rather than the carousel itself Ads keyboard handling for Home, End, PageUp, PageDown, F7 buttons Updates accessibility attributes --- packages/main/cypress/specs/Carousel.cy.tsx | 134 +++++++++++++----- packages/main/src/Carousel.ts | 104 ++++++++++---- packages/main/src/CarouselTemplate.tsx | 44 +++--- .../main/src/i18n/messagebundle.properties | 2 +- packages/main/src/themes/Carousel.css | 46 ++++-- packages/main/test/pages/Carousel.html | 7 +- 6 files changed, 245 insertions(+), 92 deletions(-) diff --git a/packages/main/cypress/specs/Carousel.cy.tsx b/packages/main/cypress/specs/Carousel.cy.tsx index b540e040041f..f86c251b431f 100644 --- a/packages/main/cypress/specs/Carousel.cy.tsx +++ b/packages/main/cypress/specs/Carousel.cy.tsx @@ -42,9 +42,9 @@ describe("Carousel general interaction", () => { ); cy.get("#carousel1") - .realHover() + .trigger("mouseover") .shadow() - .find(".ui5-carousel-navigation-arrows .ui5-carousel-navigation-button:not(.ui5-carousel-navigation-button--hidden)") + .find(".ui5-carousel-navigation-arrows .ui5-carousel-navigation-button:not(.ui5-carousel-navigation-button--hidden)").first() .realClick(); cy.get("#carousel1").should("have.prop", "_selectedIndex", 2); @@ -57,11 +57,12 @@ describe("Carousel general interaction", () => { ); + cy.get("#carousel1").should("have.prop", "_selectedIndex", 0); cy.get("#carousel1") - .realHover() + .trigger("mouseover") .shadow() - .find(".ui5-carousel-navigation-arrows .ui5-carousel-navigation-button:not(.ui5-carousel-navigation-button--hidden)") + .find(".ui5-carousel-navigation-arrows .ui5-carousel-navigation-button:not(.ui5-carousel-navigation-button--hidden)").last() .realClick(); cy.get("#carousel1").should("have.prop", "_selectedIndex", 1); @@ -117,19 +118,19 @@ describe("Carousel general interaction", () => { ); cy.get("#carousel2") - .realHover() + .trigger('mouseover') .shadow() .find(".ui5-carousel-navigation-arrows .ui5-carousel-navigation-button:not(.ui5-carousel-navigation-button--hidden)") - .should("have.length", 1); + .should("have.length", 2); cy.get("#carousel2") - .realHover() + .trigger('mouseover') .shadow() .find(".ui5-carousel-navigation-arrows .ui5-carousel-navigation-button:not(.ui5-carousel-navigation-button--hidden)") .realClick(); cy.get("#carousel2") - .realHover() + .trigger('mouseover') .shadow() .find(".ui5-carousel-navigation-arrows .ui5-carousel-navigation-button:not(.ui5-carousel-navigation-button--hidden)") .should("have.length", 2); @@ -216,8 +217,12 @@ describe("Carousel general interaction", () => { cy.get("#carousel5") .shadow() - .find(".ui5-carousel-item:first-child") - .should("have.attr", "aria-selected", "true"); + .find(".ui5-carousel-content").find(":first-child") + .realClick(); + cy.get("#carousel5") + .shadow() + .find(".ui5-carousel-content").find(":first-child") + .should("have.attr", "aria-hidden"); cy.get("#carousel5") .shadow() @@ -242,8 +247,9 @@ describe("Carousel general interaction", () => { }); cy.get("#carousel5") + .trigger('mouseover') .shadow() - .find(".ui5-carousel-navigation-button:nth-child(2)") + .find(".ui5-carousel-navigation-button:nth-child(1)") .realClick(); cy.get('#carousel5') @@ -252,7 +258,7 @@ describe("Carousel general interaction", () => { cy.get('#carousel5') .shadow() .find('.ui5-carousel-root') - .should('have.attr', 'aria-activedescendant', `${el._id}-carousel-item-2`); + .should('have.attr', 'aria-activedescendant', `${el._id}-carousel-item-1`); }); cy.get("#carouselAccName") @@ -328,7 +334,7 @@ describe("Carousel general interaction", () => { it("Event navigate fired when pressing navigation arrows", () => { const navigateEventStub = cy.stub().as("myStub"); cy.mount( - + @@ -340,29 +346,33 @@ describe("Carousel general interaction", () => { ); cy.get("#carousel8") + .trigger("mouseover") .shadow() - .find("ui5-button[data-ui5-arrow-forward]") + .find("span.ui5-carousel-navigation-button[data-ui5-arrow-forward]") .should("exist") .realClick(); cy.get("@myStub").should("have.been.calledOnce"); cy.get("#carousel8") + .trigger("mouseover") .shadow() - .find("ui5-button[data-ui5-arrow-forward]") + .find("span.ui5-carousel-navigation-button[data-ui5-arrow-forward]") .should("exist") .realClick(); cy.get("@myStub").should("have.been.calledTwice"); cy.get("#carousel8") + .trigger("mouseover") .shadow() - .find("ui5-button[data-ui5-arrow-back]") + .find("span.ui5-carousel-navigation-button[data-ui5-arrow-back]") .should("exist") .realClick(); cy.get("@myStub").should("have.been.calledThrice"); cy.get("#carousel8") + .trigger("mouseover") .shadow() - .find("ui5-button[data-ui5-arrow-back]") + .find("span.ui5-carousel-navigation-button[data-ui5-arrow-back]") .should("exist") .realClick(); cy.get("@myStub").should("have.callCount", 4); @@ -409,9 +419,9 @@ describe("Carousel general interaction", () => {
Page 1
- +
- +
@@ -469,31 +479,83 @@ describe("Carousel general interaction", () => { ); cy.get(".myCard").should("be.visible"); + cy.get("#carouselF7").shadow().find(".ui5-carousel-content").find(".ui5-carousel-item").first().focus(); + + cy.realPress("F7") + cy.wait(500) - cy.get("#carouselF7Button").realClick(); cy.get("#carouselF7Button").should('be.focused'); - cy.realPress("F7"); - cy.focused().should("have.class", "ui5-carousel-root"); + cy.realPress("F7") + cy.wait(500) + + cy.get("#carouselF7").shadow().find(".ui5-carousel-content").find(":first-child").should("be.focused"); + }); - cy.realPress("F7"); - cy.focused().should("have.class", "ui5-button-root"); - cy.get("#carouselF7Input").realClick(); - cy.realPress("F7"); - cy.focused().should("have.class", "ui5-carousel-root"); + it("'Home' and 'End' button press", () => { + cy.mount( + + + + + + + + + + + + ); - cy.realPress("F7"); - cy.focused().should("have.class", "ui5-input-inner"); + cy.get("#testHomeAndEnd").shadow().find(".ui5-carousel-content").find(".ui5-carousel-item").first().focus(); + cy.realPress("End"); + cy.get("#testHomeAndEnd").should("have.prop", "_selectedIndex", 9); + cy.realPress("Home"); + cy.get("#testHomeAndEnd").should("have.prop", "_selectedIndex", 0); + }); - cy.get("#carouselF7Button").realClick(); - cy.realPress("F7"); + it("'PageUp' and 'PageDown' button press", () => { + cy.mount( + + + + + + + + + + + + + + + + + + + + + + + + ); - cy.get("#carouselF7").then(($carousel) => { - $carousel[0].navigateTo(1); - }); - cy.realPress("F7"); - cy.focused().should("have.class", "ui5-input-inner"); + cy.get("#firstButton").realClick(); + cy.get("#testPageUpDown").should("have.prop", "_selectedIndex", 0); + cy.realPress("PageUp"); + cy.get("#testPageUpDown").should("have.prop", "_selectedIndex", 10); + cy.realPress("PageUp"); + cy.get("#testPageUpDown").should("have.prop", "_selectedIndex", 20); + cy.realPress("PageUp"); + cy.get("#testPageUpDown").should("have.prop", "_selectedIndex", 21); + cy.realPress("PageDown"); + cy.get("#testPageUpDown").should("have.prop", "_selectedIndex", 11); + cy.realPress("PageDown"); + cy.get("#testPageUpDown").should("have.prop", "_selectedIndex", 1); + cy.realPress("PageDown"); + cy.get("#testPageUpDown").should("have.prop", "_selectedIndex", 0); }); it("Items per page", () => { diff --git a/packages/main/src/Carousel.ts b/packages/main/src/Carousel.ts index 01d66d2fa640..d47054898cf0 100644 --- a/packages/main/src/Carousel.ts +++ b/packages/main/src/Carousel.ts @@ -10,6 +10,10 @@ import { isDown, isUp, isF7, + isHome, + isEnd, + isPageDown, + isPageUp, } from "@ui5/webcomponents-base/dist/Keys.js"; import type { UI5CustomEvent } from "@ui5/webcomponents-base"; import i18n from "@ui5/webcomponents-base/dist/decorators/i18n.js"; @@ -18,7 +22,6 @@ import ScrollEnablement from "@ui5/webcomponents-base/dist/delegate/ScrollEnable import type { ScrollEnablementEventListenerParam } from "@ui5/webcomponents-base/dist/delegate/ScrollEnablement.js"; import ResizeHandler from "@ui5/webcomponents-base/dist/delegate/ResizeHandler.js"; import type { ResizeObserverCallback } from "@ui5/webcomponents-base/dist/delegate/ResizeHandler.js"; -import { renderFinished } from "@ui5/webcomponents-base/dist/Render.js"; import { isDesktop } from "@ui5/webcomponents-base/dist/Device.js"; import AnimationMode from "@ui5/webcomponents-base/dist/types/AnimationMode.js"; import { getAnimationMode } from "@ui5/webcomponents-base/dist/config/AnimationMode.js"; @@ -35,8 +38,8 @@ import CarouselPageIndicatorType from "./types/CarouselPageIndicatorType.js"; import type BackgroundDesign from "./types/BackgroundDesign.js"; import type BorderDesign from "./types/BorderDesign.js"; import CarouselTemplate from "./CarouselTemplate.js"; +import { getFirstFocusableElement } from "@ui5/webcomponents-base/dist/util/FocusableElements.js"; -import type Button from "./Button.js"; // Styles import CarouselCss from "./generated/themes/Carousel.css.js"; @@ -277,6 +280,8 @@ class Carousel extends UI5Element { _resizing: boolean; _lastFocusedElements: Array; _orderOfLastFocusedPages: Array; + _lastInnerFocusedElement?: HTMLElement; + _pageStep: number = 10; _visibleItemsIndexes: Array; _calculatedX: number = 0; _itemIndicator: number = 0; @@ -384,10 +389,24 @@ class Carousel extends UI5Element { this._handleF7Key(e); return; } + if (isHome(e)) { + this._handleHome(e); + } + if (isEnd(e)) { + this._handleEnd(e); + } + if (isPageUp(e)) { + this._handlePageUp(e); + } + if (isPageDown(e)) { + this._handlePageDown(e); + } if (isLeft(e) || isDown(e)) { + e.preventDefault(); this.navigateLeft(); } else if (isRight(e) || isUp(e)) { + e.preventDefault(); this.navigateRight(); } } @@ -435,17 +454,40 @@ class Carousel extends UI5Element { } } - _handleF7Key(e: KeyboardEvent) { + async _handleF7Key(e: KeyboardEvent) { const lastFocusedElement = this._lastFocusedElements[this._getLastFocusedActivePageIndex]; - - if (e.target === this.getDomRef() && lastFocusedElement) { + if(!this._lastInnerFocusedElement) { + const firstFocusable = await getFirstFocusableElement(this.items[this._selectedIndex].item); + firstFocusable?.focus() + this._lastInnerFocusedElement = firstFocusable || undefined; + } else if (this.carouselItemDomRef(this._selectedIndex)[0] === lastFocusedElement && lastFocusedElement !== e.target) { lastFocusedElement.focus(); - } else { - //@TODO where shoul the focus go? - this.getDomRef()!.focus(); + this._lastInnerFocusedElement = e.target as HTMLElement; + } else if(this._lastInnerFocusedElement) { + this._lastInnerFocusedElement.focus(); } } + _handleHome(e: KeyboardEvent) { + e.preventDefault(); + this.navigateTo(0); + } + + _handleEnd(e: KeyboardEvent) { + e.preventDefault(); + this.navigateTo(this.items.length - 1); + } + + _handlePageUp(e: KeyboardEvent) { + e.preventDefault(); + this.navigateTo(this._selectedIndex + this._pageStep < this.items.length ? this._selectedIndex + this._pageStep : this.items.length - 1); + } + + _handlePageDown(e: KeyboardEvent) { + e.preventDefault(); + this.navigateTo(this._selectedIndex - this._pageStep > 0 ? this._selectedIndex - this._pageStep : 0); + } + get _backgroundDesign() { return this.backgroundDesign.toLowerCase(); } @@ -468,9 +510,7 @@ class Carousel extends UI5Element { const previousSelectedIndex = this._selectedIndex; if (this._selectedIndex - 1 < 0) { - // TODO clean up logic, no cyclic for morre than pne page - if (this.cyclic) { - // make better + if (this.cyclic && this._visibleItemsIndexes.length >= 1) { if (this._selectedIndex === 0 && this.effectiveItemsPerPage > 1) { this._selectedIndex = 0; } else { @@ -493,7 +533,6 @@ class Carousel extends UI5Element { const previousSelectedIndex = this._selectedIndex; if (this._selectedIndex + 1 > this.items.length - 1) { - // TODO clean up logic, no cyclic for mоre than оne page if (this.cyclic) { if (this._selectedIndex === this.items.length - 1 && this.effectiveItemsPerPage > 1) { this._selectedIndex = this.items.length - 1; @@ -507,7 +546,6 @@ class Carousel extends UI5Element { ++this._selectedIndex; } - //TOOD fix fast clicking of arrow left breaks animation if (previousSelectedIndex !== this._selectedIndex) { this.skipToItem(this._selectedIndex, 1); this.fireDecoratorEvent("navigate", { selectedIndex: this._selectedIndex }); @@ -515,6 +553,9 @@ class Carousel extends UI5Element { } _calculateItemSlideIndex(currentSlideIndex: number, itemStep: number) { + if (this.isItemInViewport(this._selectedIndex)) { + return 0; + } const itemsPerPage = this.effectiveItemsPerPage; let slideIndex; @@ -566,12 +607,12 @@ class Carousel extends UI5Element { } focusItem() { - this.carouselItemDomRef(this._selectedIndex)[0].focus(); + this.carouselItemDomRef(this._selectedIndex)[0].focus({ preventScroll: true }); } - _navButtonClick(e: UI5CustomEvent) { - const button = e.target as Button; - if (button.hasAttribute("data-ui5-arrow-forward")) { + _navButtonClick(e: MouseEvent) { + const target = e.currentTarget as HTMLElement; + if (target.hasAttribute("data-ui5-arrow-forward")) { this.navigateRight(); } else { this.navigateLeft(); @@ -585,22 +626,30 @@ class Carousel extends UI5Element { * @public */ navigateTo(itemIndex: number) { - //TODO test this and fix this._resizing = false; - //TODO check if _SelectedIndex is grater or smaller than itemIndex for skipToItemOffset - this._selectedIndex = itemIndex; - this._currentSlideIndex = itemIndex; + if (this._selectedIndex < itemIndex) { + this._itemIndicator = 1; + } + this._selectedIndex = itemIndex; + this._currentSlideIndex = itemIndex - this._itemIndicator; + if (this.isItemInViewport(itemIndex)) { + this._currentSlideIndex = this._visibleItemsIndexes[0]; + this._resizing = false; + this.focusItem(); + return; + } this.skipToItem(this._selectedIndex, 1); } - async skipToItem(focusIndex: number, offset: number) { + + skipToItem(focusIndex: number, offset: number) { if (!this.isItemInViewport(focusIndex)) { - const slideIndex = this._calculateItemSlideIndex(this._currentSlideIndex, offset); - console.log(" SLIDE INDEX ", slideIndex); + let slideIndex = this._calculateItemSlideIndex(this._currentSlideIndex, offset); + if (focusIndex === 0) { + slideIndex = 0; + } this._moveToItem(slideIndex); } - - await renderFinished(); this.focusItem(); } /** @@ -612,7 +661,7 @@ class Carousel extends UI5Element { return { id: `${this._id}-carousel-item-${idx + 1}`, item, - tabIndex: this._selectedIndex === idx ? 0 : -1, + tabIndex: this.isItemInViewport(this._selectedIndex) ? 0 : -1, posinset: idx + 1, setsize: this.content.length, visible: this.isItemInViewport(idx), @@ -680,7 +729,6 @@ class Carousel extends UI5Element { for (let i = newItemIndex; i < lastItemIndex; i++) { this._visibleItemsIndexes.push(i); } - console.log(" VISIBLE ", this._visibleItemsIndexes); } isIndexInRange(index: number): boolean { diff --git a/packages/main/src/CarouselTemplate.tsx b/packages/main/src/CarouselTemplate.tsx index a4e160deb3ef..2d38a6ad6a86 100644 --- a/packages/main/src/CarouselTemplate.tsx +++ b/packages/main/src/CarouselTemplate.tsx @@ -1,5 +1,5 @@ import type Carousel from "./Carousel.js"; -import Button from "./Button.js"; +import Icon from "./Icon.js"; import slimArrowLeft from "@ui5/webcomponents-icons/dist/slim-arrow-left.js"; import slimArrowRight from "@ui5/webcomponents-icons/dist/slim-arrow-right.js"; @@ -52,11 +52,9 @@ export default function CarouselTemplate(this: Carousel) { {this.renderNavigation &&
{this.showArrows.navigation && arrowBack.call(this)} -
{ !this.hidePageIndicator && navIndicator.call(this) }
- {this.showArrows.navigation && arrowForward.call(this)}
} @@ -65,38 +63,48 @@ export default function CarouselTemplate(this: Carousel) { } function arrowBack(this: Carousel) { - return ); - cy.get("#carousel1").should("have.prop", "_selectedIndex", 0); + cy.get("#carousel1").should("have.prop", "_focusedItemIndex", 0); cy.get("#carousel1") .trigger("mouseover") @@ -65,7 +65,7 @@ describe("Carousel general interaction", () => { .find(".ui5-carousel-navigation-arrows .ui5-carousel-navigation-button:not(.ui5-carousel-navigation-button--hidden)").last() .realClick(); - cy.get("#carousel1").should("have.prop", "_selectedIndex", 1); + cy.get("#carousel1").should("have.prop", "_focusedItemIndex", 1); }); it("Navigation is rendered for carousel with less than 9 elements", () => { @@ -471,7 +471,7 @@ describe("Carousel general interaction", () => { it("'Home' and 'End' button press", () => { cy.mount( - + @@ -483,11 +483,11 @@ describe("Carousel general interaction", () => { ); - cy.get("#testHomeAndEnd").shadow().find(".ui5-carousel-content").find(".ui5-carousel-item").first().focus(); + cy.get("#firstButton").realClick(); cy.realPress("End"); - cy.get("#testHomeAndEnd").should("have.prop", "_selectedIndex", 9); + cy.get("#testHomeAndEnd").should("have.prop", "_focusedItemIndex", 9); cy.realPress("Home"); - cy.get("#testHomeAndEnd").should("have.prop", "_selectedIndex", 0); + cy.get("#testHomeAndEnd").should("have.prop", "_focusedItemIndex", 0); }); it("'PageUp' and 'PageDown' button press", () => { @@ -518,19 +518,19 @@ describe("Carousel general interaction", () => { ); cy.get("#firstButton").realClick(); - cy.get("#testPageUpDown").should("have.prop", "_selectedIndex", 0); + cy.get("#testPageUpDown").should("have.prop", "_focusedItemIndex", 0); cy.realPress("PageUp"); - cy.get("#testPageUpDown").should("have.prop", "_selectedIndex", 10); + cy.get("#testPageUpDown").should("have.prop", "_focusedItemIndex", 10); cy.realPress("PageUp"); - cy.get("#testPageUpDown").should("have.prop", "_selectedIndex", 20); + cy.get("#testPageUpDown").should("have.prop", "_focusedItemIndex", 20); cy.realPress("PageUp"); - cy.get("#testPageUpDown").should("have.prop", "_selectedIndex", 21); + cy.get("#testPageUpDown").should("have.prop", "_focusedItemIndex", 21); cy.realPress("PageDown"); - cy.get("#testPageUpDown").should("have.prop", "_selectedIndex", 11); + cy.get("#testPageUpDown").should("have.prop", "_focusedItemIndex", 11); cy.realPress("PageDown"); - cy.get("#testPageUpDown").should("have.prop", "_selectedIndex", 1); + cy.get("#testPageUpDown").should("have.prop", "_focusedItemIndex", 1); cy.realPress("PageDown"); - cy.get("#testPageUpDown").should("have.prop", "_selectedIndex", 0); + cy.get("#testPageUpDown").should("have.prop", "_focusedItemIndex", 0); }); it("Items per page", () => { diff --git a/packages/main/src/Carousel.ts b/packages/main/src/Carousel.ts index 5d9b78c85b49..207b1de3479c 100644 --- a/packages/main/src/Carousel.ts +++ b/packages/main/src/Carousel.ts @@ -39,6 +39,7 @@ import type BackgroundDesign from "./types/BackgroundDesign.js"; import type BorderDesign from "./types/BorderDesign.js"; import CarouselTemplate from "./CarouselTemplate.js"; import { getFirstFocusableElement } from "@ui5/webcomponents-base/dist/util/FocusableElements.js"; +import clamp from "@ui5/webcomponents-base/dist/util/clamp.js"; // Styles import CarouselCss from "./generated/themes/Carousel.css.js"; @@ -189,8 +190,8 @@ class Carousel extends UI5Element { * @default 0 * @public */ - @property({ type: Number }) - _firstVisibleItemIndex = 0; + @property({ type: Number, noAttribute: true }) + _currentSlideIndex: number = 0; /** * Defines the visibility of the page indicator. @@ -247,8 +248,8 @@ class Carousel extends UI5Element { * @default 0 * @private */ - @property({ type: Number }) - _selectedIndex = 0; + @property({ type: Number, noAttribute: true }) + _focusedItemIndex = 0; /** * Defines the position of arrows. @@ -293,9 +294,7 @@ class Carousel extends UI5Element { _lastInnerFocusedElement?: HTMLElement; _pageStep: number = 10; _visibleItemsIndexes: Array; - _calculatedX: number = 0; _itemIndicator: number = 0; - _currentSlideIndex: number = 0; /** * Defines the content of the component. @@ -344,9 +343,6 @@ class Carousel extends UI5Element { if (isDesktop()) { this.setAttribute("desktop", ""); } - this._width = this.offsetWidth; - this._updateVisibleItems(this._selectedIndex); - this._currentSlideIndex = Math.min(this._selectedIndex, this.pagesCount - 1); } onExitDOM() { @@ -354,8 +350,8 @@ class Carousel extends UI5Element { } validateSelectedIndex() { - if (!this.isIndexInRange(this._selectedIndex)) { - this._selectedIndex = 0; + if (!this.isIndexInRange(this._focusedItemIndex)) { + this._focusedItemIndex = 0; } } @@ -368,7 +364,7 @@ class Carousel extends UI5Element { // Change transitively effectiveItemsPerPage by modifying _width this._width = this.offsetWidth; this._itemWidth = Math.floor(this._width / this.effectiveItemsPerPage); - this._updateVisibleItems(this._selectedIndex); + this._updateVisibleItems(this._focusedItemIndex); // Items per page did not change or the current, // therefore page index does not need to be re-adjusted @@ -376,10 +372,7 @@ class Carousel extends UI5Element { return; } - if (this._selectedIndex > this.items.length - 1) { - this._selectedIndex = this.items.length - 1; - this.fireDecoratorEvent("navigate", { selectedIndex: this._selectedIndex }); - } + this._focusedItemIndex = clamp(this._focusedItemIndex, this._currentSlideIndex, this.items.length - this.effectiveItemsPerPage); } _updateScrolling(e: ScrollEnablementEventListenerParam) { @@ -440,7 +433,7 @@ class Carousel extends UI5Element { return; } - this._selectedIndex = pageIndex; + this._focusedItemIndex = pageIndex; // Save reference of the last focused element for each page this._lastFocusedElements[pageIndex] = target; @@ -467,10 +460,10 @@ class Carousel extends UI5Element { async _handleF7Key(e: KeyboardEvent) { const lastFocusedElement = this._lastFocusedElements[this._getLastFocusedActivePageIndex]; if (!this._lastInnerFocusedElement) { - const firstFocusable = await getFirstFocusableElement(this.items[this._selectedIndex].item); + const firstFocusable = await getFirstFocusableElement(this.items[this._focusedItemIndex].item); firstFocusable?.focus(); this._lastInnerFocusedElement = firstFocusable || undefined; - } else if (this.carouselItemDomRef(this._selectedIndex)[0] === lastFocusedElement && lastFocusedElement !== e.target) { + } else if (this.carouselItemDomRef(this._focusedItemIndex)[0] === lastFocusedElement && lastFocusedElement !== e.target) { lastFocusedElement.focus(); this._lastInnerFocusedElement = e.target as HTMLElement; } else if (this._lastInnerFocusedElement) { @@ -490,12 +483,12 @@ class Carousel extends UI5Element { _handlePageUp(e: KeyboardEvent) { e.preventDefault(); - this.navigateTo(this._selectedIndex + this._pageStep < this.items.length ? this._selectedIndex + this._pageStep : this.items.length - 1); + this.navigateTo(this._focusedItemIndex + this._pageStep < this.items.length ? this._focusedItemIndex + this._pageStep : this.items.length - 1); } _handlePageDown(e: KeyboardEvent) { e.preventDefault(); - this.navigateTo(this._selectedIndex - this._pageStep > 0 ? this._selectedIndex - this._pageStep : 0); + this.navigateTo(this._focusedItemIndex - this._pageStep > 0 ? this._focusedItemIndex - this._pageStep : 0); } get _backgroundDesign() { @@ -511,79 +504,79 @@ class Carousel extends UI5Element { } } - return this._selectedIndex; + return this._focusedItemIndex; } navigateLeft() { this._resizing = false; - const previousSelectedIndex = this._selectedIndex; + const previousSelectedIndex = this._focusedItemIndex; - if (this._selectedIndex - 1 < 0) { + if (this._focusedItemIndex - 1 < 0) { if (this.cyclic && this._visibleItemsIndexes.length >= 1) { - if (this._selectedIndex === 0 && this.effectiveItemsPerPage > 1) { - this._selectedIndex = 0; + if (this._focusedItemIndex === 0 && this.effectiveItemsPerPage > 1) { + this._focusedItemIndex = 0; } else { - this._selectedIndex = this.items.length - 1; + this._focusedItemIndex = this.items.length - 1; } } } else { - --this._selectedIndex; + --this._focusedItemIndex; } - if (previousSelectedIndex !== this._selectedIndex) { - this.skipToItem(this._selectedIndex, -1); - this.fireDecoratorEvent("navigate", { selectedIndex: this._selectedIndex }); + if (previousSelectedIndex !== this._focusedItemIndex) { + this.skipToItem(this._focusedItemIndex, -1); + this.fireDecoratorEvent("navigate", { selectedIndex: this._focusedItemIndex }); } } navigateRight() { this._resizing = false; - const previousSelectedIndex = this._selectedIndex; + const previousSelectedIndex = this._focusedItemIndex; - if (this._selectedIndex + 1 > this.items.length - 1) { + if (this._focusedItemIndex + 1 > this.items.length - 1) { if (this.cyclic) { - if (this._selectedIndex === this.items.length - 1 && this.effectiveItemsPerPage > 1) { - this._selectedIndex = this.items.length - 1; + if (this._focusedItemIndex === this.items.length - 1 && this.effectiveItemsPerPage > 1) { + this._focusedItemIndex = this.items.length - 1; } else { - this._selectedIndex = 0; + this._focusedItemIndex = 0; } } else { return; } } else { - ++this._selectedIndex; + ++this._focusedItemIndex; } - if (previousSelectedIndex !== this._selectedIndex) { - this.skipToItem(this._selectedIndex, 1); - this.fireDecoratorEvent("navigate", { selectedIndex: this._selectedIndex }); + if (previousSelectedIndex !== this._focusedItemIndex) { + this.skipToItem(this._focusedItemIndex, 1); + this.fireDecoratorEvent("navigate", { selectedIndex: this._focusedItemIndex }); } } navigateArrowRight() { - if (this._selectedIndex === this._visibleItemsIndexes[0]) { - this.navigateTo(this._selectedIndex + 1); + if (this._focusedItemIndex === this._visibleItemsIndexes[0]) { + this.navigateTo(this._focusedItemIndex + 1); this._moveToItem(this._currentSlideIndex + 1); } else { this._moveToItem(this._currentSlideIndex + 1); - this.navigateTo(this._selectedIndex); + this.navigateTo(this._focusedItemIndex); } } navigateArrowLeft() { - if (this._selectedIndex > 0 && this._selectedIndex === this._visibleItemsIndexes[this._visibleItemsIndexes.length - 1]) { - this.navigateTo(this._selectedIndex - 1); + if (this._focusedItemIndex > 0 && this._focusedItemIndex === this._visibleItemsIndexes[this._visibleItemsIndexes.length - 1]) { + this.navigateTo(this._focusedItemIndex - 1); this._moveToItem(this._currentSlideIndex - 1); } else { this._moveToItem(this._currentSlideIndex === 0 ? this.pagesCount - 1 : this._currentSlideIndex - 1); - this.navigateTo(this._selectedIndex === 0 ? this.items.length - 1 : this._selectedIndex); + this.navigateTo(this._focusedItemIndex === 0 ? this.items.length - 1 : this._focusedItemIndex); } } _calculateItemSlideIndex(currentSlideIndex: number, itemStep: number) { - if (this.isItemInViewport(this._selectedIndex)) { + if (this.isItemInViewport(this._focusedItemIndex)) { return 0; } const itemsPerPage = this.effectiveItemsPerPage; @@ -637,7 +630,7 @@ class Carousel extends UI5Element { } focusItem() { - this.carouselItemDomRef(this._selectedIndex)[0].focus({ preventScroll: true }); + this.carouselItemDomRef(this._focusedItemIndex)[0].focus({ preventScroll: true }); } _navButtonClick(e: MouseEvent) { @@ -645,10 +638,8 @@ class Carousel extends UI5Element { if (this._visibleItemsIndexes.length > 1) { if (target.hasAttribute("data-ui5-arrow-forward")) { this.navigateArrowRight(); - this._firstVisibleItemIndex += 1; } else { this.navigateArrowLeft(); - this._firstVisibleItemIndex -= 1; } } else if (this._visibleItemsIndexes.length <= 1) { if (target.hasAttribute("data-ui5-arrow-forward")) { @@ -666,17 +657,17 @@ class Carousel extends UI5Element { * @public */ navigateTo(itemIndex: number) { - if (this._selectedIndex < itemIndex) { + if (this._focusedItemIndex < itemIndex) { this._itemIndicator = 1; } - this._selectedIndex = itemIndex; + this._focusedItemIndex = itemIndex; this._currentSlideIndex = itemIndex - this._itemIndicator; if (this.isItemInViewport(itemIndex)) { this._currentSlideIndex = this._visibleItemsIndexes[0]; this.focusItem(); return; } - this.skipToItem(this._selectedIndex, 1); + this.skipToItem(this._focusedItemIndex, 1); } async skipToItem(focusIndex: number, offset: number) { @@ -702,7 +693,7 @@ class Carousel extends UI5Element { return { id: `${this._id}-carousel-item-${idx + 1}`, item, - tabIndex: this.isItemInViewport(this._selectedIndex) ? 0 : -1, + tabIndex: this.isItemInViewport(this._focusedItemIndex) ? 0 : -1, posinset: idx + 1, setsize: this.content.length, visible: this.isItemInViewport(idx), @@ -855,11 +846,11 @@ class Carousel extends UI5Element { } get hasPrev() { - return this.cyclic || (this._selectedIndex - 1 >= 0 && this._currentSlideIndex !== 0); + return this.cyclic || (this._focusedItemIndex - 1 >= 0 && this._currentSlideIndex !== 0); } get hasNext() { - return this.cyclic || (this._selectedIndex + 1 <= this.content.length - 1 && this._currentSlideIndex < this.pagesCount - 1); + return this.cyclic || (this._focusedItemIndex + 1 <= this.content.length - 1 && this._currentSlideIndex < this.pagesCount - 1); } get suppressAnimation() { @@ -871,7 +862,7 @@ class Carousel extends UI5Element { } get selectedIndexToShow() { - return this._isRTL ? this.items.length - (this.items.length - this._selectedIndex) + 1 : this._selectedIndex + 1; + return this._isRTL ? this.items.length - (this.items.length - this._focusedItemIndex) + 1 : this._focusedItemIndex + 1; } get ofText() { @@ -879,7 +870,7 @@ class Carousel extends UI5Element { } get ariaActiveDescendant() { - return this.content.length ? `${this._id}-carousel-item-${this._selectedIndex + 1}` : undefined; + return this.content.length ? `${this._id}-carousel-item-${this._focusedItemIndex + 1}` : undefined; } get ariaLabelTxt() { diff --git a/packages/main/src/themes/Carousel.css b/packages/main/src/themes/Carousel.css index 290b0f88dd52..503f5eb02078 100644 --- a/packages/main/src/themes/Carousel.css +++ b/packages/main/src/themes/Carousel.css @@ -131,17 +131,12 @@ width: var(--ui5_carousel_button_size); height: var(--ui5_carousel_button_size); border-radius: 50%; - box-shadow: var(--sapContent_Shadow1); - cursor: pointer; - outline-offset: .1rem; - --_ui5-v2-15-0-rc-0_button_focused_border_radius: 50%; box-sizing: border-box; align-items: center; justify-content: center; display: flex; padding: 0 0.5rem; border: 1px solid var(--sapButton_Hover_BorderColor); - display: flex; background: var(--sapButton_Background); } From a0e4c46705dcdc19c658bd332c7e3716b40429e8 Mon Sep 17 00:00:00 2001 From: Konstantin Kondov Date: Tue, 7 Oct 2025 17:21:39 +0300 Subject: [PATCH 12/17] feat(ui5-carousel): improve keyboard handling and update accessibility attributes addressed review comments --- packages/main/src/Carousel.ts | 6 ++-- packages/main/src/CarouselTemplate.tsx | 35 +++++++------------ packages/main/src/themes/Carousel.css | 6 ---- .../src/themes/base/Carousel-parameters.css | 2 +- .../sap_fiori_3_hcb/Carousel-parameters.css | 2 +- .../sap_fiori_3_hcw/Carousel-parameters.css | 2 +- .../sap_horizon_hcb/Carousel-parameters.css | 2 +- .../sap_horizon_hcw/Carousel-parameters.css | 2 +- 8 files changed, 21 insertions(+), 36 deletions(-) diff --git a/packages/main/src/Carousel.ts b/packages/main/src/Carousel.ts index 207b1de3479c..0a48cbcf4022 100644 --- a/packages/main/src/Carousel.ts +++ b/packages/main/src/Carousel.ts @@ -16,6 +16,7 @@ import { isPageDown, isPageUp, } from "@ui5/webcomponents-base/dist/Keys.js"; +import type { UI5CustomEvent } from "@ui5/webcomponents-base"; import i18n from "@ui5/webcomponents-base/dist/decorators/i18n.js"; import type I18nBundle from "@ui5/webcomponents-base/dist/i18nBundle.js"; import ScrollEnablement from "@ui5/webcomponents-base/dist/delegate/ScrollEnablement.js"; @@ -38,6 +39,7 @@ import CarouselPageIndicatorType from "./types/CarouselPageIndicatorType.js"; import type BackgroundDesign from "./types/BackgroundDesign.js"; import type BorderDesign from "./types/BorderDesign.js"; import CarouselTemplate from "./CarouselTemplate.js"; +import type Icon from "./Icon.js"; import { getFirstFocusableElement } from "@ui5/webcomponents-base/dist/util/FocusableElements.js"; import clamp from "@ui5/webcomponents-base/dist/util/clamp.js"; @@ -633,8 +635,8 @@ class Carousel extends UI5Element { this.carouselItemDomRef(this._focusedItemIndex)[0].focus({ preventScroll: true }); } - _navButtonClick(e: MouseEvent) { - const target = e.currentTarget as HTMLElement; + _navButtonClick(e: UI5CustomEvent) { + const target = e.target as Icon; if (this._visibleItemsIndexes.length > 1) { if (target.hasAttribute("data-ui5-arrow-forward")) { this.navigateArrowRight(); diff --git a/packages/main/src/CarouselTemplate.tsx b/packages/main/src/CarouselTemplate.tsx index c92ea5ae8fa9..579c1a8a0dfb 100644 --- a/packages/main/src/CarouselTemplate.tsx +++ b/packages/main/src/CarouselTemplate.tsx @@ -62,42 +62,31 @@ export default function CarouselTemplate(this: Carousel) { } function arrowBack(this: Carousel) { - return ; + onClick={this._navButtonClick} + />; } function arrowForward(this: Carousel) { - return ; + onClick={this._navButtonClick} + />; } function navIndicator(this: Carousel) { diff --git a/packages/main/src/themes/Carousel.css b/packages/main/src/themes/Carousel.css index 503f5eb02078..a287989e270b 100644 --- a/packages/main/src/themes/Carousel.css +++ b/packages/main/src/themes/Carousel.css @@ -138,12 +138,7 @@ padding: 0 0.5rem; border: 1px solid var(--sapButton_Hover_BorderColor); background: var(--sapButton_Background); -} - -.ui5-carousel-navigation-button { pointer-events: all; -} -.ui5-carousel-navigation-button [ui5-icon] { color: var(--sapButton_TextColor); } @@ -159,7 +154,6 @@ outline-offset: 1px; color: var(--sapButton_Active_TextColor); box-shadow: none; - } .ui5-carousel-navigation-arrows .ui5-carousel-navigation-button { diff --git a/packages/main/src/themes/base/Carousel-parameters.css b/packages/main/src/themes/base/Carousel-parameters.css index 4c6a3a9c2e4a..164ba83ddfce 100644 --- a/packages/main/src/themes/base/Carousel-parameters.css +++ b/packages/main/src/themes/base/Carousel-parameters.css @@ -1,7 +1,7 @@ :root { --ui5_carousel_background_color_solid: var(--sapGroup_ContentBackground); --ui5_carousel_background_color_translucent: var(--sapBackgroundColor); - --ui5_carousel_button_size: 2.5rem; + --ui5_carousel_button_size: 2.25rem; --ui5_carousel_inactive_dot_size: 0.25rem; --ui5_carousel_inactive_dot_margin: 0 0.375rem; --ui5_carousel_inactive_dot_border: 1px solid var(--sapContent_ForegroundBorderColor); diff --git a/packages/main/src/themes/sap_fiori_3_hcb/Carousel-parameters.css b/packages/main/src/themes/sap_fiori_3_hcb/Carousel-parameters.css index 8ac76606d1d8..3091ab0854bf 100644 --- a/packages/main/src/themes/sap_fiori_3_hcb/Carousel-parameters.css +++ b/packages/main/src/themes/sap_fiori_3_hcb/Carousel-parameters.css @@ -1,5 +1,5 @@ :root { - --ui5_carousel_button_size: 2.5rem; + --ui5_carousel_button_size: 2.25rem; --ui5_carousel_inactive_dot_size: 0.5rem; --ui5_carousel_inactive_dot_margin: 0 0.25rem; --ui5_carousel_inactive_dot_border: 1px solid var(--sapContent_ForegroundBorderColor); diff --git a/packages/main/src/themes/sap_fiori_3_hcw/Carousel-parameters.css b/packages/main/src/themes/sap_fiori_3_hcw/Carousel-parameters.css index 8ac76606d1d8..3091ab0854bf 100644 --- a/packages/main/src/themes/sap_fiori_3_hcw/Carousel-parameters.css +++ b/packages/main/src/themes/sap_fiori_3_hcw/Carousel-parameters.css @@ -1,5 +1,5 @@ :root { - --ui5_carousel_button_size: 2.5rem; + --ui5_carousel_button_size: 2.25rem; --ui5_carousel_inactive_dot_size: 0.5rem; --ui5_carousel_inactive_dot_margin: 0 0.25rem; --ui5_carousel_inactive_dot_border: 1px solid var(--sapContent_ForegroundBorderColor); diff --git a/packages/main/src/themes/sap_horizon_hcb/Carousel-parameters.css b/packages/main/src/themes/sap_horizon_hcb/Carousel-parameters.css index 71d68de9aeb5..4aa8dac41480 100644 --- a/packages/main/src/themes/sap_horizon_hcb/Carousel-parameters.css +++ b/packages/main/src/themes/sap_horizon_hcb/Carousel-parameters.css @@ -1,5 +1,5 @@ :root { - --ui5_carousel_button_size: 2.5rem; + --ui5_carousel_button_size: 2.25rem; --ui5_carousel_inactive_dot_size: 0.5rem; --ui5_carousel_inactive_dot_margin: 0 0.25rem; --ui5_carousel_inactive_dot_border: 1px solid var(--sapContent_ForegroundBorderColor); diff --git a/packages/main/src/themes/sap_horizon_hcw/Carousel-parameters.css b/packages/main/src/themes/sap_horizon_hcw/Carousel-parameters.css index 8ac76606d1d8..3091ab0854bf 100644 --- a/packages/main/src/themes/sap_horizon_hcw/Carousel-parameters.css +++ b/packages/main/src/themes/sap_horizon_hcw/Carousel-parameters.css @@ -1,5 +1,5 @@ :root { - --ui5_carousel_button_size: 2.5rem; + --ui5_carousel_button_size: 2.25rem; --ui5_carousel_inactive_dot_size: 0.5rem; --ui5_carousel_inactive_dot_margin: 0 0.25rem; --ui5_carousel_inactive_dot_border: 1px solid var(--sapContent_ForegroundBorderColor); From d812deeb042d342bd93d3238e5fcce6df3a7d79e Mon Sep 17 00:00:00 2001 From: Konstantin Kondov Date: Wed, 8 Oct 2025 10:50:29 +0300 Subject: [PATCH 13/17] feat(ui5-carousel): improve keyboard handling and update accessibility attributes updates arrow buttons handling --- packages/main/cypress/specs/Carousel.cy.tsx | 13 +++++++++---- packages/main/src/Carousel.ts | 4 ++-- packages/main/src/CarouselTemplate.tsx | 4 ++-- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/packages/main/cypress/specs/Carousel.cy.tsx b/packages/main/cypress/specs/Carousel.cy.tsx index bc55f309590b..84a65384b3c6 100644 --- a/packages/main/cypress/specs/Carousel.cy.tsx +++ b/packages/main/cypress/specs/Carousel.cy.tsx @@ -249,6 +249,11 @@ describe("Carousel general interaction", () => { cy.get("#carousel5") .shadow() .find(".ui5-carousel-root") + .should("have.attr", "role", "region"); + + cy.get("#carousel5") + .shadow() + .find(".ui5-carousel-content") .should("have.attr", "role", "list"); cy.get("#carousel5") @@ -323,7 +328,7 @@ describe("Carousel general interaction", () => { cy.get("#carousel8") .trigger("mouseover") .shadow() - .find("span.ui5-carousel-navigation-button[data-ui5-arrow-forward]") + .find(".ui5-carousel-navigation-button[data-ui5-arrow-forward]") .should("exist") .realClick(); cy.get("@myStub").should("have.been.calledOnce"); @@ -331,7 +336,7 @@ describe("Carousel general interaction", () => { cy.get("#carousel8") .trigger("mouseover") .shadow() - .find("span.ui5-carousel-navigation-button[data-ui5-arrow-forward]") + .find(".ui5-carousel-navigation-button[data-ui5-arrow-forward]") .should("exist") .realClick(); cy.get("@myStub").should("have.been.calledTwice"); @@ -339,7 +344,7 @@ describe("Carousel general interaction", () => { cy.get("#carousel8") .trigger("mouseover") .shadow() - .find("span.ui5-carousel-navigation-button[data-ui5-arrow-back]") + .find(".ui5-carousel-navigation-button[data-ui5-arrow-back]") .should("exist") .realClick(); cy.get("@myStub").should("have.been.calledThrice"); @@ -347,7 +352,7 @@ describe("Carousel general interaction", () => { cy.get("#carousel8") .trigger("mouseover") .shadow() - .find("span.ui5-carousel-navigation-button[data-ui5-arrow-back]") + .find(".ui5-carousel-navigation-button[data-ui5-arrow-back]") .should("exist") .realClick(); cy.get("@myStub").should("have.callCount", 4); diff --git a/packages/main/src/Carousel.ts b/packages/main/src/Carousel.ts index 0a48cbcf4022..4b77391157b5 100644 --- a/packages/main/src/Carousel.ts +++ b/packages/main/src/Carousel.ts @@ -407,10 +407,10 @@ class Carousel extends UI5Element { this._handlePageDown(e); } - if (isLeft(e) || isDown(e)) { + if (isLeft(e) || isUp(e)) { e.preventDefault(); this.navigateLeft(); - } else if (isRight(e) || isUp(e)) { + } else if (isRight(e) || isDown(e)) { e.preventDefault(); this.navigateRight(); } diff --git a/packages/main/src/CarouselTemplate.tsx b/packages/main/src/CarouselTemplate.tsx index 579c1a8a0dfb..ec15300c5710 100644 --- a/packages/main/src/CarouselTemplate.tsx +++ b/packages/main/src/CarouselTemplate.tsx @@ -10,7 +10,7 @@ export default function CarouselTemplate(this: Carousel) { "ui5-carousel-root": true, [`ui5-carousel-background-${this._backgroundDesign}`]: true, }} - role="list" + role="region" aria-label={this.ariaLabelTxt} aria-roledescription={this._roleDescription} onFocusIn={this._onfocusin} @@ -19,7 +19,7 @@ export default function CarouselTemplate(this: Carousel) { onMouseOver={this._onmouseover} >
-
+
{this.items.map(itemInfo =>
Date: Wed, 8 Oct 2025 11:09:34 +0300 Subject: [PATCH 14/17] feat(ui5-carousel): improve keyboard handling and update accessibility attributes properly update visible items on resize --- 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 4b77391157b5..9351e9a31820 100644 --- a/packages/main/src/Carousel.ts +++ b/packages/main/src/Carousel.ts @@ -366,7 +366,7 @@ class Carousel extends UI5Element { // Change transitively effectiveItemsPerPage by modifying _width this._width = this.offsetWidth; this._itemWidth = Math.floor(this._width / this.effectiveItemsPerPage); - this._updateVisibleItems(this._focusedItemIndex); + this._updateVisibleItems(this._currentSlideIndex); // Items per page did not change or the current, // therefore page index does not need to be re-adjusted From be40a01f2e4e6a0353173afd378c34d79fd27cbd Mon Sep 17 00:00:00 2001 From: Konstantin Kondov Date: Thu, 9 Oct 2025 12:36:05 +0300 Subject: [PATCH 15/17] feat(ui5-carousel): improve keyboard handling and update accessibility attributes fix focus flashing --- packages/main/src/Carousel.ts | 14 ++++++++++++++ packages/main/src/CarouselTemplate.tsx | 2 ++ 2 files changed, 16 insertions(+) diff --git a/packages/main/src/Carousel.ts b/packages/main/src/Carousel.ts index 9351e9a31820..a8826f0d1b69 100644 --- a/packages/main/src/Carousel.ts +++ b/packages/main/src/Carousel.ts @@ -459,6 +459,20 @@ class Carousel extends UI5Element { } } + _ontouchstart(e: TouchEvent) { + const target = e.target as HTMLElement; + if (target.hasAttribute("data-ui5-arrow-forward") || target.hasAttribute("data-ui5-arrow-back")) { + e.preventDefault(); + } + } + + _onmousedown(e: MouseEvent) { + const target = e.target as HTMLElement; + if (target.hasAttribute("data-ui5-arrow-forward") || target.hasAttribute("data-ui5-arrow-back")) { + e.preventDefault(); + } + } + async _handleF7Key(e: KeyboardEvent) { const lastFocusedElement = this._lastFocusedElements[this._getLastFocusedActivePageIndex]; if (!this._lastInnerFocusedElement) { diff --git a/packages/main/src/CarouselTemplate.tsx b/packages/main/src/CarouselTemplate.tsx index ec15300c5710..c518f2d6e8f1 100644 --- a/packages/main/src/CarouselTemplate.tsx +++ b/packages/main/src/CarouselTemplate.tsx @@ -17,6 +17,8 @@ export default function CarouselTemplate(this: Carousel) { onKeyDown={this._onkeydown} onMouseOut={this._onmouseout} onMouseOver={this._onmouseover} + onTouchStart={this._ontouchstart} + onMouseDown={this._onmousedown} >
From 2d2326acd721b5cc299b7f66be45bed4567f2303 Mon Sep 17 00:00:00 2001 From: Konstantin Kondov Date: Mon, 13 Oct 2025 15:38:52 +0300 Subject: [PATCH 16/17] feat(ui5-carousel): improve keyboard handling and update accessibility attributes address code review comments --- packages/main/src/themes/Carousel.css | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/packages/main/src/themes/Carousel.css b/packages/main/src/themes/Carousel.css index a287989e270b..bfb7042a608a 100644 --- a/packages/main/src/themes/Carousel.css +++ b/packages/main/src/themes/Carousel.css @@ -90,11 +90,6 @@ display: flex; justify-content: space-between; box-sizing: border-box; - pointer-events: none; -} - -.ui5-carousel-navigation-arrows > [ui5-button] { - pointer-events: all; } .ui5-carousel-navigation-wrapper { @@ -138,8 +133,8 @@ padding: 0 0.5rem; border: 1px solid var(--sapButton_Hover_BorderColor); background: var(--sapButton_Background); - pointer-events: all; color: var(--sapButton_TextColor); + cursor: pointer; } .ui5-carousel-navigation-button:hover { From 5be4766115f332c34a730f8b3a80b8ce4655a829 Mon Sep 17 00:00:00 2001 From: Konstantin Kondov Date: Mon, 13 Oct 2025 16:51:55 +0300 Subject: [PATCH 17/17] feat(ui5-carousel): improve keyboard handling and update accessibility attributes fix failing test --- packages/main/src/themes/Carousel.css | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/main/src/themes/Carousel.css b/packages/main/src/themes/Carousel.css index bfb7042a608a..84d46358f697 100644 --- a/packages/main/src/themes/Carousel.css +++ b/packages/main/src/themes/Carousel.css @@ -90,6 +90,7 @@ display: flex; justify-content: space-between; box-sizing: border-box; + pointer-events: none; } .ui5-carousel-navigation-wrapper { @@ -135,6 +136,7 @@ background: var(--sapButton_Background); color: var(--sapButton_TextColor); cursor: pointer; + pointer-events: all; } .ui5-carousel-navigation-button:hover {