Accessibility Support for (most) Chart types #3520

mathewa6 · 2018-06-20T00:03:26Z

Issue Link 🔗

Goals ⚽

This PR adds VoiceOver (VO) support to the following chart types:

Line Chart
Bar Chart
Pie Chart
Bubble Chart
Radar Chart
CandleStick Chart

Implementation Details 🚧

This PR aimed to minimize intrusion with rendering or drawing of Charts as possible while still remaining usable. I'm leaving out support for Combined charts and Scatter charts for now for the same reason, as I think they need a bit more work to be made accessible in a usable manner (i.e. without an information overload to a VO user).

Accessibility is added as extensions to NSUIView, which is done in Platforms+Accessibility.swift to abstract differences between macOS/iOS. With that done, all individual ChartRenderers need to do is populate accessibleChartElements property of NSUIView during drawing. Some chart types like Bar Charts which can have multiple datasets/stacks use an accessibilityOrderedElements array internally to reorder elements to make more sense when using VO, rather than in drawing order. There are some minor optional properties in ChartData.swift to help make data sound better when presented over VO such as prefixes, suffixes to each data point.

Testing Details 🔍

No changes to drawing, so none added. Manual on-device testing & audited for usability at WWDC.

This commit makes PieChartView adhere to the UIAccessibility container protocol. See the Accessibility section marked for the methods. To allow conformance, 3 properties are added to IPieChartDataSet and PieChartDataSet that enable a more user friendly audio descriptionfor data elements. PieChartRenderer has a new private method createAccessibleElement() that populates the accessiblePieChartElements property, which in turn is used in PieChartView. Note that to prevent contextless audio descriptions, the default Chart Description header text was deleted in Description.swift since it is already an optional.

Minor changes to spacing in PieChartRenderer.swift. Removed formatting and use of "self." to match library style.

Updated ChartViewBase, ChartData and ChartDataRendererBase to declare the primary properties required for accessibility support within the Charts library. This includes the UIAccessibility protocol methods within ChartViewBase and the internal accessibleChartElements property in the base renderer. ChartData also has 3 optional properties to allow proper formatting of audio.

Added accessibilityChildren to ChartViewBase which is a layer over both UIAccessibilityContainer and NSAccessibilityGroup protocols. Updated PieChartRenderer to use the platform agnostic NSUIAccessibilityElement. Added init() overrides in NSUIView declaration for macOS to add .list NSAccessibilityRole. Added Platform+Accessibility.swift which extends NSUIView with accessibility container and group protocols and also declares NSUIAccessibilityElement, which acts as an abstraction over NSAccessibilityElement and UIAccessibilityElement.

…lity

Moved Platform+Accessibility.swift to Utils folder. Changed accessibleChartElements to be final since Renderer subclasses should not need to modify its working. Simply populating it in draw() functions will add basic accessibility.

Created internal property accessibilityOrderedElements to make BarChartRenderer be composed of logically ordered accessible elements (See inline comments for details). Updated ChartDataRendererBase, PieChartRenderer and Platform+Accessibility with updated comments to reflect the platform agnostic NSUIAccessibilityElement's use.

Updated HorizontalBarChartRenderer to populate accessibilityOrderedElements, which in turn is used by the BarChartRenderer superclass to enable accessibility. Added minor note to BarChartRenderer's createAccesibleElement() for edge case where x-axis labels can be inaccurate if multiple data sets are present.

LineChartRenderer now has a private accessibilityOrderedElements nested array that is then used to populate accessibleChartElements. Do note that the nesting is unnecessary for now, but will be needed once a custom rotor is added. Also unlike most other renderers, LineChartRenderer's accessibleChartElements is populated in drawCircles(). This required moving the check for isDrawCirclesEnabled() after accessibleChartElements are populated.

Updated BubbleChartRenderer to mirror LineChartRenderer's nested use of accessibilityOrderedElements to populate accessibleChartElements. Minor updates to comments in LineChartRenderer.

Updated RadarChartRenderer with accessibility properties and calls in drawData and drawDataSet. Due to the unique spatial arrangement of radar charts, accessibleChartElements is populated by dataset, within which variables are ordered in decreasing order.

Fixed crash due to incorrect use of maxEntryCountSet instead of dataSetCount in BubbleChartRenderer and LineChartRenderer's accessibilityCreateEmptyOrderedElements(). Updated BubbleChartRenderer's bubble accessibilityLabel to include percentage size of bubble based on maxSize property of IBubbleChartDataSet.

Updated CandleStickChartRenderer to populate accessibleChartElements. Unlike most other renderers with multiple dataSet support, we do not attempt to order elements logically and hence dataSets are presented to VO in the same order they are drawn with the dataSet label acting as a separating heading.

Added a workaround for non updating accessibility frame when resizing windows and using Charts on macOS by using setAccessibilityFrameInParent() in Platform+Accessibility. See inline comments for details.

codecov-io · 2018-06-20T00:13:50Z

Codecov Report

Merging #3520 into master will increase coverage by 0.67%.
The diff coverage is 50.22%.

@@            Coverage Diff             @@
##           master    #3520      +/-   ##
==========================================
+ Coverage   29.26%   29.94%   +0.67%     
==========================================
  Files         117      118       +1     
  Lines       13277    13793     +516     
==========================================
+ Hits         3886     4130     +244     
- Misses       9391     9663     +272

Impacted Files	Coverage Δ
...ata/Implementations/Standard/PieChartDataSet.swift	`35.71% <ø> (ø)`	⬆️
...arts/Data/Implementations/Standard/ChartData.swift	`41.74% <ø> (+2.75%)`	⬆️
Source/Charts/Utils/Platform.swift	`15.38% <ø> (ø)`	⬆️
Source/Charts/Components/Description.swift	`100% <ø> (ø)`	⬆️
Source/Charts/Renderers/ScatterChartRenderer.swift	`0% <0%> (ø)`	⬆️
Source/Charts/Renderers/BubbleChartRenderer.swift	`0% <0%> (ø)`	⬆️
Source/Charts/Renderers/RadarChartRenderer.swift	`0% <0%> (ø)`	⬆️
Source/Charts/Charts/ChartViewBase.swift	`21.59% <0%> (-3.53%)`	⬇️
.../Charts/Renderers/HorizontalBarChartRenderer.swift	`0% <0%> (ø)`	⬆️
...ce/Charts/Renderers/CandleStickChartRenderer.swift	`0% <0%> (ø)`	⬆️
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0d33db4...71f5c34. Read the comment docs.

pmairoldi · 2018-06-20T00:29:53Z

Do you have any idea why voice over is saying "padding" when going through the datasets?

pmairoldi · 2018-06-20T00:32:35Z

Great work btw. Seems to work good with the demos.

pmairoldi

Was there a reason for omitting the other chart type?

Note: We should also localize the voiceover strings. To be done in a future pull request.

pmairoldi · 2018-06-20T00:51:40Z

Source/Charts/Renderers/BarChartRenderer.swift

+            let dataSetCount = barData.dataSets.count
+            let
+            element = NSUIAccessibilityElement(accessibilityContainer: chart)
+            element.accessibilityLabel = chartDescriptionText + ". \(dataSetCount) dataset\(dataSetCount == 1 ? "" : "s"). \(dataSetDescriptionText)"


The screen reader reads "padding" at the end of the label.

pmairoldi · 2018-06-20T00:58:59Z

Source/Charts/Renderers/BubbleChartRenderer.swift

+            let dataSetCount = bubbleData.dataSets.count
+            let
+            element = NSUIAccessibilityElement(accessibilityContainer: chart)
+            element.accessibilityLabel = chartDescriptionText + ". \(dataSetCount) dataset\(dataSetCount == 1 ? "" : "s"). \(dataSetDescriptionText)"


The screen reader reads "padding" at the end of the label.

pmairoldi · 2018-06-20T00:59:49Z

Source/Charts/Renderers/CandleStickChartRenderer.swift

+
+        let
+        element = NSUIAccessibilityElement(accessibilityContainer: chart)
+        element.accessibilityLabel = "Candle Stick chart: " + description + ". \(entryCount) \(prefix + (entryCount == 1 ? "" : "s"))"


The screen reader reads "padding" at the end of the label.

pmairoldi · 2018-06-20T01:03:59Z

Source/Charts/Renderers/LineChartRenderer.swift

+            let dataSetCount = lineData.dataSets.count
+            let
+            element = NSUIAccessibilityElement(accessibilityContainer: chart)
+            element.accessibilityLabel = chartDescriptionText + ". \(dataSetCount) dataset\(dataSetCount == 1 ? "" : "s"). \(dataSetDescriptionText)"


The screen reader reads "padding" at the end of the label.

pmairoldi · 2018-06-20T01:04:46Z

Source/Charts/Renderers/PieChartRenderer.swift

+
+        let
+        element = NSUIAccessibilityElement(accessibilityContainer: chart)
+        element.accessibilityLabel = description + ". \(entryCount) \(prefix + (entryCount == 1 ? "" : "s"))"


The screen reader reads "padding" at the end of the label.

pmairoldi · 2018-06-20T01:09:50Z

Source/Charts/Renderers/RadarChartRenderer.swift

+                let dataSetCount = radarData!.dataSets.count
+                let
+                element = NSUIAccessibilityElement(accessibilityContainer: chart)
+                element.accessibilityLabel = chartDescriptionText + ". \(dataSetCount) dataset\(dataSetCount == 1 ? "" : "s")"


The screen reader reads "padding" at the end of the label.

pmairoldi · 2018-06-20T01:10:15Z

Source/Charts/Renderers/RadarChartRenderer.swift

+            self.accessibleChartElements.removeAll()
+
+            // Make the chart header the first element in the accessible elements array
+            if let chartDescriptionText: String = chart.chartDescription?.text {


Why does this chart not say the datasets in the chart description is present?

I believe this was because, for Radar chart, each individual dataSet is a heading element, so it's the next item in VO scroll order. Can definitely be added here, but I thought it was redundant, so let me know if you think I should add it.

pmairoldi · 2018-06-20T01:11:20Z

Source/Charts/Renderers/RadarChartRenderer.swift

+        // Otherwise, there is no non-visual logic to the data presented
+        let accessibilityEntryValues =  Array(0 ..< entryCount).map { (dataSet.entryForIndex($0)?.y ?? 0, $0) }
+        let accessibilityAxisLabelValueTuples = zip(accessibilityXLabels, accessibilityEntryValues).map { ($0, $1.0, $1.1) }.sorted { $0.1 > $1.1 }
+        let accessibilityDataSetDescription: String = description + ". \(entryCount) \(prefix + (entryCount == 1 ? "" : "s")). "


The screen reader reads "padding" at the end of the label.

pmairoldi · 2018-06-20T01:14:30Z

Source/Charts/Renderers/BarChartRenderer.swift

@@ -18,6 +18,28 @@ import CoreGraphics

 open class BarChartRenderer: BarLineScatterCandleBubbleRenderer


In the demo project the "Another Bar Chart" demo has a frame that is too large around the bars.

I see it! Will fix tomorrow.

mathewa6 · 2018-06-20T02:01:57Z

Thanks!

Regarding omission of Scatter & Combined charts: the same method using accessibleChartElements can be used, but almost all the folks we tested with found it challenging to use (We tested most charts by showing a simple example and asking questions like "Can you find the day when x was smallest?"). So I left that for another PR: since a "summarizing" method will probably need fiddling with the rendering to allow parsing by axis values rather than by chart data itself.
Now that you mention it: I will add the chart itself as an accessibilityElement, so that at least a label/description can be added in the case of Scatter & combined.

I will look into the "padding" issue: not sure why that occurs.

Added createAccessibleHeader() to ChartRendererBase.swift that is used to create a descriptive header for all subclasses based on dataSet count and labels. RadarChartRenderer and PieChartRenderer updated to add dataSet label to each item and with documentation respectively.

Added an offset to BarChartRenderer's barRect calculation in prepareBuffer() to prevent calculation of rects outside visible chart area, while still allowing automatic offset of the axis minima visually. This workaround is only used when using auto calculated y-axis minima and isn't used when a custom minimum is manually set.

mathewa6 · 2018-06-20T23:54:48Z

@petester42

Alright! I've addressed the rects in the AnotherBarChart sample and made the headers/titles more consistent overall.

I cannot reproduce the "padding" issue. The word padding doesn't even appear in the project (excluding comments/variables) so I'm a little lost.
Is there a specific config you are using? Perhaps traverse the element by character using VO and see if indeed it is saying "padding" and I can look over what string it is saying?

pmairoldi · 2018-06-26T01:48:05Z

Thanks for the changes. I still have the padding issue. I don't have any special settings, I just turned on voiceover without changing anything (iOS 11.4) and was trying out navigating by swiping through elements. If you can't find out why we can merge and then open an issue if other people have this problem.

pmairoldi · 2018-06-26T01:58:01Z

Is it possible that it is reading the CGRect for the frame?

mathewa6 · 2018-06-26T13:46:55Z

Interesting. I will play with it again and see if it could be the frame or perhaps a superclass' label. It doesn't occur on 11.4.1 beta or 12 beta. Will test on 11.4 as well to be sure.

If you or another reviewer can try on another device and it still occurs, probably needs to be fixed before merge.
I'm wondering though, if it's somehow distorting the "Heading" text it should be saying. Most of these charts have the top level or multiple elements (Radar ch) as heading, which causes VO to explicitly append "heading" to the end of the label. I'm imagining it is messing it up somehow and saying "padding". That's a funny bug to have 😄

pmairoldi · 2018-06-26T14:11:49Z

It is a weird one for sure. I tried debugging it for a bit and my only thought would be that it is reading the whole element rather than just the label. I have an older phone with maybe an older version I’ll try it out there when I have time.

mathewa6 · 2018-06-26T14:18:22Z

Alright, in that case, it might be worth trying to see what happens if you remove any attributes like heading and set the label to some constant string literal. If it still shows then perhaps see if its there in other apps too? FYI it doesn't occur on 11.4 for me either.

Thanks for looking into it and sorry about this!

pmairoldi · 2018-06-26T14:20:19Z

Alright. Thanks. For looking into this. I’ll check stuff out tonight and if it’s only my phone then I’ll merge. We can fix it later if it is an issue.

Updated return value for the index(of:) function to be NSNotFound in Platform+Accessibility's iOS section. This is as required by the documentation for UIAccessibilityContainer protocol.

pmairoldi · 2018-07-06T13:28:25Z

I couldn’t find out why. We can open an issue if people have problems.

mathewa6 · 2018-07-06T13:40:31Z

That's fine. I couldn't make it happen on another device either so I'm not too concerned.

I'll be opening another PR for panning support with VO and the remaining chart types, so if it does become an issue I'd be willing to look again.

Thanks for the merge!

mathewa6 added 16 commits May 10, 2018 13:10

Formatting & cleanup in PieChartRenderer. (ChartsOrg#1060)

2f00321

Minor changes to spacing in PieChartRenderer.swift. Removed formatting and use of "self." to match library style.

Merge branch 'master' of github.com:danielgindi/Charts into accessibi…

557d0d3

…lity

Fixed missing accessibility file.

bce41d3

Added accessibility for Bubble charts. (ChartsOrg#1060)

2329bd7

Updated BubbleChartRenderer to mirror LineChartRenderer's nested use of accessibilityOrderedElements to populate accessibleChartElements. Minor updates to comments in LineChartRenderer.

Merge branch 'master' of github.com:danielgindi/Charts

5a3cc22

Fixed accessibility frame calculation on macOS. (ChartsOrg#1060)

afaf9e5

Added a workaround for non updating accessibility frame when resizing windows and using Charts on macOS by using setAccessibilityFrameInParent() in Platform+Accessibility. See inline comments for details.

mathewa6 mentioned this pull request Jun 20, 2018

Add Accessibility support for charts. #1060

Open

pmairoldi self-requested a review June 20, 2018 00:29

pmairoldi requested changes Jun 20, 2018

View reviewed changes

mathewa6 added 2 commits June 20, 2018 12:16

Minor change to adhere to UIAccessibility docs. (ChartsOrg#1060)

71f5c34

Updated return value for the index(of:) function to be NSNotFound in Platform+Accessibility's iOS section. This is as required by the documentation for UIAccessibilityContainer protocol.

pmairoldi merged commit fafd58c into ChartsOrg:master Jul 6, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Accessibility Support for (most) Chart types #3520

Accessibility Support for (most) Chart types #3520

mathewa6 commented Jun 20, 2018

codecov-io commented Jun 20, 2018 •

edited

Loading

pmairoldi commented Jun 20, 2018

pmairoldi commented Jun 20, 2018

pmairoldi left a comment

pmairoldi Jun 20, 2018

pmairoldi Jun 20, 2018

pmairoldi Jun 20, 2018

pmairoldi Jun 20, 2018

pmairoldi Jun 20, 2018

pmairoldi Jun 20, 2018

pmairoldi Jun 20, 2018

mathewa6 Jun 20, 2018

pmairoldi Jun 20, 2018

pmairoldi Jun 20, 2018

mathewa6 Jun 20, 2018

mathewa6 commented Jun 20, 2018

mathewa6 commented Jun 20, 2018

pmairoldi commented Jun 26, 2018

pmairoldi commented Jun 26, 2018

mathewa6 commented Jun 26, 2018

pmairoldi commented Jun 26, 2018

mathewa6 commented Jun 26, 2018 •

edited

Loading

pmairoldi commented Jun 26, 2018

pmairoldi commented Jul 6, 2018

mathewa6 commented Jul 6, 2018

		@@ -18,6 +18,28 @@ import CoreGraphics

		open class BarChartRenderer: BarLineScatterCandleBubbleRenderer

Accessibility Support for (most) Chart types #3520

Accessibility Support for (most) Chart types #3520

Conversation

mathewa6 commented Jun 20, 2018

Issue Link 🔗

Goals ⚽

Implementation Details 🚧

Testing Details 🔍

codecov-io commented Jun 20, 2018 • edited Loading

Codecov Report

pmairoldi commented Jun 20, 2018

pmairoldi commented Jun 20, 2018

pmairoldi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mathewa6 commented Jun 20, 2018

mathewa6 commented Jun 20, 2018

pmairoldi commented Jun 26, 2018

pmairoldi commented Jun 26, 2018

mathewa6 commented Jun 26, 2018

pmairoldi commented Jun 26, 2018

mathewa6 commented Jun 26, 2018 • edited Loading

pmairoldi commented Jun 26, 2018

pmairoldi commented Jul 6, 2018

mathewa6 commented Jul 6, 2018

codecov-io commented Jun 20, 2018 •

edited

Loading

mathewa6 commented Jun 26, 2018 •

edited

Loading