Fix link list to follow with a11y practise (#2098)

* fix link list to follow with a11y practise
SalesforceCommerceCloud · Oct 28, 2024 · 17b9e0c · 17b9e0c
1 parent 7fff71d
commit 17b9e0c
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 27 deletions.
diff --git a/packages/template-retail-react-app/CHANGELOG.md b/packages/template-retail-react-app/CHANGELOG.md
@@ -1,3 +1,8 @@
+## v5.1.0 (TBD)
+
+### Accessibility Improvements
+- [a11y] Fix LinkList component to follow a11y practise [#2098])(https://github.com/SalesforceCommerceCloud/pwa-kit/pull/2098)
+
 ## v5.0.0 (TBD)
 
 ### New Features

diff --git a/packages/template-retail-react-app/app/components/links-list/index.jsx b/packages/template-retail-react-app/app/components/links-list/index.jsx
@@ -11,7 +11,6 @@ import {
     List,
     ListItem,
     Heading,
-    HStack,
     useMultiStyleConfig
 } from '@salesforce/retail-react-app/app/components/shared/ui'
 import Link from '@salesforce/retail-react-app/app/components/link'
@@ -44,24 +43,24 @@ const LinksList = ({
                     <Heading {...styles.heading}>{heading}</Heading>
                 ))}
 
-            {links && (
-                <List spacing={5} {...styles.list}>
-                    {variant === 'horizontal' ? (
-                        <HStack>
-                            {links.map((link, i) => (
-                                <ListItem key={i} {...styles.listItem} sx={styles.listItemSx}>
-                                    <Link
-                                        to={link.href}
-                                        onClick={onLinkClick}
-                                        {...(link.styles ? link.styles : {})}
-                                    >
-                                        {link.text}
-                                    </Link>
-                                </ListItem>
-                            ))}
-                        </HStack>
-                    ) : (
-                        links.map((link, i) => (
+            {links &&
+                (variant === 'horizontal' ? (
+                    <List {...styles.list} data-testid="horizontal-list">
+                        {links.map((link, i) => (
+                            <ListItem key={i} {...styles.listItem} sx={styles.listItemSx}>
+                                <Link
+                                    to={link.href}
+                                    onClick={onLinkClick}
+                                    {...(link.styles ? link.styles : {})}
+                                >
+                                    {link.text}
+                                </Link>
+                            </ListItem>
+                        ))}
+                    </List>
+                ) : (
+                    <List spacing={5} {...styles.list}>
+                        {links.map((link, i) => (
                             <ListItem key={i}>
                                 <Link
                                     to={link.href}
@@ -71,10 +70,9 @@ const LinksList = ({
                                     {link.text}
                                 </Link>
                             </ListItem>
-                        ))
-                    )}
-                </List>
-            )}
+                        ))}
+                    </List>
+                ))}
         </Box>
     )
 }

diff --git a/packages/template-retail-react-app/app/components/links-list/index.test.js b/packages/template-retail-react-app/app/components/links-list/index.test.js
@@ -21,7 +21,7 @@ const links = [
         text: 'Privacy Policy'
     }
 ]
-const horizontalVariantSelector = 'ul > .chakra-stack > li'
+const horizontalVariantSelector = 'ul > li'
 
 const FooterStylesProvider = ({children}) => {
     const styles = useMultiStyleConfig('Footer')
@@ -41,7 +41,7 @@ test('renders LinksList with default arguments', () => {
     expect(screen.getAllByRole('listitem')).toHaveLength(1)
     expect(screen.getByRole('link', {name: links[0].text})).toBeInTheDocument()
     expect(screen.queryByRole('heading')).toBeNull()
-    expect(document.querySelector(horizontalVariantSelector)).toBeNull()
+    expect(screen.queryByTestId('horizontal-list')).toBeNull()
 })
 
 test('renders LinksList with heading', () => {
@@ -55,11 +55,12 @@ test('renders LinksList with heading', () => {
 })
 
 test('renders LinksList with horizontal variant', () => {
-    renderWithProviders(
+    const {container} = renderWithProviders(
         <FooterStylesProvider>
             <LinksList links={links} variant="horizontal" />
         </FooterStylesProvider>
     )
+    screen.logTestingPlaygroundURL()
 
-    expect(document.querySelector(horizontalVariantSelector)).toBeInTheDocument()
+    expect(container.querySelector(horizontalVariantSelector)).toBeInTheDocument()
 })
diff --git a/packages/template-retail-react-app/app/theme/components/project/links-list.js b/packages/template-retail-react-app/app/theme/components/project/links-list.js
@@ -25,6 +25,11 @@ export default {
     variants: {
         vertical: {},
         horizontal: {
+            list: {
+                display: 'flex',
+                flexDirection: 'row',
+                gap: '0.5rem'
+            },
             listItem: {
                 borderLeft: '1px solid',
                 paddingLeft: 2