Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[A11y] refactor(ProductList): improve structure and accessibility #9795

Merged
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 71 additions & 49 deletions src/components/ProductList.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,15 @@
import React from "react"
import { GatsbyImage } from "gatsby-plugin-image"
import { Box, Flex, Heading, Image, useColorModeValue } from "@chakra-ui/react"
import {
Box,
Flex,
Heading,
Image,
List,
ListItem,
useColorModeValue,
VisuallyHidden,
} from "@chakra-ui/react"

import ButtonLink from "./ButtonLink"
import Translation from "./Translation"
Expand All @@ -22,10 +31,13 @@ export interface IProps {
const ProductList: React.FC<IProps> = ({ content, category }) => {
const shadow = useColorModeValue("tableBox.light", "tableBox.dark")

const ariaLabelledby = "cat-name"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just had a question abut this label. Im a little confused what "cat-name" is. Should the be "category-name", or should we actually be passing the category prop instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just an id because it is being used in two places (lines 40 and 51) for the aria-labelledby feature.

I can be clearer here in using a more verbose id, along with uppercase variable naming to denote this as a const variable 😁

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure! I pushed those changes up. This should be good to go now :)


return (
<Box width="full">
<Box boxSize="full">
<Heading
as="h3"
id={ariaLabelledby}
fontSize="2xl"
borderBottom="2px solid"
borderColor="border"
Expand All @@ -34,57 +46,67 @@ const ProductList: React.FC<IProps> = ({ content, category }) => {
>
{category}
</Heading>
{content.map(({ title, description, link, image, alt, id }, idx) => (
<Flex
key={id || idx}
color="text"
marginBottom="px"
marginTop={8}
alignItems={{ sm: "flex-start" }}
>
<Box width="5rem">
{image && (
<Image
as={GatsbyImage}
image={image}
alt={alt}
boxShadow={shadow}
borderRadius="sm"
/>
)}
</Box>
<Flex
as={List}
aria-labelledby={ariaLabelledby}
m={0}
flexDirection="column"
height="inherit"
>
{content.map(({ title, description, link, image, alt, id }, idx) => (
<Flex
paddingBottom={4}
width="full"
alignItems={{ base: "flex-start", sm: "center" }}
marginLeft={{ base: 4, sm: 6 }}
justifyContent="space-between"
borderBottom="1px solid"
borderColor="border"
flexDir={{ base: "column", sm: "row" }}
as={ListItem}
key={id || idx}
color="text"
marginBottom="px"
marginTop={8}
height="inherit"
>
<Box flexDir="column">
<Box>{title}</Box>
<Box fontSize="sm" marginBottom={0} opacity="0.6">
{description}
</Box>
<Box width="5rem">
{image && (
<Image
as={GatsbyImage}
image={image}
alt={alt}
boxShadow={shadow}
borderRadius="sm"
/>
)}
</Box>
{link && (
<ButtonLink
variant="outline"
to={link}
marginLeft={{ base: 0, sm: 8 }}
paddingY={1}
paddingX={6}
borderRadius="sm"
marginTop={{ base: 4, sm: 0 }}
>
<Translation id="page-dapps-ready-button" />
</ButtonLink>
)}
<Flex
justifyContent="space-between"
flexDir={{ base: "column", sm: "row" }}
paddingBottom={4}
width="full"
marginLeft={{ base: 4, sm: 6 }}
borderBottom="1px solid"
borderColor="border"
>
<Box flex={1}>
<Box>{title}</Box>
<Box fontSize="sm" marginBottom={0} opacity="0.6">
{description}
</Box>
</Box>
{link && (
<ButtonLink
variant="outline"
to={link}
alignSelf="center"
marginLeft={{ base: 0, sm: 8 }}
paddingY={1}
paddingX={6}
borderRadius="sm"
marginTop={{ base: 4, sm: 0 }}
>
<Translation id="page-dapps-ready-button" />
<VisuallyHidden>to {title} website</VisuallyHidden>
</ButtonLink>
)}
</Flex>
</Flex>
</Flex>
))}
))}
</Flex>
</Box>
)
}
Expand Down