TablePaginator fix keyboard navigation out of sync (#2318)

iTwin · Oct 29, 2024 · f4cb894 · f4cb894
1 parent 07ff58c
commit f4cb894
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 2 deletions.
diff --git a/packages/itwinui-react/src/core/Table/TablePaginator.tsx b/packages/itwinui-react/src/core/Table/TablePaginator.tsx
@@ -275,6 +275,7 @@ export const TablePaginator = (props: TablePaginatorProps) => {
               <TablePaginatorPageButtons
                 size={size}
                 focusedIndex={focusedIndex}
+                setFocusedIndex={setFocusedIndex}
                 totalPagesCount={totalPagesCount}
                 onPageChange={onPageChange}
                 currentPage={currentPage}
@@ -343,13 +344,15 @@ type TablePaginatorPageButtonsProps = Pick<
 > &
   Required<Pick<TablePaginatorProps, 'localization' | 'size' | 'isLoading'>> & {
     focusedIndex: number;
+    setFocusedIndex: React.Dispatch<React.SetStateAction<number>>;
     totalPagesCount: number;
     currentPage: number;
   };
 
 const TablePaginatorPageButtons = (props: TablePaginatorPageButtonsProps) => {
   const {
     focusedIndex,
+    setFocusedIndex,
     totalPagesCount,
     onPageChange,
     currentPage,
@@ -370,15 +373,25 @@ const TablePaginatorPageButtons = (props: TablePaginatorPageButtonsProps) => {
         styleType='borderless'
         size={buttonSize}
         data-iui-active={index === currentPage}
-        onClick={() => onPageChange(index)}
+        onClick={() => {
+          setFocusedIndex(index);
+          onPageChange(index);
+        }}
         aria-current={index === currentPage}
         aria-label={localization.goToPageLabel?.(index + 1)}
         tabIndex={tabIndex}
       >
         {index + 1}
       </Button>
     ),
-    [focusedIndex, currentPage, localization, buttonSize, onPageChange],
+    [
+      focusedIndex,
+      buttonSize,
+      currentPage,
+      localization,
+      setFocusedIndex,
+      onPageChange,
+    ],
   );
 
   const pageList = React.useMemo(

diff --git a/testing/e2e/app/routes/Table/spec.ts b/testing/e2e/app/routes/Table/spec.ts
@@ -638,6 +638,44 @@ test.describe('Table Paginator', () => {
     await page.waitForTimeout(300);
   });
 
+  test('should handle focus of page buttons when focusActivationMode is manual', async ({
+    page,
+  }) => {
+    await page.goto(
+      '/Table?exampleType=withTablePaginator&focusActivationMode=manual',
+    );
+
+    const paginatorButtons = page.locator('#paginator button', {
+      hasText: /[0-9]+/,
+    });
+
+    await paginatorButtons.nth(8).click();
+    await expect(paginatorButtons.nth(8)).toBeFocused();
+
+    // Confirm that arrow keys move focus from _previously focused page_.
+    await page.keyboard.press('ArrowRight');
+    await expect(paginatorButtons.nth(9)).toBeFocused();
+    await expect(paginatorButtons.nth(1)).not.toBeFocused();
+
+    // Clicking current page itself focuses that page button.
+    await paginatorButtons.nth(8).click();
+    await expect(paginatorButtons.nth(8)).toBeFocused();
+
+    // Confirm that arrow keys move focus from _previously focused page_.
+    await page.keyboard.press('ArrowRight');
+    await expect(paginatorButtons.nth(9)).toBeFocused();
+    await expect(paginatorButtons.nth(10)).not.toBeFocused();
+
+    // Clicking non-current page focuses that page button.
+    await paginatorButtons.nth(4).click();
+    await expect(paginatorButtons.nth(4)).toBeFocused();
+
+    // Confirm that arrow keys move focus from _previously focused page_.
+    await page.keyboard.press('ArrowLeft');
+    await expect(paginatorButtons.nth(3)).toBeFocused();
+    await expect(paginatorButtons.nth(8)).not.toBeFocused();
+  });
+
   //#region Helpers for table paginator tests
   const setContainerSize = async (page: Page, value: string | undefined) => {
     await page.locator('#container').evaluate(