feat(b-pagination): if number of pages changes, try and keep current page active (closes #3716) (#3990)

tmorehouse · web-flow · commit ae8ce78f019a · 2019-08-31T04:06:50.000-03:00
diff --git a/src/components/pagination/pagination.js b/src/components/pagination/pagination.js
@@ -1,25 +1,16 @@
 import Vue from '../../utils/vue'
 import { getComponentConfig } from '../../utils/config'
 import { isVisible } from '../../utils/dom'
+import { isUndefinedOrNull } from '../../utils/inspect'
 import paginationMixin from '../../mixins/pagination'
 
+// --- Constants ---
+
 const NAME = 'BPagination'
 
 const DEFAULT_PER_PAGE = 20
 const DEFAULT_TOTAL_ROWS = 0
 
-// Sanitize the provided per page number (converting to a number)
-const sanitizePerPage = val => {
-  const perPage = parseInt(val, 10) || DEFAULT_PER_PAGE
-  return perPage < 1 ? 1 : perPage
-}
-
-// Sanitize the provided total rows number (converting to a number)
-const sanitizeTotalRows = val => {
-  const totalRows = parseInt(val, 10) || DEFAULT_TOTAL_ROWS
-  return totalRows < 0 ? 0 : totalRows
-}
-
 const props = {
   size: {
     type: String,
@@ -39,7 +30,21 @@ const props = {
   }
 }
 
-// The render function is brought in via the pagination mixin
+// --- Helper functions ---
+
+// Sanitize the provided per page number (converting to a number)
+const sanitizePerPage = val => {
+  const perPage = parseInt(val, 10) || DEFAULT_PER_PAGE
+  return perPage < 1 ? 1 : perPage
+}
+
+// Sanitize the provided total rows number (converting to a number)
+const sanitizeTotalRows = val => {
+  const totalRows = parseInt(val, 10) || DEFAULT_TOTAL_ROWS
+  return totalRows < 0 ? 0 : totalRows
+}
+
+// The render function is brought in via the `paginationMixin`
 // @vue/component
 export const BPagination = /*#__PURE__*/ Vue.extend({
   name: NAME,
@@ -49,16 +54,32 @@ export const BPagination = /*#__PURE__*/ Vue.extend({
     numberOfPages() {
       const result = Math.ceil(sanitizeTotalRows(this.totalRows) / sanitizePerPage(this.perPage))
       return result < 1 ? 1 : result
+    },
+    pageSizeNumberOfPages() {
+      // Used for watching changes to `perPage` and `numberOfPages`
+      return {
+        perPage: sanitizePerPage(this.perPage),
+        totalRows: sanitizeTotalRows(this.totalRows),
+        numberOfPages: this.numberOfPages
+      }
     }
   },
   watch: {
-    numberOfPages(newVal) {
-      if (newVal === this.localNumberOfPages) {
-        /* istanbul ignore next */
-        return
+    pageSizeNumberOfPages(newVal, oldVal) {
+      if (!isUndefinedOrNull(oldVal)) {
+        if (newVal.perPage !== oldVal.perPage && newVal.totalRows === oldVal.totalRows) {
+          // If the page size changes, reset to page 1
+          this.currentPage = 1
+        } else if (
+          newVal.numberOfPages !== oldVal.numberOfPages &&
+          this.currentPage > newVal.numberOfPages
+        ) {
+          // If `numberOfPages` changes and is less than
+          // the `currentPage` number, reset to page 1
+          this.currentPage = 1
+        }
       }
-      this.localNumberOfPages = newVal
-      this.currentPage = 1
+      this.localNumberOfPages = newVal.numberOfPages
     }
   },
   created() {
diff --git a/src/components/pagination/pagination.spec.js b/src/components/pagination/pagination.spec.js
@@ -150,13 +150,15 @@ describe('pagination', () => {
     wrapper.setProps({
       totalRows: 4
     })
+    await waitNT(wrapper.vm)
 
     expect(wrapper.is('ul')).toBe(true)
     expect(wrapper.findAll('li').length).toBe(8)
 
     wrapper.setProps({
       perPage: 2
     })
+    await waitNT(wrapper.vm)
 
     expect(wrapper.is('ul')).toBe(true)
     expect(wrapper.findAll('li').length).toBe(6)
@@ -679,8 +681,8 @@ describe('pagination', () => {
     wrapper.destroy()
   })
 
-  it('changing the pagesize resets to page 1', async () => {
-    // https://github.com/bootstrap-vue/bootstrap-vue/issues/2987
+  it('changing the number of pages to less than current page number resets to page 1', async () => {
+    // https://github.com/bootstrap-vue/bootstrap-vue/issues/3716
     const wrapper = mount(BPagination, {
       propsData: {
         totalRows: 10,
@@ -694,16 +696,79 @@ describe('pagination', () => {
     expect(wrapper.vm.currentPage).toBe(10)
     expect(wrapper.emitted('input')).not.toBeDefined()
 
+    // Change total rows to larger value. Should not change page number
+    wrapper.setProps({
+      totalRows: 20
+    })
+    await waitNT(wrapper.vm)
+    expect(wrapper.vm.currentPage).toBe(10)
+    expect(wrapper.emitted('input')).not.toBeDefined()
+
+    // Change to page 20
+    wrapper.setProps({
+      value: 20
+    })
+    await waitNT(wrapper.vm)
+    expect(wrapper.vm.currentPage).toBe(20)
+    expect(wrapper.emitted('input')).toBeDefined()
+    expect(wrapper.emitted('input').length).toBe(1)
+    expect(wrapper.emitted('input')[0][0]).toBe(20)
+
+    // Decrease number of pages should reset to page 1
+    wrapper.setProps({
+      totalRows: 10
+    })
+    await waitNT(wrapper.vm)
+    expect(wrapper.vm.currentPage).toBe(1)
+    expect(wrapper.emitted('input').length).toBe(2)
+    expect(wrapper.emitted('input')[1][0]).toBe(1)
+
+    // Change to page 3
+    wrapper.setProps({
+      value: 3
+    })
+    await waitNT(wrapper.vm)
+    expect(wrapper.vm.currentPage).toBe(3)
+    expect(wrapper.emitted('input').length).toBe(3)
+    expect(wrapper.emitted('input')[2][0]).toBe(3)
+
+    // Decrease number of pages to 5 should not reset to page 1
+    wrapper.setProps({
+      totalRows: 5
+    })
+    await waitNT(wrapper.vm)
+    expect(wrapper.vm.currentPage).toBe(3)
+    expect(wrapper.emitted('input').length).toBe(3)
+
+    wrapper.destroy()
+  })
+
+  it('changing per-page resets to page 1', async () => {
+    // https://github.com/bootstrap-vue/bootstrap-vue/issues/2987
+    const wrapper = mount(BPagination, {
+      propsData: {
+        totalRows: 10,
+        perPage: 1,
+        value: 4,
+        limit: 20
+      }
+    })
+    expect(wrapper.isVueInstance()).toBe(true)
+
+    expect(wrapper.vm.currentPage).toBe(4)
+    expect(wrapper.emitted('input')).not.toBeDefined()
+
+    // Change perPage
     wrapper.setProps({
-      perPage: 3
+      perPage: 2
     })
     await waitNT(wrapper.vm)
     expect(wrapper.vm.currentPage).toBe(1)
     expect(wrapper.emitted('input')).toBeDefined()
     expect(wrapper.emitted('input').length).toBe(1)
     expect(wrapper.emitted('input')[0][0]).toBe(1)
 
-    // Change to page 3
+    // Change page to 3
     wrapper.setProps({
       value: 3
     })
@@ -712,7 +777,8 @@ describe('pagination', () => {
     expect(wrapper.emitted('input').length).toBe(2)
     expect(wrapper.emitted('input')[1][0]).toBe(3)
 
-    // Increasing number of pages should reset to page 1
+    // Change perPage. Should reset to page 1, even though
+    // current page is within range of numberOfPages
     wrapper.setProps({
       perPage: 1
     })
