Skip to content

Commit d8adbd7

Browse files
Merge pull request #765 from opentripplanner/a11y-layout-adjustments-lang
App Menu Selected Language a11y
2 parents 123c2d7 + 1ec68f3 commit d8adbd7

File tree

8 files changed

+146
-73
lines changed

8 files changed

+146
-73
lines changed

Diff for: __tests__/util/i18n.js

+62
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
/* globals describe, expect, it */
2+
3+
import { getLanguageOptions } from '../../lib/util/i18n'
4+
5+
describe('util > i18n', () => {
6+
describe('getLanguageOptions', () => {
7+
const testCases = [
8+
{
9+
expected: {
10+
'en-US': {
11+
name: 'English (US)'
12+
},
13+
fr: {
14+
name: 'French'
15+
}
16+
},
17+
input: {
18+
allLanguages: {
19+
name: 'All Languages'
20+
},
21+
'en-US': {
22+
name: 'English (US)'
23+
},
24+
fr: {
25+
name: 'French'
26+
}
27+
}
28+
},
29+
{
30+
expected: null,
31+
input: {
32+
allLanguages: {
33+
name: 'All Languages'
34+
},
35+
'en-US': {
36+
name: 'English (US)'
37+
},
38+
fr: {
39+
label: 'French'
40+
}
41+
}
42+
},
43+
{
44+
expected: null,
45+
input: {
46+
allLanguages: {
47+
name: 'All Languages'
48+
},
49+
'en-US': {
50+
name: 'English (US)'
51+
}
52+
}
53+
}
54+
]
55+
56+
testCases.forEach((testCase) => {
57+
it('should show at least two language options or null', () => {
58+
expect(getLanguageOptions(testCase.input)).toEqual(testCase.expected)
59+
})
60+
})
61+
})
62+
})

Diff for: index.css

+5
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,11 @@ input[type="text"]::-ms-clear {
121121
outline-offset: -2px;
122122
}
123123

124+
.app-menu button[aria-selected="true"],
125+
#locale-selector li[aria-selected="true"] {
126+
font-weight: bold;
127+
}
128+
124129
.skip-nav-button {
125130
color: initial;
126131
position: fixed;

Diff for: lib/components/app/app-menu-item.tsx

+3-4
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,8 @@ import React, { Component, HTMLAttributes, KeyboardEvent } from 'react'
66
interface Props extends HTMLAttributes<HTMLElement> {
77
href?: string
88
icon?: JSX.Element
9-
isDropdown?: boolean
109
onClick?: () => void
11-
subItems?: unknown[]
10+
subItems?: JSX.Element[]
1211
text: JSX.Element | string
1312
}
1413

@@ -81,7 +80,7 @@ export default class AppMenuItem extends Component<Props, State> {
8180
onKeyDown={this._handleKeyDown}
8281
{...otherProps}
8382
>
84-
<span>{icon}</span>
83+
<span aria-hidden>{icon}</span>
8584
<span>{text}</span>
8685
{subItems && (
8786
<span className="expand-menu-chevron">
@@ -91,7 +90,7 @@ export default class AppMenuItem extends Component<Props, State> {
9190
</Element>
9291
{subItems && (
9392
<AnimateHeight duration={500} height={isExpanded ? 'auto' : 0}>
94-
<div className="sub-menu-container" id={containerId}>
93+
<div className="sub-menu-container" id={containerId} role="group">
9594
{subItems}
9695
</div>
9796
</AnimateHeight>

Diff for: lib/components/app/app-menu.tsx

+22-17
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import * as callTakerActions from '../../actions/call-taker'
1717
import * as fieldTripActions from '../../actions/field-trip'
1818
import * as uiActions from '../../actions/ui'
1919
import { ComponentContext } from '../../util/contexts'
20+
import { getLanguageOptions } from '../../util/i18n'
2021
import { isModuleEnabled, Modules } from '../../util/config'
2122
import { MainPanelContent, setMainPanelContent } from '../../actions/ui'
2223
import startOver from '../util/start-over'
@@ -50,7 +51,9 @@ type menuItem = {
5051
iconType: string | JSX.Element
5152
iconUrl?: string
5253
id: string
54+
isSelected?: boolean
5355
label: string | JSX.Element
56+
lang?: string
5457
onClick?: () => void
5558
subMenuDivider: boolean
5659
}
@@ -99,7 +102,7 @@ class AppMenu extends Component<
99102
document.querySelector('main')?.focus()
100103
}
101104

102-
_addExtraMenuItems = (menuItems?: menuItem[]) => {
105+
_addExtraMenuItems = (menuItems?: menuItem[] | null) => {
103106
return (
104107
menuItems &&
105108
menuItems.map((menuItem) => {
@@ -109,7 +112,9 @@ class AppMenu extends Component<
109112
iconType,
110113
iconUrl,
111114
id,
115+
isSelected,
112116
label: configLabel,
117+
lang,
113118
onClick,
114119
subMenuDivider
115120
} = menuItem
@@ -122,6 +127,7 @@ class AppMenu extends Component<
122127

123128
return (
124129
<AppMenuItem
130+
aria-selected={isSelected || undefined}
125131
className={subMenuDivider ? 'app-menu-divider' : undefined}
126132
href={href}
127133
icon={
@@ -133,8 +139,10 @@ class AppMenu extends Component<
133139
}
134140
id={id}
135141
key={id}
142+
lang={lang}
136143
onClick={onClick}
137-
subItems={this._addExtraMenuItems(children)}
144+
role={isSelected !== undefined ? 'option' : undefined}
145+
subItems={this._addExtraMenuItems(children) || undefined}
138146
text={label}
139147
/>
140148
)
@@ -158,22 +166,19 @@ class AppMenu extends Component<
158166
toggleMailables
159167
} = this.props
160168

161-
const languageMenuItems: menuItem[] | undefined = configLanguages && [
169+
const languageOptions: Record<string, any> | null =
170+
getLanguageOptions(configLanguages)
171+
const languageMenuItems: menuItem[] | null = languageOptions && [
162172
{
163-
children: Object.keys(configLanguages)
164-
.filter((locale) => locale !== 'allLanguages')
165-
.map((locale) => ({
166-
iconType: <svg />,
167-
id: configLanguages[locale].name,
168-
label:
169-
activeLocale === locale ? (
170-
<strong>{configLanguages[locale].name}</strong>
171-
) : (
172-
configLanguages[locale].name
173-
),
174-
onClick: () => setLocale(locale),
175-
subMenuDivider: false
176-
})),
173+
children: Object.keys(languageOptions).map((locale: string) => ({
174+
iconType: <svg />,
175+
id: locale,
176+
isSelected: activeLocale === locale,
177+
label: languageOptions[locale].name,
178+
lang: locale,
179+
onClick: () => setLocale(locale),
180+
subMenuDivider: false
181+
})),
177182
iconType: <GlobeAmericas />,
178183
id: 'app-menu-locale-selector',
179184
label: <FormattedMessage id="components.SubNav.languageSelector" />,

Diff for: lib/components/app/desktop-nav.tsx

+1-7
Original file line numberDiff line numberDiff line change
@@ -72,13 +72,7 @@ const DesktopNav = ({ otpConfig, popupTarget, setPopupContent }: Props) => {
7272
<FormattedMessage id={`config.popups.${popupTarget}`} />
7373
</NavItemOnLargeScreens>
7474
)}
75-
{configLanguages &&
76-
// Ensure that > 1 valid language is defined
77-
Object.keys(configLanguages).filter(
78-
(key) => key !== 'allLanguages' && configLanguages[key].name
79-
).length > 1 && (
80-
<LocaleSelector configLanguages={configLanguages} />
81-
)}
75+
<LocaleSelector configLanguages={configLanguages} />
8276
{showLogin && (
8377
<NavLoginButtonAuth0
8478
id="login-control"

Diff for: lib/components/app/locale-selector.tsx

+30-35
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,28 @@
1-
import { connect, ConnectedProps } from 'react-redux'
1+
import { connect } from 'react-redux'
22
import { GlobeAmericas } from '@styled-icons/fa-solid/GlobeAmericas'
33
import { useIntl } from 'react-intl'
4-
import React, { MouseEvent } from 'react'
4+
import React from 'react'
55

66
import * as uiActions from '../../actions/ui'
7-
import * as userActions from '../../actions/user'
8-
import { StyledIconWrapper } from '../util/styledIcon'
7+
import { getLanguageOptions } from '../../util/i18n'
98
import { UnstyledButton } from '../util/unstyled-button'
109
import Dropdown from '../util/dropdown'
1110

12-
type PropsFromRedux = ConnectedProps<typeof connector>
13-
14-
interface LocaleSelectorProps extends PropsFromRedux {
11+
interface LocaleSelectorProps {
1512
// Typescript TODO configLanguageType
1613
configLanguages: Record<string, any>
14+
locale: string
15+
setLocale: (locale: string) => void
1716
}
1817

19-
const LocaleSelector = (props: LocaleSelectorProps): JSX.Element => {
18+
const LocaleSelector = (props: LocaleSelectorProps): JSX.Element | null => {
2019
const { configLanguages, locale: currentLocale, setLocale } = props
21-
20+
const languageOptions: Record<string, any> | null =
21+
getLanguageOptions(configLanguages)
2222
const intl = useIntl()
2323

24-
return (
24+
// Only render if two or more languages are configured.
25+
return languageOptions ? (
2526
<Dropdown
2627
id="locale-selector"
2728
label={intl.formatMessage({ id: 'components.SubNav.selectALanguage' })}
@@ -38,30 +39,25 @@ const LocaleSelector = (props: LocaleSelectorProps): JSX.Element => {
3839
style={{ display: 'block ruby' }}
3940
// TODO: How to make this work without block ruby?
4041
>
41-
{Object.keys(configLanguages)
42-
.filter((locale) => locale !== 'allLanguages')
43-
.map((locale) => (
44-
<li
45-
aria-selected={locale === currentLocale}
46-
key={locale}
47-
lang={locale}
48-
onClick={() => setLocale(locale)}
49-
onKeyPress={() => setLocale(locale)}
50-
// We are correct, not eslint: https://w3c.github.io/aria-practices/examples/combobox/combobox-select-only.html
51-
// eslint-disable-next-line jsx-a11y/no-noninteractive-element-to-interactive-role
52-
role="option"
53-
tabIndex={0}
54-
>
55-
<UnstyledButton
56-
style={locale === currentLocale ? { fontWeight: 'bold' } : {}}
57-
tabIndex={-1}
58-
>
59-
{configLanguages[locale].name}
60-
</UnstyledButton>
61-
</li>
62-
))}
42+
{Object.keys(languageOptions).map((locale: string) => (
43+
<li
44+
aria-selected={locale === currentLocale || undefined}
45+
key={locale}
46+
lang={locale}
47+
onClick={() => setLocale(locale)}
48+
onKeyPress={() => setLocale(locale)}
49+
// We are correct, not eslint: https://w3c.github.io/aria-practices/examples/combobox/combobox-select-only.html
50+
// eslint-disable-next-line jsx-a11y/no-noninteractive-element-to-interactive-role
51+
role="option"
52+
tabIndex={0}
53+
>
54+
<UnstyledButton tabIndex={-1}>
55+
{languageOptions[locale].name}
56+
</UnstyledButton>
57+
</li>
58+
))}
6359
</Dropdown>
64-
)
60+
) : null
6561
}
6662

6763
// Typescript TODO: type state properly
@@ -75,5 +71,4 @@ const mapDispatchToProps = {
7571
setLocale: uiActions.setLocale
7672
}
7773

78-
const connector = connect(mapStateToProps, mapDispatchToProps)
79-
export default connector(LocaleSelector)
74+
export default connect(mapStateToProps, mapDispatchToProps)(LocaleSelector)

Diff for: lib/components/mobile/navigation-bar.js

+1-10
Original file line numberDiff line numberDiff line change
@@ -74,16 +74,7 @@ class MobileNavigationBar extends Component {
7474
)}
7575

7676
<div className="locale-selector-and-login">
77-
{configLanguages &&
78-
// Ensure that > 1 valid language is defined
79-
Object.keys(configLanguages).filter(
80-
(key) => key !== 'allLanguages' && configLanguages[key].name
81-
).length > 1 && (
82-
<LocaleSelector
83-
className="locale-selector"
84-
configLanguages={configLanguages}
85-
/>
86-
)}
77+
<LocaleSelector configLanguages={configLanguages} />
8778
{/**
8879
* HACK: Normally, NavLoginButtonAuth0 should be inside a <Nav> element,
8980
* however, in mobile mode, react-bootstrap's <Nav> causes the

Diff for: lib/util/i18n.js

+22
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,28 @@ export function getConfigLocales(configLanguages) {
4949
return locales
5050
}
5151

52+
/**
53+
* Gets entries to display in language selectors, ensuring that at least one language is shown.
54+
* This excludes the generic 'allLanguages' section in the config.
55+
* @param {*} configLanguages The configured languages.
56+
* @returns An array of the supported locale ids if 2 or more are configured, null otherwise.
57+
*/
58+
export function getLanguageOptions(configLanguages) {
59+
const filteredKeys =
60+
(configLanguages &&
61+
Object.keys(configLanguages).filter(
62+
(key) => key !== 'allLanguages' && configLanguages[key].name
63+
)) ||
64+
[]
65+
if (filteredKeys.length < 2) return null
66+
67+
const filteredLanguages = {}
68+
filteredKeys.forEach((key) => {
69+
filteredLanguages[key] = configLanguages[key]
70+
})
71+
return filteredLanguages
72+
}
73+
5274
/**
5375
* Loads in all localized strings from @opentripplanner packages.
5476
* Package list is loaded from the package.json and each yml file then imported.

0 commit comments

Comments
 (0)