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

fix(a11y): user select menu labels #6602

Merged
merged 2 commits into from
Jan 17, 2025
Merged
Show file tree
Hide file tree
Changes from all 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
12 changes: 7 additions & 5 deletions packages/lib-user/src/components/shared/GroupForm/GroupForm.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ function GroupForm({

const [value, setValue] = useState(defaultValue)
const statsVisibilityOptions = value.visibility === 'Private' ? PRIVATE_STATS_VISIBILITY : PUBLIC_STATS_VISIBILITY
const selectedVisibilityOption = statsVisibilityOptions.find((option) => option.value === value.stats_visibility)

useEffect(() => {
setValue(defaultValue)
Expand All @@ -64,11 +65,12 @@ function GroupForm({
return (
<Form
onChange={(nextValue, { touched }) => {
const selectedVisibilityOption = statsVisibilityOptions.find((option) => option.label === nextValue.stats_visibility)
let stats_visibility = selectedVisibilityOption ? selectedVisibilityOption.value : value.stats_visibility
if (nextValue.visibility !== value.visibility) {
const statsVisibility = nextValue.visibility === 'Private' ? 'private_agg_only' : 'public_agg_only'
nextValue.stats_visibility = statsVisibility
stats_visibility = nextValue.visibility === 'Private' ? 'private_agg_only' : 'public_agg_only'
}
setValue(nextValue)
setValue({ ...nextValue, stats_visibility })
}}
onSubmit={handleSubmit}
value={value}
Expand Down Expand Up @@ -151,10 +153,10 @@ function GroupForm({
<Select
id='stats_visibility'
aria-label={t('GroupForm.visibility')}
labelKey='label'
name='stats_visibility'
options={statsVisibilityOptions}
valueKey={{ key: 'value', reduce: true }}
value={selectedVisibilityOption.label}
valueKey={{ key: 'label', reduce: true }}
/>
</FormField>
</ThemeContext.Extend>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ describe('components > shared > GroupForm', function() {
render(<CreateStory />)

// the Grommet Select component renders as a button with a textbox. The statsVisibility input uses the Grommet Select, therefore we need to find the textbox role. The textbox name includes the value of the select, which by default is 'private_agg_only'.
const statsVisibility = screen.getByRole('textbox', { name: 'Stats Visibility, private_agg_only' })
const statsVisibility = screen.getByRole('textbox', { name: 'Stats Visibility, No, don\'t show individual stats to members' })
expect(statsVisibility).to.be.ok()
})

Expand All @@ -49,7 +49,7 @@ describe('components > shared > GroupForm', function() {
const privateRadio = screen.getByRole('radio', { name: 'Private - only members can view this group' })
expect(privateRadio.checked).to.be.true()

const statsVisibility = screen.getByRole('textbox', { name: 'Stats Visibility, private_agg_only' })
const statsVisibility = screen.getByRole('textbox', { name: 'Stats Visibility, No, don\'t show individual stats to members' })
await user.click(statsVisibility)

const options = screen.getAllByRole('option')
Expand All @@ -67,7 +67,7 @@ describe('components > shared > GroupForm', function() {
await user.click(publicRadio)
expect(publicRadio.checked).to.be.true()

const statsVisibility = screen.getByRole('textbox', { name: 'Stats Visibility, public_agg_only' })
const statsVisibility = screen.getByRole('textbox', { name: 'Stats Visibility, No, never show individual stats' })
await user.click(statsVisibility)

const options = screen.getAllByRole('option')
Expand Down Expand Up @@ -148,7 +148,7 @@ describe('components > shared > GroupForm', function() {
it('should show the group stats visibility', function() {
render(<ManageStory />)

const statsVisibility = screen.getByRole('textbox', { name: 'Stats Visibility, public_show_all' })
const statsVisibility = screen.getByRole('textbox', { name: 'Stats Visibility, Yes, always show individual stats' })
expect(statsVisibility).to.be.ok()
})
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,13 +212,15 @@ function MainContent({
<Select
id='project-select'
name='project-select'
aria-label={t('MainContent.selectProject')}
handleChange={handleProjectSelect}
options={projectOptions}
value={selectedProjectOption}
/>
<Select
id='date-range-select'
name='date-range-select'
aria-label={t('MainContent.selectDateRange')}
handleChange={handleDateRangeSelect}
options={dateRangeOptions}
value={selectedDateRangeOption}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { USER } from '../../../../test/mocks/panoptes'
import { STATS } from '../../../../test/mocks/stats.mock.js'

import Meta, { Default, NoStats, ParamsValidationMessage } from './MainContent.stories.js'
import { expect } from 'chai'

const todayUTC = getStatsDateString(new Date())
const sevenDaysAgoUTC = getStatsDateString(new Date(Date.now() - 6 * 24 * 60 * 60 * 1000))
Expand Down Expand Up @@ -37,16 +38,20 @@ describe('components > shared > MainContent', function () {

it('should show "ALL PROJECTS" as the selected project', function () {
render(<DefaultStory />)
const projectSelect = screen.getByRole('textbox', { name: 'project-select' })
const projectSelectMenu = screen.getByRole('button', { name: 'Select project; Selected: ALL PROJECTS' })
const projectSelect = screen.getByRole('textbox', { name: 'Select project, ALL PROJECTS' })
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Grommet appends the selected value to the name of the textbox. I'm not sure why, since it will also be read out as the textbox value. 🤷‍♂️

https://github.com/grommet/grommet/blob/474cc08ed3ced7b1021985db0fcacae448bab2ca/src/js/components/Select/Select.js#L374-L379


expect(projectSelectMenu).to.be.ok()
expect(projectSelect).to.be.ok()
expect(projectSelect.value).to.equal('ALL PROJECTS')
})

it('should show "LAST 7 DAYS" as the selected date range', function () {
render(<DefaultStory />)
const dateRangeSelect = screen.getByRole('textbox', { name: 'date-range-select' })
const dateRangeSelectMenu = screen.getByRole('button', { name: 'Select date range; Selected: LAST 7 DAYS' })
const dateRangeSelect = screen.getByRole('textbox', { name: 'Select date range, LAST 7 DAYS' })

expect(dateRangeSelectMenu).to.be.ok()
expect(dateRangeSelect).to.be.ok()
expect(dateRangeSelect.value).to.equal('LAST 7 DAYS')
})
Expand Down
8 changes: 5 additions & 3 deletions packages/lib-user/src/components/shared/Select/Select.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ function Select({
name = '',
handleChange = DEFAULT_HANDLER,
options = [],
value = DEFAULT_VALUE
value = DEFAULT_VALUE,
...props
}) {
const [selected, setSelected] = useState(value)

Expand All @@ -34,14 +35,15 @@ function Select({
return (
<ThemeContext.Extend value={selectTheme}>
<StyledSelect
a11yTitle={name}
id={id}
name={name}
labelKey='label'
onChange={({ option }) => handleSelect(option)}
options={options}
size='medium'
value={selected}
value={selected.label}
valueKey={{ key: 'label', reduce: true }}
{...props}
/>
</ThemeContext.Extend>
)
Expand Down
2 changes: 2 additions & 0 deletions packages/lib-user/src/translations/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,8 @@
"error": "There was an error.",
"hoursTip": "Hours are calculated based on the start and end times of your classification efforts. Hours do not reflect your time spent on Talk.",
"noData": "No data found.",
"selectDateRange": "Select date range",
"selectProject": "Select project",
"start": "Start by <0>classifying a project</0> now, or change the date range.",
"tabContents": "Tab Contents"
},
Expand Down
Loading