Fix pagination issues (#273)

* Scroll to search bar when changing pages * Remove hover state for disabled page button * Improve page query param validation * Disable text selection for pagination * Update snapshots * Fix cursor flicker
chanzuckerberg · Oct 6, 2021 · 1fdaf8f · 1fdaf8f
1 parent eee9940
commit 1fdaf8f
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 11 deletions.
diff --git a/frontend/src/components/PluginSearch/PluginSearch.tsx b/frontend/src/components/PluginSearch/PluginSearch.tsx
@@ -8,6 +8,7 @@ import { ColumnLayout, Pagination } from '@/components/common';
 import { loadingStore } from '@/store/loading';
 import { searchFormStore } from '@/store/search/form.store';
 import { searchResultsStore } from '@/store/search/results.store';
+import { scrollToSearchBar } from '@/utils';
 
 import { PluginSearchBar } from './PluginSearchBar';
 import { PluginSearchControls } from './PluginSearchControls';
@@ -29,8 +30,9 @@ function updatePage(value: number) {
   // Update page value.
   searchFormStore.page += value;
 
-  // Schedule scroll for later execution. Wrap in `raf()` to remove scroll flicker.
-  requestAnimationFrame(() => window.scroll(0, document.body.scrollHeight));
+  // Scroll to top of results so that the user will see the most relevant for
+  // the current page.
+  scrollToSearchBar();
 }
 
 /**

diff --git a/frontend/src/components/common/Pagination/Pagination.tsx b/frontend/src/components/common/Pagination/Pagination.tsx
@@ -45,7 +45,7 @@ export function Pagination({
           styles.pageButton,
           'focus-visible:bg-napari-hover-gray hover:bg-napari-hover-gray',
 
-          isDisabled && 'opacity-0',
+          isDisabled && 'opacity-0 cursor-default',
           type === 'left' ? 'mr-2' : 'ml-2',
         )}
         data-testid={`pagination${upperFirst(type)}`}
@@ -59,7 +59,16 @@ export function Pagination({
   }
 
   return (
-    <nav className={clsx(className, 'flex items-center justify-center')}>
+    <nav
+      className={clsx(
+        className,
+        'flex items-center justify-center',
+
+        // Disable user selection so that the cursor doesn't flicker between the
+        // default and select pointers.
+        'select-none',
+      )}
+    >
       {renderPageButton('left')}
 
       <span className={styles.value} data-testid="paginationValue">

diff --git a/frontend/src/components/common/Pagination/__snapshots__/Pagination.test.tsx.snap b/frontend/src/components/common/Pagination/__snapshots__/Pagination.test.tsx.snap
@@ -3,10 +3,10 @@
 exports[`<Pagination /> should match snapshot 1`] = `
 <DocumentFragment>
   <nav
-    class="flex items-center justify-center"
+    class="flex items-center justify-center select-none"
   >
     <button
-      class="pageButton focus-visible:bg-napari-hover-gray hover:bg-napari-hover-gray opacity-0 mr-2"
+      class="pageButton focus-visible:bg-napari-hover-gray hover:bg-napari-hover-gray opacity-0 cursor-default mr-2"
       data-testid="paginationLeft"
       disabled=""
       type="button"

diff --git a/frontend/src/store/search/queryParameters.ts b/frontend/src/store/search/queryParameters.ts
@@ -1,4 +1,4 @@
-import { inRange, set } from 'lodash';
+import { set } from 'lodash';
 import { subscribe } from 'valtio';
 
 import { BEGINNING_PAGE, RESULTS_PER_PAGE } from '@/constants/search';
@@ -84,10 +84,15 @@ function initStateFromQueryParameters() {
       searchFormStore.search.index.length / RESULTS_PER_PAGE,
     );
 
-    searchFormStore.page =
-      Number.isNaN(page) || !inRange(page, BEGINNING_PAGE, totalPages + 1)
-        ? BEGINNING_PAGE
-        : page;
+    // If the page number is not a number or not in range, then set it to the
+    // beginning or ending page depending on which is closer.
+    if (Number.isNaN(page) || page < BEGINNING_PAGE) {
+      searchFormStore.page = BEGINNING_PAGE;
+    } else if (page > totalPages) {
+      searchFormStore.page = totalPages;
+    } else {
+      searchFormStore.page = page;
+    }
   }
 }