From dc2d52c1f2f01102a1eef7576ff6ea8c58b13d22 Mon Sep 17 00:00:00 2001
From: Kerry <kerrya@element.io>
Date: Thu, 6 Jan 2022 18:52:06 +0100
Subject: [PATCH] a11y - wrap notification level radios in fieldsets (#7471)

* notification settings radios table -> fieldset

Signed-off-by: Kerry Archibald <kerrya@element.io>

* aria-label for inputs

Signed-off-by: Kerry Archibald <kerrya@element.io>

* update tests for fielset

Signed-off-by: Kerry Archibald <kerrya@element.io>

* remove unneccessary wrapping div

Signed-off-by: Kerry Archibald <kerrya@element.io>

* fix stylelint

Signed-off-by: Kerry Archibald <kerrya@element.io>
---
 res/css/views/settings/_Notifications.scss    | 85 ++++++++++---------
 .../views/settings/Notifications.tsx          | 42 +++++----
 .../views/settings/Notifications-test.tsx     |  4 +-
 3 files changed, 65 insertions(+), 66 deletions(-)

diff --git a/res/css/views/settings/_Notifications.scss b/res/css/views/settings/_Notifications.scss
index b950256ced..c6c8385368 100644
--- a/res/css/views/settings/_Notifications.scss
+++ b/res/css/views/settings/_Notifications.scss
@@ -14,53 +14,56 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-.mx_UserNotifSettings {
-    color: $primary-content; // override from default settings page styles
+.mx_UserNotifSettings_grid {
+    width: calc(100% + 12px); // +12px to line up center of 'Noisy' column with toggle switches
+    display: grid;
+    grid-template-columns: auto repeat(3, 62px);
+    place-items: center center;
+    grid-gap: 8px;
+    margin-top: $spacing-40;
 
-    .mx_UserNotifSettings_pushRulesTable {
-        width: calc(100% + 12px); // +12px to line up center of 'Noisy' column with toggle switches
-        table-layout: fixed;
-        border-collapse: collapse;
-        border-spacing: 0;
-        margin-top: 40px;
+    // Override StyledRadioButton default styles
+    .mx_StyledRadioButton {
+        justify-content: center;
 
-        tr > th {
-            font-weight: $font-semi-bold;
+        .mx_StyledRadioButton_content {
+            display: none;
         }
 
-        tr > th:first-child {
-            text-align: left;
-            font-size: $font-18px;
-        }
-
-        tr > th:nth-child(n + 2) {
-            color: $secondary-content;
-            font-size: $font-12px;
-            vertical-align: middle;
-            width: 66px;
-        }
-
-        tr > td:nth-child(n + 2) {
-            text-align: center;
-        }
-
-        tr > td {
-            padding-top: 8px;
-        }
-
-        // Override StyledRadioButton default styles
-        .mx_StyledRadioButton {
-            justify-content: center;
-
-            .mx_StyledRadioButton_content {
-                display: none;
-            }
-
-            .mx_StyledRadioButton_spacer {
-                display: none;
-            }
+        .mx_StyledRadioButton_spacer {
+            display: none;
         }
     }
+}
+
+.mx_UserNotifSettings_gridRowContainer {
+    display: contents;
+}
+
+.mx_UserNotifSettings_gridRow {
+    display: contents;
+}
+
+.mx_UserNotifSettings_gridRowLabel {
+    justify-self: start;
+    // <legend> does not accept
+    // display: inline | inline-block
+    // force it inline using float
+    float: left;
+}
+.mx_UserNotifSettings_gridRowHeading {
+    font-size: $font-18px;
+    font-weight: $font-semi-bold;
+}
+
+.mx_UserNotifSettings_gridColumnLabel {
+    color: $secondary-content;
+    font-size: $font-12px;
+    font-weight: $font-semi-bold;
+}
+
+.mx_UserNotifSettings {
+    color: $primary-content; // override from default settings page styles
 
     .mx_UserNotifSettings_floatingSection {
         margin-top: 40px;
diff --git a/src/components/views/settings/Notifications.tsx b/src/components/views/settings/Notifications.tsx
index 658d02a3f7..d86d6e7c7b 100644
--- a/src/components/views/settings/Notifications.tsx
+++ b/src/components/views/settings/Notifications.tsx
@@ -572,7 +572,7 @@ export default class Notifications extends React.PureComponent<IProps, IState> {
 
         const makeRadio = (r: IVectorPushRule, s: VectorState) => (
             <StyledRadioButton
-                key={r.ruleId}
+                key={r.ruleId + s}
                 name={r.ruleId}
                 checked={r.vectorState === s}
                 onChange={this.onRadioChecked.bind(this, r, s)}
@@ -581,15 +581,17 @@ export default class Notifications extends React.PureComponent<IProps, IState> {
             />
         );
 
-        const rows = this.state.vectorPushRules[category].map(r => <tr
-            data-test-id={category + r.ruleId}
-            key={category + r.ruleId}
-        >
-            <td>{ r.description }</td>
-            <td>{ makeRadio(r, VectorState.Off) }</td>
-            <td>{ makeRadio(r, VectorState.On) }</td>
-            <td>{ makeRadio(r, VectorState.Loud) }</td>
-        </tr>);
+        const fieldsetRows = this.state.vectorPushRules[category].map(r =>
+            <fieldset
+                key={category + r.ruleId}
+                data-test-id={category + r.ruleId}
+                className='mx_UserNotifSettings_gridRowContainer'
+            >
+                <legend className='mx_UserNotifSettings_gridRowLabel'>{ r.description }</legend>
+                { makeRadio(r, VectorState.Off) }
+                { makeRadio(r, VectorState.On) }
+                { makeRadio(r, VectorState.Loud) }
+            </fieldset>);
 
         let sectionName: TranslatedString;
         switch (category) {
@@ -607,19 +609,13 @@ export default class Notifications extends React.PureComponent<IProps, IState> {
         }
 
         return <>
-            <table data-test-id={`notif-section-${category}`} className='mx_UserNotifSettings_pushRulesTable'>
-                <thead>
-                    <tr>
-                        <th>{ sectionName }</th>
-                        <th>{ VectorStateToLabel[VectorState.Off] }</th>
-                        <th>{ VectorStateToLabel[VectorState.On] }</th>
-                        <th>{ VectorStateToLabel[VectorState.Loud] }</th>
-                    </tr>
-                </thead>
-                <tbody>
-                    { rows }
-                </tbody>
-            </table>
+            <div data-test-id={`notif-section-${category}`} className='mx_UserNotifSettings_grid'>
+                <span className='mx_UserNotifSettings_gridRowLabel mx_UserNotifSettings_gridRowHeading'>{ sectionName }</span>
+                <span className='mx_UserNotifSettings_gridColumnLabel'>{ VectorStateToLabel[VectorState.Off] }</span>
+                <span className='mx_UserNotifSettings_gridColumnLabel'>{ VectorStateToLabel[VectorState.On] }</span>
+                <span className='mx_UserNotifSettings_gridColumnLabel'>{ VectorStateToLabel[VectorState.Loud] }</span>
+                { fieldsetRows }
+            </div>
             { clearNotifsButton }
             { keywordComposer }
         </>;
diff --git a/test/components/views/settings/Notifications-test.tsx b/test/components/views/settings/Notifications-test.tsx
index eda2167221..57ba1285c7 100644
--- a/test/components/views/settings/Notifications-test.tsx
+++ b/test/components/views/settings/Notifications-test.tsx
@@ -245,8 +245,8 @@ describe('<Notifications />', () => {
             const section = 'vector_global';
 
             const globalSection = findByTestId(component, `notif-section-${section}`);
-            // 16 notification rules with class 'global'
-            expect(globalSection.find('td').length).toEqual(16);
+            // 4 notification rules with class 'global'
+            expect(globalSection.find('fieldset').length).toEqual(4);
             // oneToOneRule is set to 'on'
             const oneToOneRuleElement = findByTestId(component, section + oneToOneRule.rule_id);
             expect(getCheckedRadioForRule(oneToOneRuleElement)).toEqual('On');