From 37ee83ffecc861a2386b6502109e736b502a7c6a Mon Sep 17 00:00:00 2001 From: Georgi Date: Mon, 30 Nov 2020 16:04:07 +0200 Subject: [PATCH] fix(ui5-carousel): add all visible items to tab chain (#2530) Fixes: #1996 --- packages/main/src/Carousel.js | 9 ++++---- packages/main/test/pages/Carousel.html | 2 +- packages/main/test/specs/Carousel.spec.js | 25 ++++++++++++++++++----- 3 files changed, 26 insertions(+), 10 deletions(-) diff --git a/packages/main/src/Carousel.js b/packages/main/src/Carousel.js index 1df020143dd7..e0a4879ce8bb 100644 --- a/packages/main/src/Carousel.js +++ b/packages/main/src/Carousel.js @@ -103,7 +103,7 @@ const metadata = { }, /** - * Defines when the load-more event is thrown. If not applied the event will not be thrown. + * Defines when the load-more event is fired. If not applied the event will not be fired. * @type {Integer} * @defaultvalue 1 * @public @@ -185,7 +185,7 @@ const metadata = { /** * Fired for the last items of the ui5-carousel if it is scrolled and the direction of scrolling is to the end. - * The number of items for which the event is thrown is controlled by the infiniteScrollOffset property. + * The number of items for which the event is fired is controlled by the infiniteScrollOffset property. * @event sap.ui.webcomponents.main.Carousel#load-more * @public * @since 1.0.0-rc.8 @@ -377,14 +377,15 @@ class Carousel extends UI5Element { */ get items() { return this.content.map((item, idx) => { + const visible = this.isItemInViewport(idx); return { id: `${this._id}-carousel-item-${idx + 1}`, item, - tabIndex: idx === this.selectedIndex ? "0" : "-1", + tabIndex: visible ? "0" : "-1", posinset: idx + 1, setsize: this.content.length, width: this._itemWidth, - classes: this.isItemInViewport(idx) ? "" : "ui5-carousel-item--hidden", + classes: visible ? "" : "ui5-carousel-item--hidden", }; }); } diff --git a/packages/main/test/pages/Carousel.html b/packages/main/test/pages/Carousel.html index 65ae170b8f9d..d54814c7657f 100644 --- a/packages/main/test/pages/Carousel.html +++ b/packages/main/test/pages/Carousel.html @@ -26,7 +26,7 @@ } - + diff --git a/packages/main/test/specs/Carousel.spec.js b/packages/main/test/specs/Carousel.spec.js index 835397a3450e..046648bf86c3 100644 --- a/packages/main/test/specs/Carousel.spec.js +++ b/packages/main/test/specs/Carousel.spec.js @@ -65,7 +65,6 @@ describe("Carousel general interaction", () => { assert.strictEqual(pageIndicatorDot1.getAttribute("aria-label"), PAGE_INDICATOR_ARIA_LABEL1, "The aria-label of page indicator is correct."); assert.strictEqual(pageIndicatorDot2.getAttribute("aria-label"), PAGE_INDICATOR_ARIA_LABEL2, "The aria-label of page indicator is correct."); - const carouselItem3 = carousel.shadow$(".ui5-carousel-item:nth-child(3)"); const carouselItem4 = carousel.shadow$(".ui5-carousel-item:nth-child(4)"); const CAROUSEL_ITEM3_POS = "3"; @@ -90,13 +89,29 @@ describe("Carousel general interaction", () => { assert.strictEqual(carouselRoot.getAttribute("aria-activedescendant"), ACTIVEDESCENDANT_PAGE_2, "The aria-activedescendant of carousel is correct."); }); + it("all visible elements in the current page have correct tabindex values", () => { + const carousel = browser.$("#carouselCards"); + + const visibleItems = [ + carousel.shadow$(".ui5-carousel-item:nth-child(1) slot"), + carousel.shadow$(".ui5-carousel-item:nth-child(2) slot"), + carousel.shadow$(".ui5-carousel-item:nth-child(3) slot"), + ]; + + assert.strictEqual( + visibleItems.every(el => el.getAttribute("tabindex") === "0"), + true, + "all visible items have correct tabindex values" + ); + }); + it("Arrows and Dots not displayed in case of single page", () => { const carousel = browser.$("#carousel6"); const pages = carousel.getProperty("pagesCount"); const pageIndicator = carousel.shadow$(".ui5-carousel-navigation-wrapper"); const navigationArrows = carousel.shadow$(".ui5-carousel-navigation-arrows"); - assert.ok(!pageIndicator.isExisting(), "Page indicator is not srendered"); + assert.ok(!pageIndicator.isExisting(), "Page indicator is not rendered"); assert.ok(!navigationArrows.isExisting(), "Navigation arrows are not rendered"); assert.strictEqual(pages, 1, "There is only 1 page."); }); @@ -149,7 +164,7 @@ describe("Carousel general interaction", () => { assert.strictEqual(eventCounter.getProperty("value"), "6", "The navigate event is not fired as no previous item."); }); - it("loadMore event is thrown only when neccessary", () => { + it("loadMore event is fired only when neccessary", () => { const carousel = browser.$("#carousel9"); const eventCounter = browser.$("#loadmore-result"); const navigationArrowForward = carousel.shadow$("ui5-button[arrow-forward]"); @@ -161,12 +176,12 @@ describe("Carousel general interaction", () => { navigationArrowForward.click(); navigationArrowForward.click(); - assert.strictEqual(eventCounter.getProperty("value"), "0" , "loadMore event is not thrown"); + assert.strictEqual(eventCounter.getProperty("value"), "0" , "loadMore event is not fired"); navigationArrowForward.click(); navigationArrowForward.click(); navigationArrowForward.click(); - assert.strictEqual(eventCounter.getProperty("value"), "3", "loadMore event is thrown 3 times"); + assert.strictEqual(eventCounter.getProperty("value"), "3", "loadMore event is fired 3 times"); }); });