Skip to content

Commit 85e9be1

Browse files
TeodorTaushanovTeodorTaushanov
authored
chore(ui5-carousel): fix internal naming (#13204)
* chore(ui5-carousel): fix internal naming * chore: rename _getLastFocusedActiveItemIndex to _getLastFocusedItemIndex * chore: remove Active and Selected words
1 parent 13b71d5 commit 85e9be1

File tree

3 files changed

+74
-78
lines changed

3 files changed

+74
-78
lines changed

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -557,10 +557,10 @@ describe("Carousel general interaction", () => {
557557
cy.get("#firstButton").realClick();
558558
cy.realPress("End");
559559
cy.get("#testHomeAndEnd").should("have.prop", "_focusedItemIndex", 9);
560-
cy.get("#testHomeAndEnd").should("have.prop", "_currentSlideIndex", 9);
560+
cy.get("#testHomeAndEnd").should("have.prop", "_currentPageIndex", 9);
561561
cy.realPress("Home");
562562
cy.get("#testHomeAndEnd").should("have.prop", "_focusedItemIndex", 0);
563-
cy.get("#testHomeAndEnd").should("have.prop", "_currentSlideIndex", 0);
563+
cy.get("#testHomeAndEnd").should("have.prop", "_currentPageIndex", 0);
564564
});
565565

566566
it("'PageUp' and 'PageDown' button press", () => {
@@ -592,22 +592,22 @@ describe("Carousel general interaction", () => {
592592

593593
cy.get("#firstButton").realClick();
594594
cy.get("#testPageUpDown").should("have.prop", "_focusedItemIndex", 0);
595-
cy.get("#testPageUpDown").should("have.prop", "_currentSlideIndex", 0);
595+
cy.get("#testPageUpDown").should("have.prop", "_currentPageIndex", 0);
596596
cy.realPress("PageUp");
597597
cy.get("#testPageUpDown").should("have.prop", "_focusedItemIndex", 10);
598-
cy.get("#testPageUpDown").should("have.prop", "_currentSlideIndex", 10);
598+
cy.get("#testPageUpDown").should("have.prop", "_currentPageIndex", 10);
599599
cy.realPress("PageUp");
600600
cy.get("#testPageUpDown").should("have.prop", "_focusedItemIndex", 20);
601-
cy.get("#testPageUpDown").should("have.prop", "_currentSlideIndex", 19);
601+
cy.get("#testPageUpDown").should("have.prop", "_currentPageIndex", 19);
602602
cy.realPress("PageUp");
603603
cy.get("#testPageUpDown").should("have.prop", "_focusedItemIndex", 21);
604-
cy.get("#testPageUpDown").should("have.prop", "_currentSlideIndex", 19);
604+
cy.get("#testPageUpDown").should("have.prop", "_currentPageIndex", 19);
605605
cy.realPress("PageDown");
606606
cy.get("#testPageUpDown").should("have.prop", "_focusedItemIndex", 9);
607-
cy.get("#testPageUpDown").should("have.prop", "_currentSlideIndex", 9);
607+
cy.get("#testPageUpDown").should("have.prop", "_currentPageIndex", 9);
608608
cy.realPress("PageDown");
609609
cy.get("#testPageUpDown").should("have.prop", "_focusedItemIndex", 0);
610-
cy.get("#testPageUpDown").should("have.prop", "_currentSlideIndex", 0);
610+
cy.get("#testPageUpDown").should("have.prop", "_currentPageIndex", 0);
611611
cy.realPress("PageDown");
612612
cy.get("#testPageUpDown").should("have.prop", "_focusedItemIndex", 0);
613613
});

‎packages/main/src/Carousel.ts‎

Lines changed: 64 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ type CarouselNavigateEventDetail = {
5252
selectedIndex: number;
5353
}
5454

55-
type ChangeSlideOptions = {
55+
type ChangePageOptions = {
5656
fireEvent?: boolean;
5757
moveFocus?: boolean;
5858
}
@@ -299,18 +299,18 @@ class Carousel extends UI5Element {
299299
_visibleItemsCount = 0;
300300

301301
/**
302-
* Defines the current slide index, which contains the visible item in the viewport.
302+
* Defines the current page index, which determines the first visible item.
303303
* @private
304304
* @since 2.16.0-r.c1
305305
*/
306306
@property({ type: Number, noAttribute: true })
307-
_currentSlideIndex: number = 0;
307+
_currentPageIndex: number = 0;
308308

309309
_scrollEnablement: ScrollEnablement;
310310
_onResizeBound: ResizeObserverCallback;
311311
_resizing: boolean;
312312
_lastFocusedElements: Array<HTMLElement>;
313-
_orderOfLastFocusedPages: Array<number>;
313+
_orderOfLastFocusedItems: Array<number>;
314314
_lastInnerFocusedElement?: HTMLElement;
315315
_pageStep: number = 10;
316316
_visibleItemsIndexes: Array<number>;
@@ -346,9 +346,9 @@ class Carousel extends UI5Element {
346346

347347
this._visibleItemsCount = visibleItemsCount;
348348

349-
this._currentSlideIndex = clamp(this._currentSlideIndex, 0, Math.max(0, this.items.length - this.effectiveItemsPerPage));
350-
this._focusedItemIndex = clamp(this._focusedItemIndex, this._currentSlideIndex, this.items.length - 1);
351-
this._changeSlideIndex(this._currentSlideIndex, { fireEvent: false });
349+
this._currentPageIndex = clamp(this._currentPageIndex, 0, Math.max(0, this.items.length - this.effectiveItemsPerPage));
350+
this._focusedItemIndex = clamp(this._focusedItemIndex, this._currentPageIndex, this.items.length - 1);
351+
this._changePageIndex(this._currentPageIndex, { fireEvent: false });
352352
});
353353

354354
this._scrollEnablement = new ScrollEnablement(this);
@@ -359,7 +359,7 @@ class Carousel extends UI5Element {
359359
this._resizing = false; // indicates if the carousel is in process of resizing
360360

361361
this._lastFocusedElements = [];
362-
this._orderOfLastFocusedPages = [];
362+
this._orderOfLastFocusedItems = [];
363363
this._visibleItemsIndexes = [];
364364
}
365365

@@ -370,7 +370,7 @@ class Carousel extends UI5Element {
370370
this._visibleNavigationArrows = true;
371371
}
372372

373-
this.validateSelectedIndex();
373+
this.validateFocusedIndex();
374374
}
375375

376376
onAfterRendering() {
@@ -391,7 +391,7 @@ class Carousel extends UI5Element {
391391
ResizeHandler.deregister(this, this._onResizeBound);
392392
}
393393

394-
validateSelectedIndex() {
394+
validateFocusedIndex() {
395395
if (!this.isIndexInRange(this._focusedItemIndex)) {
396396
this._focusedItemIndex = 0;
397397
}
@@ -406,15 +406,15 @@ class Carousel extends UI5Element {
406406
// Change transitively effectiveItemsPerPage by modifying _width
407407
this._width = this.offsetWidth;
408408
this._itemWidth = Math.floor(this._width / this.effectiveItemsPerPage);
409-
this._updateVisibleItems(this._currentSlideIndex);
409+
this._updateVisibleItems(this._currentPageIndex);
410410

411411
// Items per page did not change or the current,
412412
// therefore page index does not need to be re-adjusted
413413
if (this.effectiveItemsPerPage === previousItemsPerPage) {
414414
return;
415415
}
416416

417-
this._focusedItemIndex = clamp(this._focusedItemIndex, this._currentSlideIndex, this.items.length - this.effectiveItemsPerPage);
417+
this._focusedItemIndex = clamp(this._focusedItemIndex, this._currentPageIndex, this.items.length - this.effectiveItemsPerPage);
418418
}
419419

420420
_updateScrolling(e: ScrollEnablementEventListenerParam) {
@@ -463,27 +463,27 @@ class Carousel extends UI5Element {
463463
return;
464464
}
465465

466-
let pageIndex = -1;
466+
let itemIndex = -1;
467467
for (let i = 0; i < this._visibleItems.length; i++) {
468468
if (this._visibleItems[i].isEqualNode(target?.querySelector("slot")?.assignedNodes()[0] as HTMLElement)) {
469-
pageIndex = i;
469+
itemIndex = i;
470470
break;
471471
}
472472
}
473473

474-
if (pageIndex === -1) {
474+
if (itemIndex === -1) {
475475
return;
476476
}
477477

478-
this._focusedItemIndex = pageIndex;
479-
// Save reference of the last focused element for each page
480-
this._lastFocusedElements[pageIndex] = target;
478+
this._focusedItemIndex = itemIndex;
479+
// Save reference of the last focused element for each item
480+
this._lastFocusedElements[itemIndex] = target;
481481

482-
const sortedPageIndex = this._orderOfLastFocusedPages.indexOf(pageIndex);
483-
if (sortedPageIndex === -1) {
484-
this._orderOfLastFocusedPages.unshift(pageIndex);
482+
const sortedItemIndex = this._orderOfLastFocusedItems.indexOf(itemIndex);
483+
if (sortedItemIndex === -1) {
484+
this._orderOfLastFocusedItems.unshift(itemIndex);
485485
} else {
486-
this._orderOfLastFocusedPages.splice(0, 0, this._orderOfLastFocusedPages.splice(sortedPageIndex, 1)[0]);
486+
this._orderOfLastFocusedItems.splice(0, 0, this._orderOfLastFocusedItems.splice(sortedItemIndex, 1)[0]);
487487
}
488488
}
489489

@@ -514,7 +514,7 @@ class Carousel extends UI5Element {
514514
}
515515

516516
async _handleF7Key(e: KeyboardEvent) {
517-
const lastFocusedElement = this._lastFocusedElements[this._getLastFocusedActivePageIndex];
517+
const lastFocusedElement = this._lastFocusedElements[this._getLastFocusedItemIndex];
518518
if (!this._lastInnerFocusedElement) {
519519
const firstFocusable = await getFirstFocusableElement(this.items[this._focusedItemIndex].item);
520520
firstFocusable?.focus();
@@ -557,28 +557,28 @@ class Carousel extends UI5Element {
557557

558558
async _handleHome(e: KeyboardEvent) {
559559
e.preventDefault();
560-
this._changeSlideIndex(0, { moveFocus: true });
560+
this._changePageIndex(0, { moveFocus: true });
561561
await renderFinished();
562562
this.focusItem();
563563
}
564564

565565
async _handleEnd(e: KeyboardEvent) {
566566
e.preventDefault();
567-
this._changeSlideIndex(this.items.length - 1, { moveFocus: true });
567+
this._changePageIndex(this.items.length - 1, { moveFocus: true });
568568
await renderFinished();
569569
this.focusItem();
570570
}
571571

572572
async _handlePageUp(e: KeyboardEvent) {
573573
e.preventDefault();
574-
this._changeSlideIndex(this._currentSlideIndex + this._pageStep, { moveFocus: true });
574+
this._changePageIndex(this._currentPageIndex + this._pageStep, { moveFocus: true });
575575
await renderFinished();
576576
this.focusItem();
577577
}
578578

579579
async _handlePageDown(e: KeyboardEvent) {
580580
e.preventDefault();
581-
this._changeSlideIndex(this._currentSlideIndex - this._pageStep, { moveFocus: true });
581+
this._changePageIndex(this._currentPageIndex - this._pageStep, { moveFocus: true });
582582
await renderFinished();
583583
this.focusItem();
584584
}
@@ -587,12 +587,12 @@ class Carousel extends UI5Element {
587587
return this.backgroundDesign.toLowerCase();
588588
}
589589

590-
get _getLastFocusedActivePageIndex() {
591-
for (let i = 0; i < this._orderOfLastFocusedPages.length; i++) {
592-
const pageIndex = this._orderOfLastFocusedPages[i];
590+
get _getLastFocusedItemIndex() {
591+
for (let i = 0; i < this._orderOfLastFocusedItems.length; i++) {
592+
const itemIndex = this._orderOfLastFocusedItems[i];
593593

594-
if (this.isItemInViewport(pageIndex)) {
595-
return pageIndex;
594+
if (this.isItemVisible(itemIndex)) {
595+
return itemIndex;
596596
}
597597
}
598598

@@ -622,23 +622,23 @@ class Carousel extends UI5Element {
622622
}
623623

624624
async navigateArrowRight() {
625-
let newCurrentSlideIndex = this._currentSlideIndex + 1;
626-
if (this.cyclic && newCurrentSlideIndex > this.items.length - this.effectiveItemsPerPage) {
627-
newCurrentSlideIndex = 0;
625+
let newCurrentPageIndex = this._currentPageIndex + 1;
626+
if (this.cyclic && newCurrentPageIndex > this.items.length - this.effectiveItemsPerPage) {
627+
newCurrentPageIndex = 0;
628628
}
629629

630-
this._changeSlideIndex(newCurrentSlideIndex);
630+
this._changePageIndex(newCurrentPageIndex);
631631
await renderFinished();
632632
this.focusItem();
633633
}
634634

635635
async navigateArrowLeft() {
636-
let newCurrentSlideIndex = this._currentSlideIndex - 1;
637-
if (this.cyclic && newCurrentSlideIndex < 0) {
638-
newCurrentSlideIndex = this.items.length - 1;
636+
let newCurrentPageIndex = this._currentPageIndex - 1;
637+
if (this.cyclic && newCurrentPageIndex < 0) {
638+
newCurrentPageIndex = this.items.length - 1;
639639
}
640640

641-
this._changeSlideIndex(newCurrentSlideIndex);
641+
this._changePageIndex(newCurrentPageIndex);
642642
await renderFinished();
643643
this.focusItem();
644644
}
@@ -664,44 +664,44 @@ class Carousel extends UI5Element {
664664
* @public
665665
*/
666666
navigateTo(itemIndex: number): void {
667-
this._changeSlideIndex(itemIndex, { fireEvent: false });
667+
this._changePageIndex(itemIndex, { fireEvent: false });
668668
}
669669

670-
_changeSlideIndex(itemIndex: number, options: ChangeSlideOptions = {}): void {
670+
_changePageIndex(itemIndex: number, options: ChangePageOptions = {}): void {
671671
const { fireEvent = true, moveFocus = false } = options;
672-
const newSlideIndex = clamp(itemIndex, 0, this.items.length - this.effectiveItemsPerPage);
672+
const newPageIndex = clamp(itemIndex, 0, this.items.length - this.effectiveItemsPerPage);
673673

674-
if (moveFocus || (this._focusedItemIndex < newSlideIndex || this._focusedItemIndex > newSlideIndex + this.effectiveItemsPerPage - 1)) {
674+
if (moveFocus || (this._focusedItemIndex < newPageIndex || this._focusedItemIndex > newPageIndex + this.effectiveItemsPerPage - 1)) {
675675
this._focusedItemIndex = clamp(itemIndex, 0, this.items.length - 1);
676676
}
677677

678-
if (this._currentSlideIndex === newSlideIndex) {
678+
if (this._currentPageIndex === newPageIndex) {
679679
return;
680680
}
681681

682-
this._currentSlideIndex = newSlideIndex;
683-
this._updateVisibleItems(newSlideIndex);
682+
this._currentPageIndex = newPageIndex;
683+
this._updateVisibleItems(newPageIndex);
684684

685685
if (fireEvent) {
686-
this.fireDecoratorEvent("navigate", { selectedIndex: newSlideIndex });
686+
this.fireDecoratorEvent("navigate", { selectedIndex: newPageIndex });
687687
}
688688
}
689689

690690
_changeFocusIndex(itemIndex: number) {
691691
itemIndex = clamp(itemIndex, 0, this.items.length - 1);
692-
let newSlideIndex = this._currentSlideIndex;
692+
let newPageIndex = this._currentPageIndex;
693693

694-
if (itemIndex < this._currentSlideIndex) {
695-
newSlideIndex = itemIndex;
696-
} else if (itemIndex > this._currentSlideIndex + this.effectiveItemsPerPage - 1) {
697-
newSlideIndex = itemIndex - this.effectiveItemsPerPage + 1;
694+
if (itemIndex < this._currentPageIndex) {
695+
newPageIndex = itemIndex;
696+
} else if (itemIndex > this._currentPageIndex + this.effectiveItemsPerPage - 1) {
697+
newPageIndex = itemIndex - this.effectiveItemsPerPage + 1;
698698
}
699699

700-
if (this._currentSlideIndex !== newSlideIndex) {
701-
this._currentSlideIndex = newSlideIndex;
702-
this._updateVisibleItems(newSlideIndex);
700+
if (this._currentPageIndex !== newPageIndex) {
701+
this._currentPageIndex = newPageIndex;
702+
this._updateVisibleItems(newPageIndex);
703703

704-
this.fireDecoratorEvent("navigate", { selectedIndex: newSlideIndex });
704+
this.fireDecoratorEvent("navigate", { selectedIndex: newPageIndex });
705705
}
706706

707707
this._focusedItemIndex = itemIndex;
@@ -726,10 +726,10 @@ class Carousel extends UI5Element {
726726
return {
727727
id: `${this._id}-carousel-item-${idx + 1}`,
728728
item,
729-
tabIndex: this.isItemInViewport(this._focusedItemIndex) ? 0 : -1,
729+
tabIndex: this.isItemVisible(this._focusedItemIndex) ? 0 : -1,
730730
posinset: idx + 1,
731731
setsize: this._visibleItems.length,
732-
visible: this.isItemInViewport(idx),
732+
visible: this.isItemVisible(idx),
733733
};
734734
});
735735
}
@@ -771,13 +771,13 @@ class Carousel extends UI5Element {
771771
return itemsPerPageSizeXL;
772772
}
773773

774-
isItemInViewport(index: number): boolean {
774+
isItemVisible(index: number): boolean {
775775
return this._visibleItemsIndexes.includes(index);
776776
}
777777

778778
_updateVisibleItems(index: number) {
779779
let newItemIndex = index;
780-
const effectiveItemsPerPage: number = this.effectiveItemsPerPage;
780+
const effectiveItemsPerPage = this.effectiveItemsPerPage;
781781
const items = this.items;
782782

783783
if (!items.length) {
@@ -863,7 +863,7 @@ class Carousel extends UI5Element {
863863

864864
for (let index = 0; index < pages; index++) {
865865
dots.push({
866-
active: index === this._currentSlideIndex,
866+
active: index === this._currentPageIndex,
867867
ariaLabel: Carousel.i18nBundle.getText(CAROUSEL_DOT_TEXT, index + 1, pages),
868868
});
869869
}
@@ -880,11 +880,11 @@ class Carousel extends UI5Element {
880880
}
881881

882882
get hasPrev() {
883-
return this.cyclic || (this._focusedItemIndex - 1 >= 0 && this._currentSlideIndex !== 0);
883+
return this.cyclic || (this._focusedItemIndex - 1 >= 0 && this._currentPageIndex !== 0);
884884
}
885885

886886
get hasNext() {
887-
return this.cyclic || (this._focusedItemIndex + 1 <= this._visibleItems.length - 1 && this._currentSlideIndex < this.pagesCount - 1);
887+
return this.cyclic || (this._focusedItemIndex + 1 <= this._visibleItems.length - 1 && this._currentPageIndex < this.pagesCount - 1);
888888
}
889889

890890
get suppressAnimation() {
@@ -895,10 +895,6 @@ class Carousel extends UI5Element {
895895
return this.effectiveDir === "rtl";
896896
}
897897

898-
get selectedIndexToShow() {
899-
return this._isRTL ? this.items.length - (this.items.length - this._focusedItemIndex) + 1 : this._focusedItemIndex + 1;
900-
}
901-
902898
get ofText() {
903899
return Carousel.i18nBundle.getText(CAROUSEL_OF_TEXT);
904900
}

0 commit comments

Comments
 (0)