fix(pagination): should be able to toggle Pagination

- we should be able to toggle Pagination and it should still work as expected after showing it again - the previous code was disposing of the Pagination Service when hiding (disposing) the Pagination Component but that shouldn't be the case, disposing of the Service should strictly be handled by the Slick Vanilla Bundle
ghiscoding · Jun 28, 2021 · c0367c2 · c0367c2
1 parent 3ad687a
commit c0367c2
Show file tree

Hide file tree

Showing 9 changed files with 138 additions and 23 deletions.
diff --git a/examples/webpack-demo-vanilla-bundle/src/examples/example01.html b/examples/webpack-demo-vanilla-bundle/src/examples/example01.html
@@ -14,9 +14,23 @@ <h5 class="title is-5">Grid 1</h5>
 </div>
 
 <div class="column is-half">
-  <hr />
+  <hr style="margin: 16px 0" />
+</div>
+
+<div class="columns">
+  <div class="column field is-narrow">
+    <h5 class="title is-5">Grid 2</h5>
+  </div>
+  <div class="column field has-addons is-narrow">
+    <p class="control">
+      <button class="button is-small" onclick.delegate="togglePaginationGrid2()"
+              data-text="toggle-pagination-btn">
+        <span class="icon mdi mdi-swap-vertical"></span>
+        <span>Toggle Pagination</span>
+      </button>
+    </p>
+  </div>
 </div>
 
-<h5 class="title is-5">Grid 2</h5>
 <div class="grid2">
 </div>
diff --git a/examples/webpack-demo-vanilla-bundle/src/examples/example01.ts b/examples/webpack-demo-vanilla-bundle/src/examples/example01.ts
@@ -17,6 +17,7 @@ export class Example1 {
   dataset2: any[];
   sgb1: SlickVanillaGridBundle;
   sgb2: SlickVanillaGridBundle;
+  isGrid2WithPagination = true;
 
   attached() {
     this.defineGrids();
@@ -105,4 +106,12 @@ export class Example1 {
 
     return mockDataset;
   }
+
+  // Toggle the Pagination of Grid2
+  // IMPORTANT, the Pagination MUST BE CREATED on initial page load before you can start toggling it
+  // Basically you cannot toggle a Pagination that doesn't exist (must created at the time as the grid)
+  togglePaginationGrid2() {
+    this.isGrid2WithPagination = !this.isGrid2WithPagination;
+    this.sgb2.paginationService!.togglePaginationVisibility(this.isGrid2WithPagination);
+  }
 }
diff --git a/packages/common/src/services/__tests__/pagination.service.spec.ts b/packages/common/src/services/__tests__/pagination.service.spec.ts
@@ -518,7 +518,7 @@ describe('PaginationService', () => {
       }
     });
 
-    it('should call refreshPagination when "onFilterCleared" is triggered', () => {
+    it('should call refreshPagination when "onFilterCleared" is triggered and Pagination is enabled', () => {
       const resetSpy = jest.spyOn(service, 'resetPagination');
       const refreshSpy = jest.spyOn(service, 'refreshPagination');
 
@@ -529,7 +529,7 @@ describe('PaginationService', () => {
       expect(refreshSpy).toHaveBeenCalledWith(true, true);
     });
 
-    it('should call refreshPagination when "onFilterChanged" is triggered', () => {
+    it('should call refreshPagination when "onFilterChanged" is triggered and Pagination is enabled', () => {
       const pubSubSpy = jest.spyOn(mockPubSub, 'publish');
       const resetSpy = jest.spyOn(service, 'resetPagination');
       const refreshSpy = jest.spyOn(service, 'refreshPagination');
@@ -563,6 +563,8 @@ describe('PaginationService', () => {
     });
 
     it('should reset the DataView when using local grid by calling "setPagingOptions" with page 0 and also call "refreshPagination" method', () => {
+      const gridOptionsMock = { enablePagination: true };
+      jest.spyOn(SharedService.prototype, 'gridOptions', 'get').mockReturnValue(gridOptionsMock);
       const spy = jest.spyOn(service, 'refreshPagination');
       const setPagingSpy = jest.spyOn(dataviewStub, 'setPagingOptions');
 
@@ -573,6 +575,20 @@ describe('PaginationService', () => {
       expect(setPagingSpy).toHaveBeenCalledWith({ pageSize: 25, pageNum: 0 });
       expect(spy).toHaveBeenCalledWith(true, true);
     });
+
+    it('should NOT reset the DataView "setPagingOptions" when Pagination is NOT enabled', () => {
+      const gridOptionsMock = { enablePagination: false };
+      jest.spyOn(SharedService.prototype, 'gridOptions', 'get').mockReturnValue(gridOptionsMock);
+      const spy = jest.spyOn(service, 'refreshPagination');
+      const setPagingSpy = jest.spyOn(dataviewStub, 'setPagingOptions');
+
+      mockGridOption.backendServiceApi = null as any;
+      service.init(gridStub, mockGridOption.pagination as Pagination, null as any);
+      service.resetPagination();
+
+      expect(setPagingSpy).not.toHaveBeenCalled();
+      expect(spy).toHaveBeenCalledWith(true, true);
+    });
   });
 
   // processOnItemAddedOrRemoved is private but we can spy on recalculateFromToIndexes

diff --git a/packages/common/src/services/backendUtility.service.ts b/packages/common/src/services/backendUtility.service.ts
@@ -15,7 +15,7 @@ export class BackendUtilityService {
 
     // define what our internal Post Process callback, only available for GraphQL Service for now
     // it will basically refresh the Dataset & Pagination removing the need for the user to always create his own PostProcess every time
-    if (processResult && backendApi && backendApi.internalPostProcess) {
+    if (processResult && backendApi?.internalPostProcess) {
       backendApi.internalPostProcess(processResult);
     }
 

diff --git a/packages/common/src/services/pagination.service.ts b/packages/common/src/services/pagination.service.ts
@@ -281,7 +281,7 @@ export class PaginationService {
 
   /** Reset the Pagination to first page and recalculate necessary numbers */
   resetPagination(triggerChangedEvent = true) {
-    if (this._isLocalGrid && this.dataView) {
+    if (this._isLocalGrid && this.dataView && this.sharedService?.gridOptions?.enablePagination) {
       // on a local grid we also need to reset the DataView paging to 1st page
       this.dataView.setPagingOptions({ pageSize: this._itemsPerPage, pageNum: 0 });
     }

diff --git a/packages/vanilla-bundle/src/components/__tests__/slick-vanilla-grid.spec.ts b/packages/vanilla-bundle/src/components/__tests__/slick-vanilla-grid.spec.ts
@@ -2010,21 +2010,20 @@ describe('Slick-Vanilla-Grid-Bundle Component instantiated via Constructor', ()
         sharedService.slickGrid = mockGrid as unknown as SlickGrid;
       });
 
-      it('should change "showPagination" flag when "onPaginationVisibilityChanged" from the Pagination Service is triggered', (done) => {
+      it('should change "showPagination" flag when "onPaginationVisibilityChanged" from the Pagination Service is triggered', () => {
         component.gridOptions.enablePagination = true;
         component.gridOptions.backendServiceApi = null;
 
         component.initialization(divContainer, slickEventHandler);
         component.refreshGridData([{ firstName: 'John', lastName: 'Doe' }]);
+        const disposeSpy = jest.spyOn(component.slickPagination, 'dispose');
         eventPubSubService.publish('onPaginationVisibilityChanged', { visible: false });
 
-        setTimeout(() => {
-          expect(component.showPagination).toBeFalsy();
-          done();
-        });
+        expect(component.showPagination).toBeFalsy();
+        expect(disposeSpy).toHaveBeenCalled();
       });
 
-      it('should call the backend service API to refresh the dataset', (done) => {
+      it('should call the backend service API to refresh the dataset', () => {
         const backendRefreshSpy = jest.spyOn(backendUtilityServiceStub, 'refreshBackendDataset');
         component.gridOptions.enablePagination = true;
         component.gridOptions.backendServiceApi = {
@@ -2034,13 +2033,12 @@ describe('Slick-Vanilla-Grid-Bundle Component instantiated via Constructor', ()
 
         component.initialization(divContainer, slickEventHandler);
         component.refreshGridData([{ firstName: 'John', lastName: 'Doe' }]);
-        eventPubSubService.publish('onPaginationVisibilityChanged', { visible: false });
 
-        setTimeout(() => {
-          expect(backendRefreshSpy).toHaveBeenCalled();
-          expect(component.showPagination).toBeFalsy();
-          done();
-        });
+        eventPubSubService.publish('onPaginationVisibilityChanged', { visible: true });
+
+        expect(backendRefreshSpy).toHaveBeenCalled();
+        expect(component.slickPagination).toBeTruthy();
+        expect(component.showPagination).toBeTruthy();
       });
     });
 

diff --git a/packages/vanilla-bundle/src/components/slick-pagination.component.ts b/packages/vanilla-bundle/src/components/slick-pagination.component.ts
@@ -125,7 +125,6 @@ export class SlickPaginationComponent {
     this.pubSubService.unsubscribeAll(this._subscriptions);
 
     this._bindingHelper.dispose();
-    this.paginationService.dispose();
     this._paginationElement.remove();
   }
 

diff --git a/packages/vanilla-bundle/src/components/slick-vanilla-grid-bundle.ts b/packages/vanilla-bundle/src/components/slick-vanilla-grid-bundle.ts
@@ -1176,19 +1176,35 @@ export class SlickVanillaGridBundle {
           if (this.gridOptions?.backendServiceApi) {
             this.backendUtilityService?.refreshBackendDataset(this.gridOptions);
           }
+          this.renderPagination(this.showPagination);
         })
       );
 
       // also initialize (render) the pagination component
-      if (this._gridOptions.enablePagination && !this._isPaginationInitialized) {
-        this.slickPagination = new SlickPaginationComponent(this.paginationService, this._eventPubSubService, this.sharedService, this.translaterService);
-        this.slickPagination.renderPagination(this._gridParentContainerElm);
-      }
+      this.renderPagination();
 
       this._isPaginationInitialized = true;
     }
   }
 
+  /**
+   * Render (or dispose) the Pagination Component, user can optionally provide False (to not show it) which will in term dispose of the Pagination,
+   * also while disposing we can choose to omit the disposable of the Pagination Service (if we are simply toggling the Pagination, we want to keep the Service alive)
+   * @param {Boolean} showPagination - show (new render) or not (dispose) the Pagination
+   * @param {Boolean} shouldDisposePaginationService - when disposing the Pagination, do we also want to dispose of the Pagination Service? (defaults to True)
+   */
+  private renderPagination(showPagination = true) {
+    if (this._gridOptions.enablePagination && !this._isPaginationInitialized && showPagination) {
+      this.slickPagination = new SlickPaginationComponent(this.paginationService, this._eventPubSubService, this.sharedService, this.translaterService);
+      this.slickPagination.renderPagination(this._gridParentContainerElm);
+    } else {
+      if (this.slickPagination) {
+        this.slickPagination.dispose();
+      }
+      this._isPaginationInitialized = false;
+    }
+  }
+
   /** Load the Editor Collection asynchronously and replace the "collection" property when Promise resolves */
   private loadEditorCollectionAsync(column: Column) {
     const collectionAsync = (column?.editor as ColumnEditor).collectionAsync;

diff --git a/test/cypress/integration/example01.spec.js b/test/cypress/integration/example01.spec.js
@@ -476,4 +476,67 @@ describe('Example 01 - Basic Grids', { retries: 1 }, () => {
       .find('span.close')
       .click({ force: true });
   });
+
+  it('should toggle (remove) Pagination from 2nd grid and not expect any Pagination DOM element', () => {
+    // sort by Title
+    cy.get('.grid2 .slick-header-column:nth(0)')
+      .click();
+
+    cy.get('.grid2 .slick-pagination')
+      .should('exist');
+
+    cy.get('[data-text="toggle-pagination-btn"]')
+      .click();
+
+    cy.get('.grid2 .slick-pagination')
+      .should('not.exist');
+
+    cy.get('.search-filter.filter-title')
+      .clear()
+      .type('44');
+
+    cy.get('.grid2')
+      .find('.slick-viewport-top.slick-viewport-left')
+      .scrollTo('bottom')
+      .wait(10);
+
+    cy.get(`.grid2 [style="top:${GRID_ROW_HEIGHT * 14}px"] > .slick-cell:nth(0)`).should('contain', 'Task 544');
+    cy.get(`.grid2 [style="top:${GRID_ROW_HEIGHT * 15}px"] > .slick-cell:nth(0)`).should('contain', 'Task 644');
+    cy.get(`.grid2 [style="top:${GRID_ROW_HEIGHT * 16}px"] > .slick-cell:nth(0)`).should('contain', 'Task 744');
+    cy.get(`.grid2 [style="top:${GRID_ROW_HEIGHT * 17}px"] > .slick-cell:nth(0)`).should('contain', 'Task 844');
+    cy.get(`.grid2 [style="top:${GRID_ROW_HEIGHT * 18}px"] > .slick-cell:nth(0)`).should('contain', 'Task 944');
+  });
+
+  it('should toggle again (show) the Pagination and expect to see it show it again below the grid at Page 1', () => {
+    cy.get('[data-text="toggle-pagination-btn"]')
+      .click();
+
+    cy.get('.grid2 .slick-pagination')
+      .should('exist');
+
+    cy.get('.grid2')
+      .find('[data-test=page-number-input]')
+      .invoke('val')
+      .then(pageNumber => expect(pageNumber).to.eq('1'));
+
+    cy.get('.grid2')
+      .find('[data-test=page-number-input]')
+      .click();
+
+    cy.get('.grid2')
+      .find('[data-test=page-count]')
+      .contains('4');
+
+    cy.get('.grid2')
+      .find('[data-test=item-from]')
+      .contains('1');
+
+    cy.get('.grid2')
+      .find('[data-test=item-to]')
+      .contains('5');
+
+    cy.get('.grid2')
+      .find('[data-test=total-items]')
+      .contains('19');
+  });
 });