jordibruin / Swift-Charts-Examples

An overview of the different types of charts you can make with Swift Charts
MIT License
2.04k stars 129 forks source link

Add accessibilityLabel/Value to sample charts #28

Closed mathewa6 closed 2 years ago

mathewa6 commented 2 years ago

Thanks for creating this project! I've gone ahead and started working on Issue #24 by adding accessibilityLabel and accessibilityValue to Line and the Bar charts that did not have the attributes set.

Before I continue with the other charts, I did want to bring up one difference between the simple and detail views that I think is worth exploring - namely that for the simple charts that are previews, it may not make sense to have VoiceOver users traverse all data points inline. Instead in BarChartSimpleOverview I manually set the accessibilityChartDescriptor modifier to only allow audio graph use with the cell label acting as the accessibilityLabel. The detail views continue having the full default SwiftChart accessibility.

Since this is also an example repository that people may copy paste from - I'm not sure of the value of the above differentiation between simple/detail view accessibility. However, I'm happy to make changes either way based on your thoughts and have marked this a draft for now.

jordibruin commented 2 years ago

Great point. I’m not sure what the best practice is for a preview chart in terms of accessibility 🤔 you can use your best judgement for now and we can always change it later if we come up with something better!

mathewa6 commented 2 years ago

Current accessibility status - separate AXChartDescriptor for Overview charts and full per Mark label/value for Detail charts

(I'm incorporating changes from main branch as well):

jordibruin commented 2 years ago

This is lovely @mathewa6. Feel free to submit a PR that is not fully covering everything yet so that it doesn't become too big of a thing for you to finish 🙂

mathewa6 commented 2 years ago

@jordibruin Thanks! Absolutely - I don't want review to be tedious either.

I'll address a couple recent changes on main tomorrow - for example the optimization change to switch to images breaks preview accessibility but it's straightforward refactoring mostly. I'll mark as ready once that's in place with the current charts

mathewa6 commented 2 years ago

@jordibruin Alright - well it was a small hop to get all charts covered and most chart implementation files are untouched except to add the accessibility modifiers and raise the access level of data properties. There are a lot of minor things I'll open new PRs for

Here's a couple videos showing the different dataset summaries as I scrub/pause over each:

https://user-images.githubusercontent.com/5402357/175085251-7af8df09-41bd-4891-9fea-9100f9f791b0.mov

https://user-images.githubusercontent.com/5402357/175086125-a76b8cb9-38fe-4e41-a1e9-923aebc6f25c.mov

Happy to make cleanup/changes/add comments for clarity etc.

mathewa6 commented 2 years ago

Updated to add support for UV index and Candlestick charts.

mathewa6 commented 2 years ago

Updated to work with the .isOverview pattern and add support for LinePlot, GitHubContributions and SoundBars.

It's worth noting that in order to support VoiceOver use with the SoundBars chart I made a few change that are a bit more intrusive than other charts.

Given the relatively low PR activity the past couple days, maybe we can consider review/merging at your convenience. @jordibruin

jordibruin commented 2 years ago

@mathewa6 thanks for all the work with this. I will have to go through this in a bit more detail, but also don't know all the a11y approaches so let's see what the community can find to improve on your work!

Really happy with this, thanks!!

mathewa6 commented 2 years ago

@jordibruin absolutely - happy to address any issues you encounter if you tag me in them! I only addressed VoiceOver a11y so there's always more that can be done.

Thanks for the merge 🥳