fix(Pagination): use empty li for ellipsis (#2942)

- Using empty `li` for ellipsis --------- Co-authored-by: Tobias Barsnes <[email protected]> Co-authored-by: Michael Marszalek <[email protected]>
digdir · Jan 7, 2025 · d4c1ddb · d4c1ddb
1 parent e6e5410
commit d4c1ddb
Show file tree

Hide file tree

Showing 10 changed files with 69 additions and 45 deletions.
diff --git a/.changeset/bright-eyes-beam.md b/.changeset/bright-eyes-beam.md
@@ -0,0 +1,5 @@
+---
+"@digdir/designsystemet-css": patch
+---
+
+Button: add `height: fit-content`
diff --git a/.changeset/cold-seals-draw.md b/.changeset/cold-seals-draw.md
@@ -0,0 +1,6 @@
+---
+"@digdir/designsystemet-css": patch
+"@digdir/designsystemet-react": patch
+---
+
+Pagination: Use empty `li` for ellipsis
diff --git a/apps/_components/src/Showcase/Showcase.tsx b/apps/_components/src/Showcase/Showcase.tsx
@@ -137,7 +137,14 @@ export function Showcase({ className, ...props }: ShowcaseProps) {
             </Pagination.Item>
             {pagination.pages.map(({ itemKey, buttonProps, page }) => (
               <Pagination.Item key={itemKey}>
-                <Pagination.Button {...buttonProps}>{page}</Pagination.Button>
+                {typeof page === 'number' && (
+                  <Pagination.Button
+                    {...buttonProps}
+                    aria-label={`Side ${page}`}
+                  >
+                    {page}
+                  </Pagination.Button>
+                )}
               </Pagination.Item>
             ))}
             <Pagination.Item>

diff --git a/packages/css/src/button.css b/packages/css/src/button.css
@@ -51,6 +51,7 @@
   font-family: inherit;
   font-weight: var(--ds-font-weight-medium);
   gap: var(--dsc-button-gap);
+  height: fit-content; /* If placed in flex container */
   justify-content: center;
   line-height: var(--ds-line-height-sm);
   min-height: var(--dsc-button-size);

diff --git a/packages/css/src/pagination.css b/packages/css/src/pagination.css
@@ -6,19 +6,21 @@
 
   & > :is(ol, ul) {
     display: flex;
+    align-items: center;
     flex-wrap: wrap;
     gap: var(--dsc-pagination-gap);
     list-style: none;
     margin: 0;
     padding: 0;
 
     & > li:first-child > ::before,
-    & > li:last-child > ::after {
+    & > li:last-child > ::before {
       content: '';
       background: currentcolor;
       height: var(--dsc-pagination-chevron-size);
       mask: center / contain no-repeat var(--dsc-pagination-icon-url);
       width: var(--dsc-pagination-chevron-size);
+      order: 1;
 
       @media (forced-colors: active) {
         background: LinkText;
@@ -27,21 +29,15 @@
 
     & > li:first-child > ::before {
       rotate: 180deg;
+      order: 0;
     }
 
     /* Style as non-interactive ellipsis when empty */
-    & > li > :empty {
-      color: inherit;
-      padding: 0; /* Make ellipsis element square */
-      pointer-events: none;
-
-      &::before {
-        content: '\2026'; /* ellipsis */
-      }
+    & > li:empty::before {
+      content: '\2026'; /* ellipsis */
+      display: block;
+      min-width: var(--ds-size-12);
+      text-align: center;
     }
   }
-
-  &[data-compact] {
-    --dsc-pagination-gap: var(--ds-size-0);
-  }
 }
diff --git a/packages/react/src/components/Pagination/Pagination.mdx b/packages/react/src/components/Pagination/Pagination.mdx
@@ -49,9 +49,11 @@ const { pages, prevButtonProps, nextButtonProps, hasNext, hasPrevious } = usePag
     </Pagination.Item>
     {pages.map(({ page, itemKey, buttonProps }) => (
       <Pagination.Item key={itemKey}>
-        <Pagination.Button {...buttonProps} aria-label={`Side ${page}`}>
-          {page}
-        </Pagination.Button>
+        {typeof page === 'number' && (
+          <Pagination.Button {...buttonProps} aria-label={`Side ${page}`}>
+            {page}
+          </Pagination.Button>
+        )}
       </Pagination.Item>
     ))}
     <Pagination.Item>

diff --git a/packages/react/src/components/Pagination/Pagination.stories.tsx b/packages/react/src/components/Pagination/Pagination.stories.tsx
@@ -1,6 +1,7 @@
 import { useArgs } from '@storybook/preview-api';
 import type { Meta, StoryFn } from '@storybook/react';
 
+import { useState } from 'react';
 import { Pagination, type UsePaginationProps, usePagination } from '.';
 
 export default {
@@ -9,13 +10,12 @@ export default {
 } as Meta;
 
 export const Preview: StoryFn<typeof Pagination> = (args) => {
-  const [, updateArgs] = useArgs();
+  const [page, setCurrentPage] = useState(4);
   const { pages, nextButtonProps, prevButtonProps } = usePagination({
-    currentPage: 4,
-    onChange: console.log,
+    currentPage: page,
     totalPages: 10,
     showPages: 7,
-    setCurrentPage: (currentPage) => updateArgs({ currentPage }),
+    setCurrentPage,
   });
 
   return (
@@ -28,9 +28,11 @@ export const Preview: StoryFn<typeof Pagination> = (args) => {
         </Pagination.Item>
         {pages.map(({ page, itemKey, buttonProps }) => (
           <Pagination.Item key={itemKey}>
-            <Pagination.Button {...buttonProps} aria-label={`Side ${page}`}>
-              {page}
-            </Pagination.Button>
+            {typeof page === 'number' && (
+              <Pagination.Button {...buttonProps} aria-label={`Side ${page}`}>
+                {page}
+              </Pagination.Button>
+            )}
           </Pagination.Item>
         ))}
         <Pagination.Item>
@@ -68,13 +70,15 @@ export const WithAnchor: StoryFn<UsePaginationProps> = (args) => {
         </Pagination.Item>
         {pages.map(({ page, itemKey, buttonProps }) => (
           <Pagination.Item key={itemKey}>
-            <Pagination.Button
-              asChild
-              aria-label={`Side ${page}`}
-              {...buttonProps}
-            >
-              <a href={`#side-${page}`}>{page}</a>
-            </Pagination.Button>
+            {typeof page === 'number' && (
+              <Pagination.Button
+                asChild
+                aria-label={`Side ${page}`}
+                {...buttonProps}
+              >
+                <a href={`#side-${page}`}>{page}</a>
+              </Pagination.Button>
+            )}
           </Pagination.Item>
         ))}
         <Pagination.Item>

diff --git a/packages/react/src/components/Pagination/Pagination.test.tsx b/packages/react/src/components/Pagination/Pagination.test.tsx
@@ -25,9 +25,7 @@ const renderWithRoot = (props: PaginationProps) => {
         <Pagination.Item>
           <Pagination.Button>3</Pagination.Button>
         </Pagination.Item>
-        <Pagination.Item>
-          <Pagination.Button />
-        </Pagination.Item>
+        <Pagination.Item />
         <Pagination.Item>
           <Pagination.Button>6</Pagination.Button>
         </Pagination.Item>

diff --git a/packages/react/src/components/Pagination/usePagination.ts b/packages/react/src/components/Pagination/usePagination.ts
@@ -62,15 +62,15 @@ export const usePagination = ({
       /** Number of steps */
       pages: getSteps(currentPage, totalPages, showPages).map(
         (page, index) => ({
-          page: page || '',
+          page: page || 'ellipsis',
           itemKey: page ? `page-${page}` : `ellipsis-${index}`, // React key utility
-          buttonProps: {
-            'aria-current': page === currentPage ? 'page' : undefined,
-            'aria-hidden': !page || undefined, // Hide ellipsis from screen reader
-            onClick: handleClick(page),
-            tabIndex: page ? undefined : -1, // Hide ellipsis keyboard
-            variant: page === currentPage ? 'primary' : 'tertiary',
-          } as PaginationButtonProps,
+          buttonProps: (page
+            ? {
+                'aria-current': page === currentPage ? 'page' : undefined,
+                onClick: handleClick(page),
+                variant: page === currentPage ? 'primary' : 'tertiary',
+              }
+            : null) as PaginationButtonProps | null,
         }),
       ),
       /** Properties to spread on Pagination.Button used for previous naviagation */

diff --git a/packages/react/stories/testing.stories.tsx b/packages/react/stories/testing.stories.tsx
@@ -363,9 +363,14 @@ export const Sizes: StoryFn = () => {
             </Pagination.Item>
             {pages.map(({ page, itemKey, buttonProps }) => (
               <Pagination.Item key={itemKey}>
-                <Pagination.Button {...buttonProps} aria-label={`Side ${page}`}>
-                  {page}
-                </Pagination.Button>
+                {typeof page === 'number' && (
+                  <Pagination.Button
+                    {...buttonProps}
+                    aria-label={`Side ${page}`}
+                  >
+                    {page}
+                  </Pagination.Button>
+                )}
               </Pagination.Item>
             ))}
             <Pagination.Item>