Skip to content

Commit

Permalink
fix(ui5-carousel): add all visible items to tab chain (#2530)
Browse files Browse the repository at this point in the history
Fixes: #1996
  • Loading branch information
georgimkv authored Nov 30, 2020
1 parent 455f614 commit 37ee83f
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 10 deletions.
9 changes: 5 additions & 4 deletions packages/main/src/Carousel.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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",
};
});
}
Expand Down
2 changes: 1 addition & 1 deletion packages/main/test/pages/Carousel.html
Original file line number Diff line number Diff line change
Expand Up @@ -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">
Expand Down
25 changes: 20 additions & 5 deletions packages/main/test/specs/Carousel.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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.");
});
Expand Down Expand Up @@ -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]");
Expand All @@ -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");
});
});

0 comments on commit 37ee83f

Please sign in to comment.