Tentative fix for the svgDescription method

dc-js · Aug 19, 2020 · 4d95f74 · 4d95f74
1 parent 60401fa
commit 4d95f74
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 23 deletions.
diff --git a/spec/bar-chart-spec.js b/spec/bar-chart-spec.js
@@ -1288,11 +1288,6 @@ describe('dc.BarChart', () => {
             };
         }
 
-        it('default description should match class name', () => {
-            chart.render()
-            expect(chart.svg().node().firstChild.innerHTML).toEqual('BarChart');
-        });
-
         it('internal elements are focusable by keyboard', () => {
             chart.keyboardAccessible(true);
             chart.render();

diff --git a/spec/base-mixin-spec.js b/spec/base-mixin-spec.js
@@ -680,19 +680,31 @@ describe('dc.baseMixin', () => {
 
     describe('accessibility base svg', () => {
 
-        beforeEach(() => {
+        it('should have default description when keyboardAccessible is true', () => {
             chart
-                .svgDescription('I am a chart')
+                .keyboardAccessible(true)
                 .resetSvg();
-        });
 
-        it('should have a tabindex', () => {
             expect(chart.svg().attr('tabindex')).toEqual('0');
+            expect(chart.svg().node().firstChild.innerHTML).toEqual('BaseMixin');
         });
 
-        it('should have a description for AT', () => {
+        it('should have custom description if svgDescription is set', () => {
+            chart
+                .svgDescription('I am a chart')
+                .resetSvg();
+
+            expect(chart.svg().attr('tabindex')).toEqual('0');
             expect(chart.svg().node().firstChild.innerHTML).toEqual('I am a chart');
         });
 
+        it('should not have accessibility features if not explicitly enabled', () => {
+            chart
+                .resetSvg();
+
+            expect(chart.svg().attr('tabindex')).toBeNull();
+        });
+
+
     })
 });
diff --git a/src/base/base-mixin.js b/src/base/base-mixin.js
@@ -72,7 +72,7 @@ const _defaultResetFilterHandler = filters => [];
 export class BaseMixin {
     constructor () {
         this.__dcFlag__ = utils.uniqueId();
-        this._svgDescription = this.constructor.name || '';
+        this._svgDescription = this.svgDescription()
         this._keyboardAccessible = false;
 
         this._dimension = undefined;
@@ -523,35 +523,49 @@ export class BaseMixin {
 
     generateSvg () {
         this._svg = this.root().append('svg');
+
+        if (this.svgDescription.userSet || this._keyboardAccessible) {
 
-        this._svg.append('desc')
-            .attr('id', `desc-id-${this.__dcFlag__}`)
-            .html(`${this._svgDescription}`);
+            this._svg.append('desc')
+                .attr('id', `desc-id-${this.__dcFlag__}`)
+                .html(`${this._svgDescription}`);
+
+            this._svg
+                .attr('tabindex', '0')
+                .attr('role', 'img')
+                .attr('aria-labelledby', `desc-id-${this.__dcFlag__}`);
+        }
 
-        this._svg
-            .attr('tabindex', '0')
-            .attr('role', 'img')
-            .attr('aria-labelledby', `desc-id-${this.__dcFlag__}`);
         this.sizeSvg();
         return this._svg;
     }
 
     /**
-     * Set description text for the entire SVG graphic to be read out by assistive technologies. By default
-     * will try to set the description to the parent chart's class constructor name: BarChart, HeatMap, etc.
-     * @param {String} [description='']
+     * Set description text for the entire SVG graphic to be read out by assistive technologies and make
+     * the SVG focusable from keyboard. Calling this function without any arguments is not intended and
+     * will make it **not chainable**.
+     * @param {String} [description]
      * @returns {String|BaseMixin}
      */
     svgDescription (description) {
+
         if (!arguments.length) {
-            return this._svgDescription;
+            this.svgDescription.userSet = false;
+            return this.constructor.name;
         }
+
+        this.svgDescription.userSet = true;
         this._svgDescription = description;
         return this;
+
     }
 
     /**
-     * Whether interactive chart elements will be accessible by keyboard using tabindex.
+     * If set, interactive chart elements like individual bars in a bar chart or symbols in a scatter plot
+     * will be focusable from keyboard and on pressing Enter or Space will behave as if clicked on.
+     * 
+     * If `svgDescription` has not been explicitly set, will also set SVG description text to the class
+     * constructor name, like BarChart or HeatMap, and make the entire SVG focusable.
      * @param {Boolean} [keyboardAccessible=false]
      * @returns {Boolean|BarChart}
      */