feat(pagination): improve global code quality by removing magic numbe…

…rs & adding tests

packages/ods/src/components/pagination/src/components/ods-pagination/ods-pagination.scss

@@ -1,10 +1,12 @@
+@use '../../../../../style/pagination';
+
 :host(.ods-pagination) {
   display: flex;
   flex-direction: row;
   align-items: center;
 }
 
-:host([isDisabled]) {
+:host(.ods-pagination--disabled) {
   opacity: .5;
   cursor: not-allowed;
 }
@@ -49,14 +51,13 @@
       &__ellipsis {
         display: flex;
         justify-content: center;
-        width: 2.5rem;
-        pointer-events: auto;
+        width: pagination.$ods-pagination-button-width;
       }
 
       &__button::part(button) {
         display: none;
         justify-content: center;
-        width: 2.5rem;
+        width: pagination.$ods-pagination-button-width;
       }
 
       &__button--visible::part(button) {

packages/ods/src/components/pagination/src/components/ods-pagination/ods-pagination.tsx

@@ -17,12 +17,11 @@ import { computeActualTotalPages, createPageList, getActualPage } from '../../co
 })
 export class OdsPagination {
   private actualTotalPages = 1;
-  private isFirstLoad: boolean = true;
   private leftArrowButtonId = 'pagination-left-arrow';
   private rightArrowButtonId = 'pagination-right-arrow';
   private hostId: string = '';
-  private maxVisibleItems = 7;
-  private ellipsisThreshold = 4;
+  private MAX_VISIBLE_ITEMS = 7;
+  private ELLIPSIS_THRESHOLD = 4;
 
   @Element() el!: HTMLElement;
 
@@ -76,15 +75,13 @@ export class OdsPagination {
 
   @Watch('itemPerPage')
   async onItemPerPageChange(): Promise<void> {
-    if (!this.isFirstLoad) {
-      await this.updatePagination();
-
-      this.odsPaginationItemPerPageChanged.emit({
-        current: this.itemPerPage,
-        currentPage: this.current,
-        totalPages: this.actualTotalPages,
-      });
-    }
+    await this.updatePagination();
+
+    this.odsPaginationItemPerPageChanged.emit({
+      current: this.itemPerPage,
+      currentPage: this.current,
+      totalPages: this.actualTotalPages,
+    });
   }
 
   @Watch('totalItems')
@@ -116,13 +113,10 @@ export class OdsPagination {
     this.current = getActualPage(this.defaultCurrentPage, this.actualTotalPages, this.current);
 
     this.updatePageList();
-    this.isFirstLoad = false;
   }
 
   private updatePageList(): void {
-    this.pageList = createPageList(this.actualTotalPages, this.current).map((page) => ({
-      ...page,
-    }));
+    this.pageList = createPageList(this.actualTotalPages, this.current);
   }
 
   private emitChange(current: number, oldCurrent?: number): void {
@@ -185,7 +179,7 @@ export class OdsPagination {
           size={ODS_BUTTON_SIZE.md}
         >
         </ods-button>
-        {tooltipLabel && (
+        {tooltipLabel && !isArrowDisabled && (
           <ods-tooltip
             shadowDomTriggerId={arrowButtonId}
             triggerId={this.hostId}
@@ -217,13 +211,15 @@ export class OdsPagination {
       return;
     }
 
-    const renderEllipsisLeft = this.current > this.ellipsisThreshold && this.actualTotalPages > this.maxVisibleItems;
-    const renderEllipsisRight = this.current < this.actualTotalPages - this.ellipsisThreshold + 1 && this.actualTotalPages > this.maxVisibleItems;
+    const renderEllipsisLeft = this.current > this.ELLIPSIS_THRESHOLD && this.actualTotalPages > this.MAX_VISIBLE_ITEMS;
+    const renderEllipsisRight = this.current < this.actualTotalPages - this.ELLIPSIS_THRESHOLD + 1 && this.actualTotalPages > this.MAX_VISIBLE_ITEMS;
 
     return (
       <Host
-        class="ods-pagination"
-        isDisabled={this.isDisabled}
+        class={{
+          'ods-pagination': true,
+          'ods-pagination--disabled': this.isDisabled,
+        }}
         id={this.hostId}
       >
         {

packages/ods/src/components/pagination/src/controller/ods-pagination.ts

@@ -9,35 +9,39 @@ function computeActualTotalPages(itemPerPage: number, totalItems: number | undef
 }
 
 function createPageList(totalPages: number, pageSelected: number): OdsPaginationPageList {
+  const MAX_VISIBLE_ITEMS = 7;
+  const ELLIPSIS_THRESHOLD = 4;
+  const MINIMUM_PAGE = 1;
+  const DEFAULT_PAGE_OFFSET = 2;
   const pageList: OdsPaginationPageList = [];
 
   // Create initial pageList with 'active' property set to false for each page.
   for (let i = 1; i <= totalPages; i++) {
     pageList.push({ active: false });
   }
 
-  let startIndex = Math.max(pageSelected - 2, 1);
-  const endIndex = Math.min(startIndex + 4, totalPages);
+  let startIndex = Math.max(pageSelected - DEFAULT_PAGE_OFFSET, 1);
+  const endIndex = Math.min(startIndex + ELLIPSIS_THRESHOLD, totalPages);
 
   // If there are less than or equal to 5 pages, set all pages as active (all displayed).
-  if (totalPages <= 5) {
+  if (totalPages <= (MAX_VISIBLE_ITEMS - DEFAULT_PAGE_OFFSET)) {
     for (let i = 0; i < pageList.length; i++) {
       pageList[i].active = true;
     }
   } else {
     // If there are more than 5 pages, set only some of them as active (not all displayed).
-    if (totalPages - pageSelected < 2) {
+    if (totalPages - pageSelected < DEFAULT_PAGE_OFFSET) {
       // If selected page is one of the last pages of a long list, show the last 5 pages.
-      startIndex = totalPages - 4;
+      startIndex = totalPages - ELLIPSIS_THRESHOLD;
     }
 
     for (let i = startIndex; i <= endIndex; i++) {
       // If i is two pages away from the selected page, skip it if it is between 4 and totalPages-2.
-      if (i == pageSelected - 2 && pageSelected < totalPages - 1 && pageSelected > 4 && pageSelected < totalPages - 2) {
+      if (i == pageSelected - DEFAULT_PAGE_OFFSET && pageSelected < totalPages - 1 && pageSelected > ELLIPSIS_THRESHOLD && pageSelected < totalPages - DEFAULT_PAGE_OFFSET) {
         continue;
       }
       // If i is two pages away from the selected page, skip it if it is greater than 5.
-      if (i == pageSelected + 2 && pageSelected < totalPages - 3 && i > 5) {
+      if (i == pageSelected + DEFAULT_PAGE_OFFSET && pageSelected < totalPages - (DEFAULT_PAGE_OFFSET + MINIMUM_PAGE) && i > (MAX_VISIBLE_ITEMS - DEFAULT_PAGE_OFFSET)) {
         continue;
       }
       pageList[i - 1].active = true;

packages/ods/src/components/pagination/src/index.html

@@ -54,7 +54,7 @@ <h3>is-disabled</h3>
   <div class="row">
     <ods-pagination default-current-page=1 total-pages=9001 ></ods-pagination>
     <p class="description">It's over 9000</p>
-  </div>
+  </div> -->
 
   <h3>Less than two pages</h3>
   <div class="row">
@@ -77,7 +77,7 @@ <h3>Total items between 10 and 300</h3>
       <span slot="after-total-items">&nbsp;results</span>
     </ods-pagination>
     <p class="description">Select will have custom options</p>
-  </div> -->
+  </div>
 
   <h3>Total items above 300</h3>
   <div class="row">

packages/ods/src/components/pagination/tests/controller/ods-pagination.spec.ts

@@ -0,0 +1,195 @@
+import { computeActualTotalPages, createPageList, getActualPage } from '../../src/controller/ods-pagination';
+
+describe('ods-pagination controller', () => {
+  beforeEach(jest.clearAllMocks);
+
+  describe('computeActualTotalPages', () => {
+    it('should return totalPages if totalItems is undefined', () => {
+      const itemPerPage = 10;
+      const totalItems = undefined;
+      const totalPages = 5;
+
+      const result = computeActualTotalPages(itemPerPage, totalItems, totalPages);
+
+      expect(result).toBe(totalPages);
+    });
+
+    it('should return the correct number of total pages when totalItems is defined', () => {
+      const itemPerPage = 10;
+      const totalItems = 95;
+      const totalPages = 5;
+
+      const result = computeActualTotalPages(itemPerPage, totalItems, totalPages);
+
+      expect(result).toBe(Math.ceil(totalItems / itemPerPage));
+    });
+
+    it('should handle itemPerPage being 0 or undefined', () => {
+      const itemPerPage = 0;
+      const totalItems = 50;
+      const totalPages = 5;
+
+      const result = computeActualTotalPages(itemPerPage, totalItems, totalPages);
+
+      expect(result).toBe(Math.ceil(totalItems / 1));
+    });
+
+    it('should handle totalItems being 0', () => {
+      const itemPerPage = 10;
+      const totalItems = 0;
+      const totalPages = 5;
+
+      const result = computeActualTotalPages(itemPerPage, totalItems, totalPages);
+
+      expect(result).toBe(totalPages);
+    });
+
+    it('should handle large numbers of totalItems and itemPerPage correctly', () => {
+      const itemPerPage = 1000;
+      const totalItems = 100000;
+      const totalPages = 100;
+
+      const result = computeActualTotalPages(itemPerPage, totalItems, totalPages);
+
+      expect(result).toBe(Math.ceil(totalItems / itemPerPage));
+    });
+
+    it('should handle small numbers of totalItems and itemPerPage correctly', () => {
+      const itemPerPage = 2;
+      const totalItems = 5;
+      const totalPages = 3;
+
+      const result = computeActualTotalPages(itemPerPage, totalItems, totalPages);
+
+      expect(result).toBe(Math.ceil(totalItems / itemPerPage));
+    });
+  });
+
+  describe('createPageList', () => {
+    it('should return all pages active when totalPages is less than or equal to MAX_VISIBLE_ITEMS', () => {
+      const totalPages = 5;
+      const pageSelected = 1;
+
+      const result = createPageList(totalPages, pageSelected);
+
+      expect(result).toHaveLength(totalPages);
+      result.forEach((page) => expect(page.active).toBe(true));
+    });
+
+    it('should activate appropriate pages when totalPages is greater than MAX_VISIBLE_ITEMS and pageSelected is in the middle', () => {
+      const totalPages = 10;
+      const pageSelected = 5;
+
+      const result = createPageList(totalPages, pageSelected);
+
+      expect(result).toHaveLength(totalPages);
+      result.forEach((page, index) => {
+        if (index === 0 || index === totalPages - 1 || (index >= (pageSelected - 2) && index <= pageSelected)) {
+          expect(page.active).toBe(true);
+        } else {
+          expect(page.active).toBe(false);
+        }
+      });
+    });
+
+    it('should activate the first page and the range around pageSelected when it is near the start', () => {
+      const totalPages = 10;
+      const pageSelected = 3;
+
+      const result = createPageList(totalPages, pageSelected);
+
+      expect(result).toHaveLength(totalPages);
+      result.forEach((page, index) => {
+        if (index <= 4 || index === totalPages - 1) {
+          expect(page.active).toBe(true);
+        } else {
+          expect(page.active).toBe(false);
+        }
+      });
+    });
+
+    it('should activate the last page and the range around pageSelected when it is near the end', () => {
+      const totalPages = 10;
+      const pageSelected = 8;
+
+      const result = createPageList(totalPages, pageSelected);
+
+      expect(result).toHaveLength(totalPages);
+      result.forEach((page, index) => {
+        if (index === 0 || index === totalPages - 1 || index >= 5) {
+          expect(page.active).toBe(true);
+        } else {
+          expect(page.active).toBe(false);
+        }
+      });
+    });
+
+    it('should handle a small number of totalPages correctly', () => {
+      const totalPages = 3;
+      const pageSelected = 2;
+
+      const result = createPageList(totalPages, pageSelected);
+
+      expect(result).toHaveLength(totalPages);
+      result.forEach((page) => expect(page.active).toBe(true));
+    });
+
+    it('should handle a large number of totalPages correctly', () => {
+      const totalPages = 100;
+      const pageSelected = 50;
+
+      const result = createPageList(totalPages, pageSelected);
+
+      expect(result).toHaveLength(totalPages);
+      result.forEach((page, index) => {
+        if (index === 0 || index === totalPages - 1 || (index >= (pageSelected - 2) && index <= pageSelected)) {
+          expect(page.active).toBe(true);
+        } else {
+          expect(page.active).toBe(false);
+        }
+      });
+    });
+  });
+
+  describe('getActualPage', () => {
+    it('should return actualTotalPages if defaultCurrentPage is greater than actualTotalPages', () => {
+      const defaultCurrentPage = 10;
+      const actualTotalPages = 5;
+      const current = 1;
+
+      const result = getActualPage(defaultCurrentPage, actualTotalPages, current);
+
+      expect(result).toBe(actualTotalPages);
+    });
+
+    it('should return 1 if defaultCurrentPage is less than 1', () => {
+      const defaultCurrentPage = 0;
+      const actualTotalPages = 5;
+      const current = 1;
+
+      const result = getActualPage(defaultCurrentPage, actualTotalPages, current);
+
+      expect(result).toBe(1);
+    });
+
+    it('should return defaultCurrentPage if it is within the valid range', () => {
+      const defaultCurrentPage = 3;
+      const actualTotalPages = 5;
+      const current = 1;
+
+      const result = getActualPage(defaultCurrentPage, actualTotalPages, current);
+
+      expect(result).toBe(defaultCurrentPage);
+    });
+
+    it('should handle defaultCurrentPage being 1 when actualTotalPages is also 1', () => {
+      const defaultCurrentPage = 1;
+      const actualTotalPages = 1;
+      const current = 3;
+
+      const result = getActualPage(defaultCurrentPage, actualTotalPages, current);
+
+      expect(result).toBe(defaultCurrentPage);
+    });
+  });
+});

packages/ods/src/style/_pagination.scss

@@ -0,0 +1 @@
+$ods-pagination-button-width: 2.5rem;