-
Notifications
You must be signed in to change notification settings - Fork 121
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
feat(a11y): accessible goal and gauge chart #1174
Changes from 6 commits
91bb738
c63b88b
a105604
6245b05
0dc89a6
ebd8af7
9d5bc31
44b9eb5
c65eb86
9216fc8
4f8e86c
9b4d573
eb6918c
16fe794
08c9c0b
7ad9893
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
/* | ||
* Licensed to Elasticsearch B.V. under one or more contributor | ||
* license agreements. See the NOTICE file distributed with | ||
* this work for additional information regarding copyright | ||
* ownership. Elasticsearch B.V. licenses this file to you under | ||
* the Apache License, Version 2.0 (the "License"); you may | ||
* not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, | ||
* software distributed under the License is distributed on an | ||
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
* KIND, either express or implied. See the License for the | ||
* specific language governing permissions and limitations | ||
* under the License. | ||
*/ | ||
|
||
import { createCustomCachedSelector } from '../../../../state/create_selector'; | ||
import { geometries } from './geometries'; | ||
|
||
/** @internal */ | ||
export type GoalChartData = { | ||
maximum: number; | ||
minimum: number; | ||
target: number; | ||
value: number; | ||
}; | ||
|
||
/** @internal */ | ||
export type GoalChartLabels = { | ||
minorLabel: string; | ||
majorLabel: string; | ||
}; | ||
|
||
/** @internal */ | ||
export const getGoalChartDataSelector = createCustomCachedSelector( | ||
[geometries], | ||
(geoms): GoalChartData => { | ||
const goalChartData: GoalChartData = { | ||
maximum: geoms.bulletViewModel.highestValue, | ||
minimum: geoms.bulletViewModel.lowestValue, | ||
target: geoms.bulletViewModel.target, | ||
value: geoms.bulletViewModel.actual, | ||
}; | ||
return goalChartData; | ||
}, | ||
); | ||
|
||
/** @internal */ | ||
export const getGoalChartLabelsSelector = createCustomCachedSelector([geometries], (geoms) => { | ||
return { majorLabel: geoms.bulletViewModel.labelMajor, minorLabel: geoms.bulletViewModel.labelMinor }; | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,11 +19,30 @@ | |
|
||
import React from 'react'; | ||
|
||
import { GoalChartLabels } from '../../chart_types/goal_chart/state/selectors/get_goal_chart_data'; | ||
import { A11ySettings } from '../../state/selectors/get_accessibility_config'; | ||
|
||
interface ScreenReaderLabelProps { | ||
goalChartLabels?: GoalChartLabels; | ||
} | ||
|
||
/** @internal */ | ||
export function ScreenReaderLabel(props: A11ySettings) { | ||
if (!props.label) return null; | ||
const Heading = props.labelHeadingLevel; | ||
return <Heading id={props.labelId}>{props.label}</Heading>; | ||
export function ScreenReaderLabel({ | ||
label, | ||
labelHeadingLevel, | ||
labelId, | ||
goalChartLabels, | ||
}: A11ySettings & ScreenReaderLabelProps) { | ||
if (!label && !goalChartLabels?.majorLabel) return null; | ||
const Heading = labelHeadingLevel; | ||
const goalChartLabelsSection = !goalChartLabels?.majorLabel | ||
? null | ||
: `Goal chart label: ${goalChartLabels.majorLabel} ${goalChartLabels.minorLabel}`; | ||
|
||
return ( | ||
<Heading id={labelId}> | ||
{label} | ||
{goalChartLabelsSection} | ||
</Heading> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this whole "while label to render" thing is, unfortunately, going to be more complicated... So, first, the easy change:
Now we've got the more difficult change that I hid under a new variable I made up called
👆 I also changed the string a little bit. Because we don't know what the user might be passing in, and it seems a little strange to pass in both, I'm being super explicit about it but also trying not to duplicate info (we elsewhere say it's a goal chart). Hope this all makes sense! Happy to zoom about it too! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I'm following!! 44b9eb5 for changes, thank you! |
||
); | ||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -19,19 +19,33 @@ | |||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
import React from 'react'; | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
import { GoalChartData } from '../../chart_types/goal_chart/state/selectors/get_goal_chart_data'; | ||||||||||||||||||||||||||||||
import { A11ySettings } from '../../state/selectors/get_accessibility_config'; | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
interface ScreenReaderTypesProps { | ||||||||||||||||||||||||||||||
chartTypeDescription: string; | ||||||||||||||||||||||||||||||
goalChartData?: GoalChartData; | ||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
/** @internal */ | ||||||||||||||||||||||||||||||
export function ScreenReaderTypes(props: A11ySettings & ScreenReaderTypesProps) { | ||||||||||||||||||||||||||||||
if (!props.defaultSummaryId) return null; | ||||||||||||||||||||||||||||||
export function ScreenReaderTypes({ | ||||||||||||||||||||||||||||||
goalChartData, | ||||||||||||||||||||||||||||||
defaultSummaryId, | ||||||||||||||||||||||||||||||
chartTypeDescription, | ||||||||||||||||||||||||||||||
}: A11ySettings & ScreenReaderTypesProps) { | ||||||||||||||||||||||||||||||
if (!defaultSummaryId && !goalChartData) return null; | ||||||||||||||||||||||||||||||
const validGoalChart = | ||||||||||||||||||||||||||||||
chartTypeDescription === 'goal chart' || | ||||||||||||||||||||||||||||||
chartTypeDescription === 'horizontalBullet chart' || | ||||||||||||||||||||||||||||||
chartTypeDescription === 'verticalBullet chart'; | ||||||||||||||||||||||||||||||
return ( | ||||||||||||||||||||||||||||||
<dl> | ||||||||||||||||||||||||||||||
<dt>Chart type:</dt> | ||||||||||||||||||||||||||||||
<dd id={props.defaultSummaryId}>{props.chartTypeDescription}</dd> | ||||||||||||||||||||||||||||||
<dd id={defaultSummaryId}>{chartTypeDescription}</dd> | ||||||||||||||||||||||||||||||
{validGoalChart && goalChartData && <dd>{`Minimum: ${goalChartData.minimum}`}</dd>} | ||||||||||||||||||||||||||||||
{validGoalChart && goalChartData && <dd>{`Maximum: ${goalChartData.maximum}`}</dd>} | ||||||||||||||||||||||||||||||
{validGoalChart && goalChartData && <dd>{`Target: ${goalChartData.target}`}</dd>} | ||||||||||||||||||||||||||||||
{validGoalChart && goalChartData && <dd>{`Value: ${goalChartData.value}`}</dd>} | ||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should follow the same DOM structure for all the items on in the list: Not tested but something like this should work:
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you, this makes sense! 44b9eb5 |
||||||||||||||||||||||||||||||
</dl> | ||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we do a
goalChartLabels.minorLabel
check here too?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yas - 44b9eb5 thanks!!