MDL-62680 output: Only hide icons with no label
authorDamyon Wiese <damyon@moodle.com>
Thu, 10 Jan 2019 02:22:48 +0000 (10:22 +0800)
committerDamyon Wiese <damyon@moodle.com>
Thu, 7 Feb 2019 02:08:19 +0000 (10:08 +0800)
For accessibility we don't want to read an icon with a label immediately next to the label,
but in this case it's clearer for the icon to have no alt text / title for both
screen readers and non-screen readers. Worse is not reading important information just
because it's displayed as an icon.

lib/classes/output/icon_system_fontawesome.php
lib/outputcomponents.php
lib/outputrenderers.php
lib/templates/pix_icon_fontawesome.mustache
lib/tests/outputcomponents_test.php
lib/upgrade.txt

index 1530895..c65af9d 100644 (file)
@@ -444,6 +444,9 @@ class icon_system_fontawesome extends icon_system_font {
         if (!$subpix->is_mapped()) {
             $data['unmappedIcon'] = $icon->export_for_template($output);
         }
+        if (isset($icon->attributes['aria-hidden'])) {
+            $data['aria-hidden'] = $icon->attributes['aria-hidden'];
+        }
         return $output->render_from_template('core/pix_icon_fontawesome', $data);
     }
 
index 772385e..7824d23 100644 (file)
@@ -694,6 +694,11 @@ class pix_icon implements renderable, templatable {
             // and some browsers might overwrite it with an empty title.
             unset($this->attributes['title']);
         }
+
+        // Hide icons from screen readers that have no alt.
+        if (empty($this->attributes['alt'])) {
+            $this->attributes['aria-hidden'] = 'true';
+        }
     }
 
     /**
index 6ccfa79..a3d110e 100644 (file)
@@ -3487,7 +3487,7 @@ EOD;
                         // Process this as a link item.
                         $pix = null;
                         if (isset($value->pix) && !empty($value->pix)) {
-                            $pix = new pix_icon($value->pix, $value->title, null, array('class' => 'iconsmall'));
+                            $pix = new pix_icon($value->pix, '', null, array('class' => 'iconsmall'));
                         } else if (isset($value->imgsrc) && !empty($value->imgsrc)) {
                             $value->title = html_writer::img(
                                 $value->imgsrc,
index 7f95b6c..34e74bd 100644 (file)
@@ -43,7 +43,7 @@
 
 }}
 {{^unmappedIcon}}
-<i class="icon fa {{key}} fa-fw {{extraclasses}}" aria-hidden="true" {{#title}}title="{{title}}"{{/title}} aria-label="{{alt}}"></i>
+<i class="icon fa {{key}} fa-fw {{extraclasses}}" {{#aria-hidden}}aria-hidden="true"{{/aria-hidden}} {{#title}}title="{{title}}"{{/title}} {{#alt}}aria-label="{{alt}}"{{/alt}}></i>
 {{/unmappedIcon}}
 {{#unmappedIcon}}
 {{! We cannot include the pix_icon template directly here because we don't have all the mustache helpers loaded. }}
index de5ec1e..67e2c0c 100644 (file)
@@ -436,4 +436,33 @@ EOF;
         $this->assertEquals($expecteda, $pbara->pagelinks);
         $this->assertEquals($expectedb, $pbarb->pagelinks);
     }
+
+    public function test_pix_icon() {
+        $this->resetAfterTest();
+
+        $page = new moodle_page();
+
+        set_config('theme', 'boost');
+        // Need to reset after changing theme.
+        $page->reset_theme_and_output();
+        $renderer = $page->get_renderer('core');
+
+        $reason = 'An icon with no alt text is hidden from screenreaders.';
+        $this->assertContains('aria-hidden="true"', $renderer->pix_icon('t/print', ''), $reason);
+
+        $reason = 'An icon with alt text is not hidden from screenreaders.';
+        $this->assertNotContains('aria-hidden="true"', $renderer->pix_icon('t/print', 'Print'), $reason);
+
+        // Test another theme with a different icon system.
+        set_config('theme', 'clean');
+        // Need to reset after changing theme.
+        $page->reset_theme_and_output();
+        $renderer = $page->get_renderer('core');
+
+        $reason = 'An icon with no alt text is hidden from screenreaders.';
+        $this->assertContains('aria-hidden="true"', $renderer->pix_icon('t/print', ''), $reason);
+
+        $reason = 'An icon with alt text is not hidden from screenreaders.';
+        $this->assertNotContains('aria-hidden="true"', $renderer->pix_icon('t/print', 'Print'), $reason);
+    }
 }
index 1c2f473..4a4aa5e 100644 (file)
@@ -4,6 +4,7 @@ information provided here is intended especially for developers.
 === 3.7 ===
 
 * The method core_user::is_real_user() now returns false for userid = 0 parameter
+* Icons are displayed for screen readers unless they have empty alt text (aria-hidden). Do not provide an icon with alt text immediately beside an element with exactly the same text.
 
 === 3.6 ===