Skip to content

Commit 977c728

Browse files
TeodorTaushanovTeodorTaushanov
committed
fix: fix focus issue and tests
1 parent 6b5a550 commit 977c728

3 files changed

Lines changed: 85 additions & 21 deletions

File tree

‎packages/main/cypress/specs/Carousel.cy.tsx‎

Lines changed: 52 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -394,7 +394,7 @@ describe("Carousel general interaction", () => {
394394

395395
});
396396

397-
it("navigateTo method and visibleItemsIndices", () => {
397+
it("navigateTo and _changeFocusIndex methods", () => {
398398
cy.mount(
399399
<Carousel id="carousel9" itemsPerPage="S2 M2 L2 XL2" arrowsPlacement="Navigation">
400400
<Button>Button 1</Button>
@@ -413,11 +413,50 @@ describe("Carousel general interaction", () => {
413413
.invoke("prop", "visibleItemsIndices")
414414
.should("deep.equal", [0, 1]);
415415

416-
cy.get("#carousel9").shadow().find('[data-ui5-arrow-forward="true"]').realClick();
416+
cy.get<Carousel>("#carousel9")
417+
.then($carousel => {
418+
$carousel[0].navigateTo(2);
419+
});
420+
421+
cy.get("#carousel9")
422+
.invoke("prop", "visibleItemsIndices")
423+
.should("deep.equal", [2, 3]);
424+
425+
cy.get<Carousel>("#carousel9")
426+
.then($carousel => {
427+
$carousel[0].navigateTo(3);
428+
});
429+
430+
cy.get("#carousel9")
431+
.invoke("prop", "visibleItemsIndices")
432+
.should("deep.equal", [3, 4]);
433+
434+
cy.get<Carousel>("#carousel9")
435+
.then($carousel => {
436+
$carousel[0]._changeFocusIndex(4);
437+
});
438+
439+
cy.get("#carousel9")
440+
.invoke("prop", "visibleItemsIndices")
441+
.should("deep.equal", [3, 4]);
442+
443+
cy.get<Carousel>("#carousel9")
444+
.then($carousel => {
445+
$carousel[0]._changeFocusIndex(5);
446+
});
417447

418448
cy.get("#carousel9")
419449
.invoke("prop", "visibleItemsIndices")
420-
.should("deep.equal", [1, 2]);
450+
.should("deep.equal", [4, 5]);
451+
452+
cy.get<Carousel>("#carousel9")
453+
.then($carousel => {
454+
$carousel[0].navigateTo(3);
455+
});
456+
457+
cy.get("#carousel9")
458+
.invoke("prop", "visibleItemsIndices")
459+
.should("deep.equal", [3, 4]);
421460
});
422461

423462
it("F7 keyboard navigation", () => {
@@ -518,8 +557,10 @@ describe("Carousel general interaction", () => {
518557
cy.get("#firstButton").realClick();
519558
cy.realPress("End");
520559
cy.get("#testHomeAndEnd").should("have.prop", "_focusedItemIndex", 9);
560+
cy.get("#testHomeAndEnd").should("have.prop", "_currentSlideIndex", 9);
521561
cy.realPress("Home");
522562
cy.get("#testHomeAndEnd").should("have.prop", "_focusedItemIndex", 0);
563+
cy.get("#testHomeAndEnd").should("have.prop", "_currentSlideIndex", 0);
523564
});
524565

525566
it("'PageUp' and 'PageDown' button press", () => {
@@ -551,16 +592,22 @@ describe("Carousel general interaction", () => {
551592

552593
cy.get("#firstButton").realClick();
553594
cy.get("#testPageUpDown").should("have.prop", "_focusedItemIndex", 0);
595+
cy.get("#testPageUpDown").should("have.prop", "_currentSlideIndex", 0);
554596
cy.realPress("PageUp");
555597
cy.get("#testPageUpDown").should("have.prop", "_focusedItemIndex", 10);
598+
cy.get("#testPageUpDown").should("have.prop", "_currentSlideIndex", 10);
556599
cy.realPress("PageUp");
557600
cy.get("#testPageUpDown").should("have.prop", "_focusedItemIndex", 20);
601+
cy.get("#testPageUpDown").should("have.prop", "_currentSlideIndex", 19);
558602
cy.realPress("PageUp");
559603
cy.get("#testPageUpDown").should("have.prop", "_focusedItemIndex", 21);
604+
cy.get("#testPageUpDown").should("have.prop", "_currentSlideIndex", 19);
560605
cy.realPress("PageDown");
561-
cy.get("#testPageUpDown").should("have.prop", "_focusedItemIndex", 11);
606+
cy.get("#testPageUpDown").should("have.prop", "_focusedItemIndex", 9);
607+
cy.get("#testPageUpDown").should("have.prop", "_currentSlideIndex", 9);
562608
cy.realPress("PageDown");
563-
cy.get("#testPageUpDown").should("have.prop", "_focusedItemIndex", 1);
609+
cy.get("#testPageUpDown").should("have.prop", "_focusedItemIndex", 0);
610+
cy.get("#testPageUpDown").should("have.prop", "_currentSlideIndex", 0);
564611
cy.realPress("PageDown");
565612
cy.get("#testPageUpDown").should("have.prop", "_focusedItemIndex", 0);
566613
});

‎packages/main/src/Carousel.ts‎

Lines changed: 31 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -343,7 +343,7 @@ class Carousel extends UI5Element {
343343

344344
this._currentSlideIndex = clamp(this._currentSlideIndex, 0, Math.max(0, this.items.length - this.effectiveItemsPerPage));
345345
this._focusedItemIndex = clamp(this._focusedItemIndex, this._currentSlideIndex, this.items.length - 1);
346-
this.navigateTo(this._currentSlideIndex);
346+
this._changeSlideIndex(this._currentSlideIndex, false);
347347
});
348348

349349
this._scrollEnablement = new ScrollEnablement(this);
@@ -552,28 +552,28 @@ class Carousel extends UI5Element {
552552

553553
async _handleHome(e: KeyboardEvent) {
554554
e.preventDefault();
555-
this.navigateTo(0);
555+
this._changeSlideIndex(0, true, true);
556556
await renderFinished();
557557
this.focusItem();
558558
}
559559

560560
async _handleEnd(e: KeyboardEvent) {
561561
e.preventDefault();
562-
this.navigateTo(this.items.length - 1);
562+
this._changeSlideIndex(this.items.length - 1, true, true);
563563
await renderFinished();
564564
this.focusItem();
565565
}
566566

567567
async _handlePageUp(e: KeyboardEvent) {
568568
e.preventDefault();
569-
this.navigateTo(this._currentSlideIndex + this._pageStep);
569+
this._changeSlideIndex(this._currentSlideIndex + this._pageStep, true, true);
570570
await renderFinished();
571571
this.focusItem();
572572
}
573573

574574
async _handlePageDown(e: KeyboardEvent) {
575575
e.preventDefault();
576-
this.navigateTo(this._currentSlideIndex - this._pageStep);
576+
this._changeSlideIndex(this._currentSlideIndex - this._pageStep, true, true);
577577
await renderFinished();
578578
this.focusItem();
579579
}
@@ -600,7 +600,7 @@ class Carousel extends UI5Element {
600600
newFocusedItemIndex = this.items.length - 1;
601601
}
602602

603-
this.focusItemIndex(newFocusedItemIndex);
603+
this._changeFocusIndex(newFocusedItemIndex);
604604
await renderFinished();
605605
this.focusItem();
606606
}
@@ -611,7 +611,7 @@ class Carousel extends UI5Element {
611611
newFocusedItemIndex = 0;
612612
}
613613

614-
this.focusItemIndex(newFocusedItemIndex);
614+
this._changeFocusIndex(newFocusedItemIndex);
615615
await renderFinished();
616616
this.focusItem();
617617
}
@@ -622,7 +622,7 @@ class Carousel extends UI5Element {
622622
newCurrentSlideIndex = 0;
623623
}
624624

625-
this.navigateTo(newCurrentSlideIndex);
625+
this._changeSlideIndex(newCurrentSlideIndex);
626626
await renderFinished();
627627
this.focusItem();
628628
}
@@ -633,7 +633,7 @@ class Carousel extends UI5Element {
633633
newCurrentSlideIndex = this.items.length - 1;
634634
}
635635

636-
this.navigateTo(newCurrentSlideIndex);
636+
this._changeSlideIndex(newCurrentSlideIndex);
637637
await renderFinished();
638638
this.focusItem();
639639
}
@@ -659,17 +659,29 @@ class Carousel extends UI5Element {
659659
* @public
660660
*/
661661
navigateTo(itemIndex: number): void {
662+
this._changeSlideIndex(itemIndex, false);
663+
}
664+
665+
_changeSlideIndex(itemIndex: number, fireEvent = true, moveFocusToNewIndex = false): void {
662666
const newSlideIndex = clamp(itemIndex, 0, this.items.length - this.effectiveItemsPerPage);
663667

668+
if (moveFocusToNewIndex || (this._focusedItemIndex < newSlideIndex || this._focusedItemIndex > newSlideIndex + this.effectiveItemsPerPage - 1)) {
669+
this._focusedItemIndex = clamp(itemIndex, 0, this.items.length - 1);
670+
}
671+
672+
if (this._currentSlideIndex === newSlideIndex) {
673+
return;
674+
}
675+
664676
this._currentSlideIndex = newSlideIndex;
665677
this._updateVisibleItems(newSlideIndex);
666678

667-
if (this._focusedItemIndex < newSlideIndex || this._focusedItemIndex > newSlideIndex + this.effectiveItemsPerPage - 1) {
668-
this._focusedItemIndex = clamp(itemIndex, 0, this.items.length - 1);
679+
if (fireEvent) {
680+
this.fireDecoratorEvent("navigate", { selectedIndex: newSlideIndex });
669681
}
670682
}
671683

672-
focusItemIndex(itemIndex: number) {
684+
_changeFocusIndex(itemIndex: number) {
673685
itemIndex = clamp(itemIndex, 0, this.items.length - 1);
674686
let newSlideIndex = this._currentSlideIndex;
675687

@@ -679,8 +691,13 @@ class Carousel extends UI5Element {
679691
newSlideIndex = itemIndex - this.effectiveItemsPerPage + 1;
680692
}
681693

682-
this._currentSlideIndex = newSlideIndex;
683-
this._updateVisibleItems(newSlideIndex);
694+
if (this._currentSlideIndex !== newSlideIndex) {
695+
this._currentSlideIndex = newSlideIndex;
696+
this._updateVisibleItems(newSlideIndex);
697+
698+
this.fireDecoratorEvent("navigate", { selectedIndex: newSlideIndex });
699+
}
700+
684701
this._focusedItemIndex = itemIndex;
685702
}
686703

‎packages/main/test/pages/CarouselPublicMethods.html‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -223,15 +223,15 @@
223223
</ui5-list>
224224
</ui5-card>
225225
<ui5-card class="myCard">
226-
<ui5-card-header slot="header" status="22 of 23" title-text="Additional Links" subtitle-text="additional links">
226+
<ui5-card-header slot="header" status="22 of 23" title-text="22" subtitle-text="additional links">
227227
</ui5-card-header>
228228
<ui5-list id="myList4" separators="Inner">
229229
<ui5-li icon="calendar">Event Planning</ui5-li>
230230
<ui5-li icon="employee">Team Management</ui5-li>
231231
</ui5-list>
232232
</ui5-card>
233233
<ui5-card class="myCard">
234-
<ui5-card-header slot="header" status="23 of 23" title-text="Resources" subtitle-text="useful resources">
234+
<ui5-card-header slot="header" status="23 of 23" title-text="23" subtitle-text="useful resources">
235235
</ui5-card-header>
236236
<ui5-list id="myList5" separators="Inner">
237237
<ui5-li icon="document">Documentation</ui5-li>

0 commit comments

Comments
 (0)