fix(shared-types,ui-pagination): pagination indicators have spacing and coded as a list for a11y

Author: balzss

packages/shared-types/src/ComponentThemeVariables.ts

@@ -949,6 +949,10 @@ export type PaginationPageInputTheme = {
   inputWidth: string
 }
 
+export type PaginationTheme = {
+  pageIndicatorGap: Spacing['xSmall']
+}
+
 export type PillTheme = {
   fontFamily: Typography['fontFamily']
   padding: string | 0

packages/ui-pagination/src/Pagination/PaginationButton/index.tsx

@@ -59,17 +59,20 @@ class PaginationButton extends Component<PaginationPageProps> {
 
     const props = omitProps(this.props, PaginationButton.allowedProps, exclude)
 
+    // wrapped in an unstyled <li> for better a11y
     return (
-      <BaseButton
-        color="primary"
-        withBackground={this.props.current}
-        withBorder={this.props.current}
-        {...props}
-        aria-current={this.props.current ? 'page' : undefined}
-        elementRef={this.handleRef}
-      >
-        {this.props.children}
-      </BaseButton>
+      <li style={{ all: 'unset' }}>
+        <BaseButton
+          color="primary"
+          withBackground={this.props.current}
+          withBorder={this.props.current}
+          {...props}
+          aria-current={this.props.current ? 'page' : undefined}
+          elementRef={this.handleRef}
+        >
+          {this.props.children}
+        </BaseButton>
+      </li>
     )
   }
 }

packages/ui-pagination/src/Pagination/__new-tests__/Pagination.test.tsx

@@ -123,7 +123,9 @@ describe('<Pagination />', () => {
     it('by default', async () => {
       const { container } = render(
         <Pagination variant="compact" labelNext="Next" labelPrev="Prev">
-          {buildPages(5)}
+          <ul>
+            {buildPages(5)}
+          </ul>
         </Pagination>
       )
 
@@ -134,7 +136,7 @@ describe('<Pagination />', () => {
     it('by default with more pages', async () => {
       const { container } = render(
         <Pagination variant="compact" labelNext="Next" labelPrev="Prev">
-          {buildPages(8)}
+          <ul>{buildPages(8)}</ul>
         </Pagination>
       )
       const axeCheck = await runAxeCheck(container)
@@ -151,7 +153,7 @@ describe('<Pagination />', () => {
           labelLast="Last"
           withFirstAndLastButton
         >
-          {buildPages(8)}
+          <ul>{buildPages(8)}</ul>
         </Pagination>
       )
       const axeCheck = await runAxeCheck(container)
@@ -169,7 +171,7 @@ describe('<Pagination />', () => {
           withFirstAndLastButton
           showDisabledButtons
         >
-          {buildPages(8)}
+          <ul>{buildPages(8)}</ul>
         </Pagination>
       )
       const axeCheck = await runAxeCheck(container)

packages/ui-pagination/src/Pagination/index.tsx

@@ -36,6 +36,7 @@ import { PaginationArrowButton } from './PaginationArrowButton'
 import { PaginationPageInput } from './PaginationPageInput'
 
 import generateStyle from './styles'
+import generateComponentTheme from './theme'
 
 import type { PaginationPageProps } from './PaginationButton/props'
 import type { PaginationArrowDirections } from './PaginationArrowButton/props'
@@ -80,7 +81,7 @@ category: components
 ---
 **/
 @withDeterministicId()
-@withStyle(generateStyle, null)
+@withStyle(generateStyle, generateComponentTheme)
 @testable()
 class Pagination extends Component<PaginationProps> {
   static readonly componentId = 'Pagination'
@@ -103,7 +104,7 @@ class Pagination extends Component<PaginationProps> {
     currentPage: 1,
     siblingCount: 1,
     boundaryCount: 1,
-    ellipsis: '...',
+    ellipsis: <li style={{all: 'unset'}}>...</li>,
     renderPageIndicator: (page: number) => page
   }
 
@@ -408,7 +409,7 @@ class Pagination extends Component<PaginationProps> {
         )
       )
     }
-    return pages
+    return (<ul css={this.props.styles?.pageIndicatorList}>{pages}</ul>)
   }
 
   renderPages(currentPageIndex: number) {

packages/ui-pagination/src/Pagination/props.ts

@@ -197,7 +197,9 @@ type PaginationProps = PaginationOwnProps &
   OtherHTMLAttributes<PaginationOwnProps> &
   WithDeterministicIdProps
 
-type PaginationStyle = ComponentStyle<'pagination' | 'pages'>
+type PaginationStyle = ComponentStyle<
+  'pagination' | 'pages' | 'pageIndicatorList'
+>
 
 const propTypes: PropValidators<PropKeys> = {
   children: Children.oneOf([PaginationButton]),

packages/ui-pagination/src/Pagination/styles.ts

@@ -22,6 +22,7 @@
  * SOFTWARE.
  */
 
+import type { PaginationTheme } from '@instructure/shared-types'
 import type { PaginationStyle } from './props'
 
 /**
@@ -34,8 +35,14 @@ import type { PaginationStyle } from './props'
  * @param  {Object} state the state of the component, the style is applied to
  * @return {Object} The final style object, which will be used in the component
  */
-const generateStyle = (): PaginationStyle => {
+const generateStyle = (componentTheme: PaginationTheme): PaginationStyle => {
   return {
+    pageIndicatorList: {
+      all: 'unset',
+      display: 'flex',
+      alignItems: 'center',
+      gap: componentTheme.pageIndicatorGap
+    },
     pagination: {
       label: 'pagination',
       textAlign: 'center'

packages/ui-pagination/src/Pagination/theme.ts

@@ -0,0 +1,45 @@
+/*
+ * The MIT License (MIT)
+ *
+ * Copyright (c) 2015 - present Instructure, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in all
+ * copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+import type { Theme } from '@instructure/ui-themes'
+import { PaginationTheme } from '@instructure/shared-types'
+
+/**
+ * Generates the theme object for the component from the theme and provided additional information
+ * @param  {Object} theme The actual theme object.
+ * @return {Object} The final theme object with the overrides and component variables
+ */
+const generateComponentTheme = (theme: Theme): PaginationTheme => {
+  const { spacing } = theme
+
+  const componentVariables: PaginationTheme = {
+    pageIndicatorGap: spacing.xSmall
+  }
+
+  return {
+    ...componentVariables
+  }
+}
+
+export default generateComponentTheme