From 68f46298a751b5f8722c4bac66fd7bf78fe84c5f Mon Sep 17 00:00:00 2001
From: Zihua Li <i@zihua.li>
Date: Fri, 25 Aug 2023 13:40:05 +0800
Subject: [PATCH] Add aria labels to toolbar (#3861)

Co-authored-by: Claire Fields <clairefields15@gmail.com>
---
 modules/toolbar.ts                |  6 ++++--
 test/unit/modules/toolbar.spec.ts | 32 +++++++++++++++----------------
 ui/picker.ts                      |  6 +++---
 3 files changed, 23 insertions(+), 21 deletions(-)

diff --git a/modules/toolbar.ts b/modules/toolbar.ts
index eb7f848392..f20d9fcb96 100644
--- a/modules/toolbar.ts
+++ b/modules/toolbar.ts
@@ -192,14 +192,16 @@ class Toolbar extends Module<ToolbarProps> {
 }
 Toolbar.DEFAULTS = {};
 
-function addButton(container: HTMLElement, format: string, value?: unknown) {
+function addButton(container: HTMLElement, format: string, value?: string) {
   const input = document.createElement('button');
   input.setAttribute('type', 'button');
   input.classList.add(`ql-${format}`);
   input.setAttribute('aria-pressed', 'false');
   if (value != null) {
-    // @ts-expect-error
     input.value = value;
+    input.setAttribute('aria-label', `${format}: ${value}`);
+  } else {
+    input.setAttribute('aria-label', format);
   }
   container.appendChild(input);
 }
diff --git a/test/unit/modules/toolbar.spec.ts b/test/unit/modules/toolbar.spec.ts
index 07d07b98fa..a374588ebc 100644
--- a/test/unit/modules/toolbar.spec.ts
+++ b/test/unit/modules/toolbar.spec.ts
@@ -28,8 +28,8 @@ describe('Toolbar', () => {
       addControls(container, ['bold', 'italic']);
       expect(container).toEqualHTML(`
         <span class="ql-formats">
-          <button type="button" class="ql-bold" aria-pressed="false"></button>
-          <button type="button" class="ql-italic" aria-pressed="false"></button>
+          <button type="button" aria-label="bold" class="ql-bold" aria-pressed="false"></button>
+          <button type="button" aria-label="italic" class="ql-italic" aria-pressed="false"></button>
         </span>
       `);
     });
@@ -42,12 +42,12 @@ describe('Toolbar', () => {
       ]);
       expect(container).toEqualHTML(`
         <span class="ql-formats">
-          <button type="button" class="ql-bold" aria-pressed="false"></button>
-          <button type="button" class="ql-italic" aria-pressed="false"></button>
+          <button type="button" aria-label="bold" class="ql-bold" aria-pressed="false"></button>
+          <button type="button" aria-label="italic" class="ql-italic" aria-pressed="false"></button>
         </span>
         <span class="ql-formats">
-          <button type="button" class="ql-underline" aria-pressed="false"></button>
-          <button type="button" class="ql-strike" aria-pressed="false"></button>
+          <button type="button" aria-label="underline" class="ql-underline" aria-pressed="false"></button>
+          <button type="button" aria-label="strike" class="ql-strike" aria-pressed="false"></button>
         </span>
       `);
     });
@@ -57,8 +57,8 @@ describe('Toolbar', () => {
       addControls(container, ['bold', { header: '2' }]);
       expect(container).toEqualHTML(`
         <span class="ql-formats">
-          <button type="button" class="ql-bold" aria-pressed="false"></button>
-          <button type="button" class="ql-header" aria-pressed="false" value="2"></button>
+          <button type="button" aria-label="bold" class="ql-bold" aria-pressed="false"></button>
+          <button type="button" aria-label="header: 2" class="ql-header" aria-pressed="false" value="2"></button>
         </span>
       `);
     });
@@ -108,14 +108,14 @@ describe('Toolbar', () => {
           </select>
         </span>
         <span class="ql-formats">
-          <button type="button" class="ql-bold" aria-pressed="false"></button>
-          <button type="button" class="ql-italic" aria-pressed="false"></button>
-          <button type="button" class="ql-underline" aria-pressed="false"></button>
-          <button type="button" class="ql-strike" aria-pressed="false"></button>
+          <button type="button" aria-label="bold" class="ql-bold" aria-pressed="false"></button>
+          <button type="button" aria-label="italic" class="ql-italic" aria-pressed="false"></button>
+          <button type="button" aria-label="underline" class="ql-underline" aria-pressed="false"></button>
+          <button type="button" aria-label="strike" class="ql-strike" aria-pressed="false"></button>
         </span>
         <span class="ql-formats">
-          <button type="button" class="ql-list" value="ordered" aria-pressed="false"></button>
-          <button type="button" class="ql-list" value="bullet" aria-pressed="false"></button>
+          <button type="button" aria-label="list: ordered" class="ql-list" value="ordered" aria-pressed="false"></button>
+          <button type="button" aria-label="list: bullet" class="ql-list" value="bullet" aria-pressed="false"></button>
           <select class="ql-align">
             <option selected="selected"></option>
             <option value="center"></option>
@@ -124,8 +124,8 @@ describe('Toolbar', () => {
           </select>
         </span>
         <span class="ql-formats">
-          <button type="button" class="ql-link" aria-pressed="false"></button>
-          <button type="button" class="ql-image" aria-pressed="false"></button>
+          <button type="button" aria-label="link" class="ql-link" aria-pressed="false"></button>
+          <button type="button" aria-label="image" class="ql-image" aria-pressed="false"></button>
         </span>
       `);
     });
diff --git a/ui/picker.ts b/ui/picker.ts
index cfefb4bc7e..d6b387c1a4 100644
--- a/ui/picker.ts
+++ b/ui/picker.ts
@@ -54,9 +54,9 @@ class Picker {
     item.tabIndex = '0';
     item.setAttribute('role', 'button');
     item.classList.add('ql-picker-item');
-    if (option.hasAttribute('value')) {
-      // @ts-expect-error Fix me later
-      item.setAttribute('data-value', option.getAttribute('value'));
+    const value = option.getAttribute('value');
+    if (value) {
+      item.setAttribute('data-value', value);
     }
     if (option.textContent) {
       item.setAttribute('data-label', option.textContent);