fix(ui5-carousel): add all visible items to tab chain (#2530)

Fixes: #1996

Author: georgimkv

packages/main/src/Carousel.js

@@ -103,7 +103,7 @@ const metadata = {
 		},
 
 		/**
-		 * Defines when the <code>load-more</code> event is thrown. If not applied the event will not be thrown.
+		 * Defines when the <code>load-more</code> 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 <code>ui5-carousel</code> 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 <code>infiniteScrollOffset</code> property.
+		 * The number of items for which the event is fired is controlled by the <code>infiniteScrollOffset</code> 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",
 			};
 		});
 	}

packages/main/test/pages/Carousel.html

@@ -26,7 +26,7 @@
         }
     </style>
 
-    <ui5-carousel items-per-page-s="3" items-per-page-m="3" items-per-page-l="3">
+    <ui5-carousel id="carouselCards" items-per-page-s="3" items-per-page-m="3" items-per-page-l="3">
         <ui5-card id="card" status="1 of 11" heading="Quick Links" subheading="quick links" header-interactive
             class="myCard">
             <ui5-list id="myList3" separators="Inner">

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");
 	});
 });