-
Notifications
You must be signed in to change notification settings - Fork 55
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
Accessible Interface for Line/Area Charts #294
Conversation
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.
This looks great @ekh64 Really looking forward to testing it out. I left a few comments and I'm happy to chat about them more offline.
`x Value: ${data[frameIndex].x}, y Value: ${data[frameIndex].y}` | ||
} | ||
numFrames={data.length} | ||
/> |
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.
I had this idea to simplify the API by using dependency injection and injecting the A11yInterface
into LineChart
so that a) the API is more declarative and b) we can be more explicit about which components support keyboard navigation. Then I realized it doesn't work for multiple line charts in the same XYPlot which is probably why you didn't go with this in the 1st place!
Still sharing for discussion. Is there any way we could do something like this and still support multiple charts in the same XYPlot?
return <XYPlot scaleType="linear" width={600} height={350}>
<XAxis title="X Axis" />
<YAxis title="Y Axis" />
<LineChart
data={data}
x={d => d.x}
y={d => d.y}
lineStyle={{stroke: '#ff7f0e', strokeWidth: 3}}
ariaLabel={(d, index_in_case_we_need_it) => {/* rename ariaLabelGenerator -> ariaLabel */}
`x Value: ${d.x}, y Value: ${d.y}`
}
numAriaLabels={data.length} {/* Do we even need this? Could be optional and default to data.length*/}
/>
</XYPlot>;
I'm guessing inside of LineChart
we'd add something like this:
{ props.ariaLabel ?
<A11yInterface
ariaLabelGenerator={props.ariaLabel}
numFrames={props.numAriaLabels || props.data.length}
/> : null
}
We can even provide a sensible default ariaLabelGenerator
function for LineChart and then the caller could get this for free.
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.
As I mentioned in another comment, I'd prefer to leave this as a standalone component because this would break if we were to use multiple LineCharts, or Line + Area Charts. My project leverages rendering multiple data in different linecharts, and this built in schema would not work for that use case. However if we export the A11yInterface
as an optional component and do this as well, I think that would be great.
src/A11yInterface.js
Outdated
* [aria-labels](https://www.w3.org/TR/WCAG20-TECHS/ARIA6.html#ARIA6-description) are critical for users accessing | ||
* the web with screenreaders or other assistive technologies. | ||
*/ | ||
|
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.
I really appreciate the detailed comment here.
I'm thinking of a few other ways to tweak the API here to make it more like other Reactochart functions. Why not just pass data
as a prop to A11yInterface
and make ariaLabel
a function that takes a datum and an optional index, similar to most D3 functions? Then numFrames
(which I would rename numLabels
) could default to data.length
and probably doesn't even have to be passed in. It seems like most of the implementations of ariaLabelGenerator need the data anyway.
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.
So I thought a lot about using data. It becomes a little more complex when dealing with multiple data sets. For example, if you have two line charts in an XYPlot
, you would only want one A11yInterface
, and that A11yInterface
would need to account for both data sets. This makes calculating the number of frames a little complicated since data1
and data2
can be different lengths, and can contain overlapping values.
I wanted to avoid looping over the data to find all unique data points, though since lodash
is a dependency here, I suppose we could find the union of x
values more easily. unionBy
seems promising and using Reactochart's passed in dataset
prop, this could work. However, it would make generating the aria-labels
a little different, since then we'd be working with the x
value, as opposed to the frameIndex
. This would also make the Interface more robust with data sets of unequal lengths and unequal x
values.
src/A11yInterface.js
Outdated
x={ | ||
sliceWidth * | ||
(frameIndex === numFrames - 1 ? frameIndex - 1 : frameIndex) | ||
// otherwise the last rect renders outside the chart |
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.
Thanks for the helpful comment!
rects.at(0).simulate('keydown'); | ||
expect(onKeyDown).toHaveBeenCalled(); | ||
}); | ||
}); |
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.
Thanks for adding these clear tests.
src/A11yInterface.js
Outdated
*/ | ||
|
||
export default function A11yInterface(props) { | ||
const { ariaLabelGenerator, numFrames, onKeyDown, height, width } = props; |
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.
I wonder if we can brainstorm some better names for this component that describes what it does. There are many other parts to A11y that we might tackle in the future using different components. How about AriaLabelContainer
, AriaLabelChart
or InteractiveAriaLabels
?
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.
Totally open to renaming, I like AriaLabelContainer
@iezer Those were great suggestions, I've gone ahead and significantly changed the schema, which now takes n data with accessors Let me know if you have other suggestions, as the |
src/A11yInterface.js
Outdated
<rect | ||
className="rct-chart-visually-hidden-rect" | ||
aria-label={ariaLabelGenerator(frameIndex)} | ||
key={frameIndex} | ||
aria-label={ariaLabelGenerator(d)} |
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.
This variable name is a bit confusing because if I'm reading this correct it seems like d is really an xValue
. Seems like ariaLabelGenerator
will almost always want the array of all data points for that xValue so how about figuring that out here in order to simplify the handlers that are passed in? This line could be something like
aria-label={ariaLabelGenerator(xValue, data, index)}
where data is the array of data just or that xValue, not the whole data for the chart.
You'll probably have to change line 78 to something like this:
data.map(d => [d, getValue(accessor, d)]),
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.
My one concern is this approach could lose the context of the data. We can return the data points, but it could become unclear to the end user which data point belongs to which dataset it belongs to. For instance, I could have a schema like:
{
name: 'Beyonce'
data: [array of values to be rendered]
}
{
name: 'Drake'
data: [...]
}
In the schema I defined, that context (name
) is lost when returning the two datapoints for a given xValue
, and since aria-labels need to provide that context so the user understand what they're hearing, I'm not sure this is a better approach.
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.
Can you give a full example with that structure? All the examples just have an array of data with no name. Isn't the same end user who's defining ariaLabelGenerator
also defining datasetWithAccessor
and could reasonably assume that the data points come in the same order?
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.
The issue is less with the datapoint and more with needing context outside of the array used to generate the LineChart, which currently is all that's being used in datasetWithAccessor
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.
So in the example I provided, we'd only be passing in the data
attribute to the LineChart
and datasetWithAccessor
, but we'd want to know what name
data0 and data1 belong to in the ariaLabelGenerator
. The user can define this themselves if we provide the xValue
, but I worry about providing the other arguments if the end user doesn't actually get value given multiple datasets. They'd have to do that figuring out themselves anyway.
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.
Oh I see, my suggestion was to pass the xValue
and the data
(and even the index in case people need it) since I'm guessing in 99% of cases (and both your docs examples) users will want the data. So in your 2 datasets example, instead of
const ariaLabelGenerator = xValue => {
const data0Point = data0.find(d => d.x === xValue);
const data1Point = data1.find(d => d.x === xValue);
let ariaLabelString = `x Value, ${xValue}`;
if(data0Point) {
ariaLabelString += `, data0 y Value ${data0Point.y}`;
}
if(data1Point) {
ariaLabelString += `, data1 y Value ${data1Point.y}`;
}
return ariaLabelString;
}
you could have
const ariaLabelGenerator = (xValue, [data0Point, data1Point], /* index */) => {
let ariaLabelString = `x Value, ${xValue}`;
if(data0Point) {
ariaLabelString += `, data0 y Value ${data0Point.y}`;
}
if(data1Point) {
ariaLabelString += `, data1 y Value ${data1Point.y}`;
}
return ariaLabelString;
}
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.
Sounds good, I'll make those changes this week and update the PR 👍
sidenote, I noticed some funkiness around precommit hooks and the browser tests. However running the browser tests in the actual browser pass 🤷♀️ |
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.
🚢 Great work @ekh64 thanks for taking this on. Looking forward to trying it out.
{zippedDatapoints.map(({ xValue, data }, index) => ( | ||
<rect | ||
className="rct-chart-visually-hidden-rect" | ||
aria-label={ariaLabelGenerator(xValue, data, index)} |
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.
👍 this looks great.
* add CODEOWNERS file (#262) * Bump elliptic from 6.5.2 to 6.5.3 (#266) Bumps [elliptic](https://github.com/indutny/elliptic) from 6.5.2 to 6.5.3. - [Release notes](https://github.com/indutny/elliptic/releases) - [Commits](indutny/elliptic@v6.5.2...v6.5.3) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * bar chart gradient example in docs (#268) Co-authored-by: Isaac Ezer <[email protected]> * Sankey sorting (#269) * remove deprecated refs definition This old usage of defining refs via a string is deprecated and seems to break when debugging reactochart using `npm link` locally. * add Sankey Sorting Function hooks Co-authored-by: Isaac Ezer <[email protected]> * 3.2.0 * v3.2.0 release * Fix YAxisLabels formatting example (#271) The current example did not demonstrate label color/weight change, since the final label (`label.text`) never was `20.00`. * 3.2.1 * 3.2.1 release * [A11y] add aria-hidden attributes to XYPlot (#273) * add aria-hidden attributes to XYPlot elements * more granual application of aria-hidden * 3.2.2 * Add changelog edit for 3.2.2 * v3.2.2 release * v3.2.2 release * Upgrade D3 6.3.1 (#276) * d3 6.0 * update imports * make all the things prettier * restrict d3 import * remove global d3 imports from specs Co-authored-by: Isaac Ezer <[email protected]> * 4.0.0 * v4.0.0 release * remove old node versions. Support >=12 (#287) * remove old node versions. Support >=12 * auto fix audit errors * remove node 16, build failing * update package-lock.json Co-authored-by: Isaac Ezer <[email protected]> * Bump lodash from 4.17.15 to 4.17.21 (#285) Bumps [lodash](https://github.com/lodash/lodash) from 4.17.15 to 4.17.21. - [Release notes](https://github.com/lodash/lodash/releases) - [Commits](lodash/lodash@4.17.15...4.17.21) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump elliptic from 6.5.3 to 6.5.4 (#278) Bumps [elliptic](https://github.com/indutny/elliptic) from 6.5.3 to 6.5.4. - [Release notes](https://github.com/indutny/elliptic/releases) - [Commits](indutny/elliptic@v6.5.3...v6.5.4) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * 5.0.0 * v5.0.0 release * Migrate jsdom tests to jest (#291) * Create dependabot.yml * [Infra] Enable a Github Action for CI (#308) * Create node.js.yml * remove browser test step * CI test * Revert "CI test" This reverts commit b2c894a. * Feature/arackis/make work with d3 v7 (#307) * Install d3, update Jest to transpile d3 * Fix unit tests - negative numbers were using the wrong negative unicode character * Include d3@6 as acceptable * Undo whitespace changes in jest config * fix lockfile * 6.0.0 * v6.0.0 release * Accessible Interface for Line/Area Charts (#294) * A11yInterface in reactochart * npm run make-docs + render A11yInterface docs * fix offset bug + more examples * refactor for simplicity * revert the styling since the outline wasn't a problem * A11yInterface Tests * refactor to use datasetsWithAccessor instead * rename + doc generation * wip: refactor of AriaLabelContainer * fix an example, fix bug with chart * fix my examples & refactor again * update package-lock * 6.1.0 * v6.1.0 release Co-authored-by: Anita <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Marko Bonaći <[email protected]> Co-authored-by: Erica Hyman <[email protected]> Co-authored-by: Anita Wang <[email protected]> Co-authored-by: Markus Wagner <[email protected]> Co-authored-by: Adam Rackis <[email protected]>
* add CODEOWNERS file (#262) * Bump elliptic from 6.5.2 to 6.5.3 (#266) Bumps [elliptic](https://github.com/indutny/elliptic) from 6.5.2 to 6.5.3. - [Release notes](https://github.com/indutny/elliptic/releases) - [Commits](indutny/elliptic@v6.5.2...v6.5.3) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * bar chart gradient example in docs (#268) Co-authored-by: Isaac Ezer <[email protected]> * Sankey sorting (#269) * remove deprecated refs definition This old usage of defining refs via a string is deprecated and seems to break when debugging reactochart using `npm link` locally. * add Sankey Sorting Function hooks Co-authored-by: Isaac Ezer <[email protected]> * 3.2.0 * v3.2.0 release * Fix YAxisLabels formatting example (#271) The current example did not demonstrate label color/weight change, since the final label (`label.text`) never was `20.00`. * 3.2.1 * 3.2.1 release * [A11y] add aria-hidden attributes to XYPlot (#273) * add aria-hidden attributes to XYPlot elements * more granual application of aria-hidden * 3.2.2 * Add changelog edit for 3.2.2 * v3.2.2 release * v3.2.2 release * Upgrade D3 6.3.1 (#276) * d3 6.0 * update imports * make all the things prettier * restrict d3 import * remove global d3 imports from specs Co-authored-by: Isaac Ezer <[email protected]> * 4.0.0 * v4.0.0 release * remove old node versions. Support >=12 (#287) * remove old node versions. Support >=12 * auto fix audit errors * remove node 16, build failing * update package-lock.json Co-authored-by: Isaac Ezer <[email protected]> * Bump lodash from 4.17.15 to 4.17.21 (#285) Bumps [lodash](https://github.com/lodash/lodash) from 4.17.15 to 4.17.21. - [Release notes](https://github.com/lodash/lodash/releases) - [Commits](lodash/lodash@4.17.15...4.17.21) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump elliptic from 6.5.3 to 6.5.4 (#278) Bumps [elliptic](https://github.com/indutny/elliptic) from 6.5.3 to 6.5.4. - [Release notes](https://github.com/indutny/elliptic/releases) - [Commits](indutny/elliptic@v6.5.3...v6.5.4) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * 5.0.0 * v5.0.0 release * Migrate jsdom tests to jest (#291) * Create dependabot.yml * [Infra] Enable a Github Action for CI (#308) * Create node.js.yml * remove browser test step * CI test * Revert "CI test" This reverts commit b2c894a. * Feature/arackis/make work with d3 v7 (#307) * Install d3, update Jest to transpile d3 * Fix unit tests - negative numbers were using the wrong negative unicode character * Include d3@6 as acceptable * Undo whitespace changes in jest config * fix lockfile * 6.0.0 * v6.0.0 release * Accessible Interface for Line/Area Charts (#294) * A11yInterface in reactochart * npm run make-docs + render A11yInterface docs * fix offset bug + more examples * refactor for simplicity * revert the styling since the outline wasn't a problem * A11yInterface Tests * refactor to use datasetsWithAccessor instead * rename + doc generation * wip: refactor of AriaLabelContainer * fix an example, fix bug with chart * fix my examples & refactor again * update package-lock * 6.1.0 * v6.1.0 release * Bump shelljs from 0.8.4 to 0.8.5 (#343) Bumps [shelljs](https://github.com/shelljs/shelljs) from 0.8.4 to 0.8.5. - [Release notes](https://github.com/shelljs/shelljs/releases) - [Changelog](https://github.com/shelljs/shelljs/blob/master/CHANGELOG.md) - [Commits](shelljs/shelljs@v0.8.4...v0.8.5) --- updated-dependencies: - dependency-name: shelljs dependency-type: direct:development ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump @babel/cli from 7.10.0 to 7.16.8 (#342) Bumps [@babel/cli](https://github.com/babel/babel/tree/HEAD/packages/babel-cli) from 7.10.0 to 7.16.8. - [Release notes](https://github.com/babel/babel/releases) - [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md) - [Commits](https://github.com/babel/babel/commits/v7.16.8/packages/babel-cli) --- updated-dependencies: - dependency-name: "@babel/cli" dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Upgrades (#345) * npm update * npm audit fix * fix warning about map without key in examples * wip web-script 5 * web-scripts 6 * wip web-scripts 7 * wip npm audit fix * wip web-scripts 8 * wip web-scripts 9 * wip web-scripts 10 * web-scripts 11 * some more audit fixes * document npm link * Bump url-parse from 1.5.4 to 1.5.10 (#354) Bumps [url-parse](https://github.com/unshiftio/url-parse) from 1.5.4 to 1.5.10. - [Release notes](https://github.com/unshiftio/url-parse/releases) - [Commits](unshiftio/url-parse@1.5.4...1.5.10) --- updated-dependencies: - dependency-name: url-parse dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Vulnerability upgrades (#358) * Bump remark + remark-react * bump web-scripts & misc updates * bump webpack to 5.x * clean up CODEOWNERS * Bump @babel/plugin-proposal-object-rest-spread from 7.10.0 to 7.17.3 (#350) Bumps [@babel/plugin-proposal-object-rest-spread](https://github.com/babel/babel/tree/HEAD/packages/babel-plugin-proposal-object-rest-spread) from 7.10.0 to 7.17.3. - [Release notes](https://github.com/babel/babel/releases) - [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md) - [Commits](https://github.com/babel/babel/commits/v7.17.3/packages/babel-plugin-proposal-object-rest-spread) --- updated-dependencies: - dependency-name: "@babel/plugin-proposal-object-rest-spread" dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump postcss from 7.0.35 to 7.0.39 (#352) Bumps [postcss](https://github.com/postcss/postcss) from 7.0.35 to 7.0.39. - [Release notes](https://github.com/postcss/postcss/releases) - [Changelog](https://github.com/postcss/postcss/blob/7.0.39/CHANGELOG.md) - [Commits](postcss/postcss@7.0.35...7.0.39) --- updated-dependencies: - dependency-name: postcss dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump follow-redirects from 1.14.7 to 1.14.9 (#353) Bumps [follow-redirects](https://github.com/follow-redirects/follow-redirects) from 1.14.7 to 1.14.9. - [Release notes](https://github.com/follow-redirects/follow-redirects/releases) - [Commits](follow-redirects/follow-redirects@v1.14.7...v1.14.9) --- updated-dependencies: - dependency-name: follow-redirects dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump d3-color from 3.0.1 to 3.1.0 (#443) Bumps [d3-color](https://github.com/d3/d3-color) from 3.0.1 to 3.1.0. - [Release notes](https://github.com/d3/d3-color/releases) - [Commits](d3/d3-color@v3.0.1...v3.1.0) --- updated-dependencies: - dependency-name: d3-color dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]> Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * 6.1.1 * v 6.1.1 release Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: Anita <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Isaac Ezer <[email protected]> Co-authored-by: Isaac Ezer <[email protected]> Co-authored-by: Marko Bonaći <[email protected]> Co-authored-by: Anita Wang <[email protected]> Co-authored-by: Markus Wagner <[email protected]> Co-authored-by: Adam Rackis <[email protected]>
This is some work I did for a project to include accessible data visualizations for line and area charts. I figured I'd bring it into Reactochart to share with the broader community.
In this PR is the interface itself, documentation, and tests.
Here is what the interface looks like. It allows users to navigate through a line/area chart with screenreaders or other assistive technology.
As this was written with a somewhat specific use case in mind, it might not be compatible with all Reactochart features (I don't, for instance, recommend using this with bar charts) and custom domains might require a little more thought to ensure we capture the data points accurately.
That being said, I do think we should include it for our consumers, and who knows, maybe one of them will have some great feedback/ideas for this.