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

Change all ARIA labels to translatable messages #1572

Merged
merged 22 commits into from
Dec 8, 2023
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
3117335
Update field component aria label to translatable message.
wjhsf Nov 28, 2023
03a34d2
Convert account icon to more accessible button.
wjhsf Nov 28, 2023
9b75ed6
Convert my account menu aria-label to translatable message.
wjhsf Nov 28, 2023
2d79b4f
Rename account menu vars for clarity.
wjhsf Nov 28, 2023
c12e799
a11y: add space key handler
wjhsf Nov 28, 2023
f2c5bdc
Improve accessibility of popup account menu.
wjhsf Nov 28, 2023
1effb7c
Convert list menu aria label to translatable message.
wjhsf Nov 28, 2023
e3805b6
Convert pagination button aria labels to translatable messages.
wjhsf Nov 28, 2023
eb95631
Convert carousel button aria labels to translatable messages.
wjhsf Nov 28, 2023
4e91034
Update translations files.
wjhsf Nov 28, 2023
f50ad10
Update changelog.
wjhsf Nov 28, 2023
5a38b5f
fix tests
wjhsf Nov 28, 2023
4c1cafa
Merge branch 'develop' into translatable-aria-label
wjhsf Nov 29, 2023
59d6735
remove rogue console
wjhsf Nov 29, 2023
bd4adf8
Use more helpful ARIA label.
wjhsf Nov 29, 2023
794a894
Fix login detection logic.
wjhsf Nov 29, 2023
21693dd
Update changelog with `withRegistration` change.
wjhsf Nov 29, 2023
0b5e2e4
lint fixes
wjhsf Nov 29, 2023
40c5025
Merge branch 'develop' into translatable-aria-label
wjhsf Nov 30, 2023
222164d
Merge branch 'develop' into translatable-aria-label
wjhsf Dec 4, 2023
9ce0b27
Merge branch 'develop' into translatable-aria-label
wjhsf Dec 4, 2023
ad4330e
Merge branch 'develop' into translatable-aria-label
wjhsf Dec 6, 2023
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
2 changes: 2 additions & 0 deletions packages/template-retail-react-app/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
## v2.2.0-dev (Nov 3, 2023)

- Replace max-age with s-maxage to only cache shared caches [#1564](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/1564)

### Accessibility Improvements
Expand All @@ -12,6 +13,7 @@
- Make security code tooltip receive keyboard focus [#1551](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/1551)
- Improve accessibility of quantity picker [#1552](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/1552)
- Improve keyboard accessibility of product scroller [#1559](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/1559)
- Improve keyboard accessibility of account menu and nav bar [#1572](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/1572)

### Other Features

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -257,18 +257,13 @@ const App = (props) => {
}

const onAccountClick = () => {
// Link to account page for registered customer, open auth modal otherwise
if (isRegistered) {
const path = buildUrl('/account')
history.push(path)
} else {
// if they already are at the login page, do not show login modal
Copy link
Collaborator

Choose a reason for hiding this comment

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

This was a bug reported from UX awhile ago saying if a user is current in login page, the account click should not show the modal since user is already seeing the login form

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That bug is also handled by the withRegistration component. Although I noticed that here the logic handled /login, and the withRegistration component handles /{locale}/login, but what the app actually uses is /global/{locale}/login. I've changed the withRegistration component to be more flexible.

if (new RegExp(`^/login$`).test(location.pathname)) return
authModal.onOpen()
}
// Link to account page if registered; Header component will show auth modal for guest users
const path = buildUrl('/account')
history.push(path)
}

const onWishlistClick = () => {
// Link to wishlist page if registered; Header component will show auth modal for guest users
const path = buildUrl('/account/wishlist')
history.push(path)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
Checkbox
} from '@salesforce/retail-react-app/app/components/shared/ui'
import {VisibilityIcon, VisibilityOffIcon} from '@salesforce/retail-react-app/app/components/icons'
import {useIntl} from 'react-intl'

const Field = ({
name,
Expand All @@ -35,8 +36,18 @@ const Field = ({
helpText,
children
}) => {
const intl = useIntl()
const [hidePassword, setHidePassword] = useState(true)
const PasswordIcon = hidePassword ? VisibilityIcon : VisibilityOffIcon
const passwordIconLabel = hidePassword
? intl.formatMessage({
id: 'field.password.assistive_msg.show_password',
defaultMessage: 'Show password'
})
: intl.formatMessage({
id: 'field.password.assistive_msg.hide_password',
defaultMessage: 'Hide password'
})
const inputType =
type === 'password' && hidePassword ? 'password' : type === 'password' ? 'text' : type
return (
Expand All @@ -51,8 +62,7 @@ const Field = ({

return (
<FormControl id={name} isInvalid={error}>
{!['checkbox', 'radio'].includes(type) &&
type !== 'hidden' &&
{!['checkbox', 'radio', 'hidden'].includes(type) &&
(formLabel || <FormLabel>{label}</FormLabel>)}

<InputGroup>
Expand Down Expand Up @@ -83,7 +93,7 @@ const Field = ({
<InputRightElement>
<IconButton
variant="ghosted"
aria-label="Show password"
aria-label={passwordIconLabel}
icon={<PasswordIcon color="gray.500" boxSize={6} />}
onClick={() => setHidePassword(!hidePassword)}
/>
Expand Down Expand Up @@ -120,7 +130,7 @@ const Field = ({
{children}
</InputGroup>

{error && !type !== 'hidden' && (
{error && type !== 'hidden' && (
<FormErrorMessage color="red.600">{error.message}</FormErrorMessage>
)}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,6 @@ import useNavigation from '@salesforce/retail-react-app/app/hooks/use-navigation
import LoadingSpinner from '@salesforce/retail-react-app/app/components/loading-spinner'
import {isHydrated, noop} from '@salesforce/retail-react-app/app/utils/utils'

const ENTER_KEY = 'Enter'

const IconButtonWithRegistration = withRegistration(IconButton)
/**
* The header is the main source for accessing
Expand Down Expand Up @@ -86,7 +84,13 @@ const Header = ({
const {isRegistered} = useCustomerType()
const logout = useAuthHelper(AuthHelpers.Logout)
const navigate = useNavigation()
const {isOpen, onClose, onOpen} = useDisclosure()
const {
getButtonProps: getAccountMenuButtonProps,
getDisclosureProps: getAccountMenuDisclosureProps,
isOpen: isAccountMenuOpen,
onClose: onAccountMenuClose,
onOpen: onAccountMenuOpen
} = useDisclosure()
const [isDesktop] = useMediaQuery('(min-width: 992px)')

const [showLoading, setShowLoading] = useState(false)
Expand All @@ -103,15 +107,10 @@ const Header = ({
setShowLoading(false)
}

const keyMap = {
Escape: () => onClose(),
Enter: () => onOpen()
}

const handleIconsMouseLeave = () => {
// don't close the menu if users enter the popover content
setTimeout(() => {
if (!hasEnterPopoverContent.current) onClose()
if (!hasEnterPopoverContent.current) onAccountMenuClose()
}, 100)
}

Expand Down Expand Up @@ -154,51 +153,51 @@ const Header = ({
{...styles.search}
/>
</Box>
<AccountIcon
{...styles.accountIcon}
tabIndex={0}
onMouseOver={isDesktop ? onOpen : noop}
onKeyDown={(e) => {
e.key === ENTER_KEY ? onMyAccountClick() : noop
}}
onClick={onMyAccountClick}
<IconButtonWithRegistration
icon={<AccountIcon {...styles.accountIcon} />}
aria-label={intl.formatMessage({
id: 'header.button.assistive_msg.my_account',
defaultMessage: 'My account'
})}
variant="unstyled"
onClick={onMyAccountClick}
onMouseOver={isDesktop ? onAccountMenuOpen : noop}
/>

{isRegistered && isHydrated() && (
<Popover
isLazy
arrowSize={15}
isOpen={isOpen}
isOpen={isAccountMenuOpen}
placement="bottom-end"
onClose={onClose}
onOpen={onOpen}
onClose={onAccountMenuClose}
onOpen={onAccountMenuOpen}
>
<PopoverTrigger>
<ChevronDownIcon
aria-label="My account trigger"
<IconButton
aria-label={intl.formatMessage({
id: 'header.button.assistive_msg.my_account_menu',
defaultMessage: 'Open account menu'
})}
icon={<ChevronDownIcon {...styles.arrowDown} />}
variant="unstyled"
{...getAccountMenuButtonProps()}
onMouseOver={onAccountMenuOpen}
onMouseLeave={handleIconsMouseLeave}
onKeyDown={(e) => {
keyMap[e.key]?.(e)
}}
{...styles.arrowDown}
onMouseOver={onOpen}
tabIndex={0}
onClick={(e) => console.log('click', e.target)}
wjhsf marked this conversation as resolved.
Show resolved Hide resolved
/>
</PopoverTrigger>

<PopoverContent
{...styles.popoverContent}
onMouseLeave={() => {
hasEnterPopoverContent.current = false
onClose()
onAccountMenuClose()
}}
onMouseOver={() => {
hasEnterPopoverContent.current = true
}}
{...getAccountMenuDisclosureProps()}
>
<PopoverArrow />
<PopoverHeader>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,11 @@ test('renders Header', async () => {
renderWithProviders(<Header />)

await waitFor(() => {
const menu = document.querySelector('button[aria-label="Menu"]')
const logo = document.querySelector('button[aria-label="Logo"]')
const account = document.querySelector('svg[aria-label="My account"]')
const cart = document.querySelector('button[aria-label="My cart, number of items: 0"]')
const wishlist = document.querySelector('button[aria-label="Wishlist"]')
const menu = screen.getByLabelText('Menu')
const logo = screen.getByLabelText('Logo')
const account = screen.getByLabelText('My account')
const cart = screen.getByLabelText('My cart, number of items: 0')
const wishlist = screen.getByLabelText('Wishlist')
const searchInput = document.querySelector('input[type="search"]')
expect(menu).toBeInTheDocument()
expect(logo).toBeInTheDocument()
Expand All @@ -91,10 +91,10 @@ test('renders Header with event handlers', async () => {
/>
)
await waitFor(() => {
const menu = document.querySelector('button[aria-label="Menu"]')
const logo = document.querySelector('button[aria-label="Logo"]')
const account = document.querySelector('svg[aria-label="My account"]')
const cart = document.querySelector('button[aria-label="My cart, number of items: 0"]')
const menu = screen.getByLabelText('Menu')
const logo = screen.getByLabelText('Logo')
const account = screen.getByLabelText('My account')
const cart = screen.getByLabelText('My cart, number of items: 0')
expect(menu).toBeInTheDocument()
fireEvent.click(menu)
expect(onMenuClick).toHaveBeenCalledTimes(1)
Expand Down Expand Up @@ -124,7 +124,7 @@ test.each(testBaskets)(
renderWithProviders(<Header />)

await waitFor(() => {
const cart = document.querySelector('button[aria-label="My cart, number of items: 0"]')
const cart = screen.getByLabelText('My cart, number of items: 0')
const badge = document.querySelector(
'button[aria-label="My cart, number of items: 0"] .chakra-badge'
)
Expand All @@ -141,9 +141,7 @@ test('renders cart badge when basket is loaded', async () => {

await waitFor(() => {
// Look for badge.
const badge = document.querySelector(
'button[aria-label="My cart, number of items: 2"] .chakra-badge'
)
const badge = screen.getByLabelText('My cart, number of items: 2')
expect(badge).toBeInTheDocument()
})
})
Expand All @@ -156,10 +154,10 @@ test('route to account page when an authenticated users click on account icon',

await waitFor(() => {
// Look for account icon
const accountTrigger = document.querySelector('svg[aria-label="My account trigger"]')
const accountTrigger = screen.getByLabelText('Open account menu')
expect(accountTrigger).toBeInTheDocument()
})
const accountIcon = document.querySelector('svg[aria-label="My account"]')
const accountIcon = screen.getByLabelText('My account')
fireEvent.click(accountIcon)
await waitFor(() => {
expect(history.push).toHaveBeenCalledWith(createPathWithDefaults('/account'))
Expand All @@ -181,7 +179,7 @@ test('route to wishlist page when an authenticated users click on wishlist icon'

await waitFor(() => {
// Look for account icon
const accountTrigger = document.querySelector('svg[aria-label="My account trigger"]')
const accountTrigger = screen.getByLabelText('Open account menu')
expect(accountTrigger).toBeInTheDocument()
})
const wishlistIcon = screen.getByRole('button', {name: /wishlist/i})
Expand All @@ -207,10 +205,10 @@ test('shows dropdown menu when an authenticated users hover on the account icon'

await waitFor(() => {
// Look for account icon
const accountTrigger = document.querySelector('svg[aria-label="My account trigger"]')
const accountTrigger = screen.getByLabelText('Open account menu')
expect(accountTrigger).toBeInTheDocument()
})
const accountIcon = document.querySelector('svg[aria-label="My account"]')
const accountIcon = screen.getByLabelText('My account')
fireEvent.click(accountIcon)
await waitFor(() => {
expect(history.push).toHaveBeenCalledWith(createPathWithDefaults('/account'))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,7 @@ const ListMenu = ({root, maxColumns = MAXIMUM_NUMBER_COLUMNS}) => {
const theme = useTheme()
const {baseStyle} = theme.components.ListMenu
const [ariaBusy, setAriaBusy] = useState(true)
const intl = useIntl()

useEffect(() => {
setAriaBusy(false)
Expand All @@ -233,7 +234,10 @@ const ListMenu = ({root, maxColumns = MAXIMUM_NUMBER_COLUMNS}) => {
return (
<nav
id="list-menu"
aria-label="main"
aria-label={intl.formatMessage({
id: 'list_menu.nav.assistive_msg',
defaultMessage: 'Main navigation'
})}
aria-live="polite"
aria-busy={ariaBusy}
aria-atomic="true"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ describe('ListMenu', () => {
const categoryTrigger = screen.getByText(/Mens/i)
await user.hover(categoryTrigger)
expect(categoryTrigger).toBeInTheDocument()
expect(screen.getByRole('navigation', {name: 'main'})).toBeInTheDocument()
expect(screen.getByRole('navigation', {name: 'Main navigation'})).toBeInTheDocument()
const suit = screen.getByText(/suits/i)
expect(suit).toBeInTheDocument()
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,10 @@ const Pagination = (props) => {
// as intended, the workaround is to use the current url when its disabled.
href={prev || currentURL}
to={prev || currentURL}
aria-label="Previous Page"
aria-label={intl.formatMessage({
id: 'pagination.link.prev.assistive_msg',
defaultMessage: 'Previous Page'
})}
isDisabled={!prev}
variant="link"
>
Expand Down Expand Up @@ -102,7 +105,10 @@ const Pagination = (props) => {
// as intended, the workaround is to use the current url when its disabled.
href={next || currentURL}
to={next || currentURL}
aria-label="Next Page"
aria-label={intl.formatMessage({
id: 'pagination.link.next.assistive_msg',
defaultMessage: 'Next Page'
})}
isDisabled={!next}
variant="link"
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
import {Component, regionPropType} from '@salesforce/commerce-sdk-react/components'
import {ChevronLeftIcon, ChevronRightIcon} from '@salesforce/retail-react-app/app/components/icons'
import {useEffect} from 'react'
import {useIntl} from '@salesforce/retail-react-app/node_modules/react-intl/index'

/**
* Display child components in a carousel slider manner. Configurations include the number of
Expand All @@ -37,6 +38,7 @@ import {useEffect} from 'react'
* @returns {React.ReactElement} - Carousel component.
*/
export const Carousel = (props = {}) => {
const intl = useIntl()
const scrollRef = useRef()
const breakpoint = useBreakpoint()
const [hasOverflow, setHasOverflow] = useState(false)
Expand Down Expand Up @@ -172,11 +174,14 @@ export const Carousel = (props = {}) => {
{/* boxShadow requires !important --> https://github.com/chakra-ui/chakra-ui/issues/3553 */}
<IconButton
data-testid="carousel-nav-left"
aria-label="Scroll carousel left"
aria-label={intl.formatMessage({
id: 'carousel.button.scroll_left.assistive_msg',
defaultMessage: 'Scroll carousel left'
})}
icon={<ChevronLeftIcon color="black" />}
borderRadius="full"
colorScheme="whiteAlpha"
boxShadow={'0 3px 10px rgb(0 0 0 / 20%) !important'}
boxShadow="0 3px 10px rgb(0 0 0 / 20%) !important"
onClick={() => scroll(-1)}
/>
</Box>
Expand All @@ -191,11 +196,14 @@ export const Carousel = (props = {}) => {
{/* boxShadow requires !important --> https://github.com/chakra-ui/chakra-ui/issues/3553 */}
<IconButton
data-testid="carousel-nav-right"
aria-label="Scroll carousel right"
aria-label={intl.formatMessage({
id: 'carousel.button.scroll_right.assistive_msg',
defaultMessage: 'Scroll carousel right'
})}
icon={<ChevronRightIcon color="black" />}
borderRadius="full"
colorScheme="whiteAlpha"
boxShadow={'0 3px 10px rgb(0 0 0 / 20%) !important'}
boxShadow="0 3px 10px rgb(0 0 0 / 20%) !important"
onClick={() => scroll(1)}
/>
</Box>
Expand Down
Loading