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

@@ -14,39 +14,13 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-.mx_UserNotifSettings {
+.mx_UserNotifSettings_grid {
-    color: $primary-content; // override from default settings page styles
-
-    .mx_UserNotifSettings_pushRulesTable {
     width: calc(100% + 12px); // +12px to line up center of 'Noisy' column with toggle switches
-        table-layout: fixed;
+    display: grid;
-        border-collapse: collapse;
+    grid-template-columns: auto repeat(3, 62px);
-        border-spacing: 0;
+    place-items: center center;
-        margin-top: 40px;
+    grid-gap: 8px;
-
+    margin-top: $spacing-40;
-        tr > th {
-            font-weight: $font-semi-bold;
-        }
-
-        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 {
@@ -60,7 +34,36 @@ limitations under the License.
             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;

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
+        const fieldsetRows = this.state.vectorPushRules[category].map(r =>
-            data-test-id={category + r.ruleId}
+            <fieldset
                 key={category + r.ruleId}
+                data-test-id={category + r.ruleId}
+                className='mx_UserNotifSettings_gridRowContainer'
             >
-            <td>{ r.description }</td>
+                <legend className='mx_UserNotifSettings_gridRowLabel'>{ r.description }</legend>
-            <td>{ makeRadio(r, VectorState.Off) }</td>
+                { makeRadio(r, VectorState.Off) }
-            <td>{ makeRadio(r, VectorState.On) }</td>
+                { makeRadio(r, VectorState.On) }
-            <td>{ makeRadio(r, VectorState.Loud) }</td>
+                { makeRadio(r, VectorState.Loud) }
-        </tr>);
+            </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'>
+            <div data-test-id={`notif-section-${category}`} className='mx_UserNotifSettings_grid'>
-                <thead>
+                <span className='mx_UserNotifSettings_gridRowLabel mx_UserNotifSettings_gridRowHeading'>{ sectionName }</span>
-                    <tr>
+                <span className='mx_UserNotifSettings_gridColumnLabel'>{ VectorStateToLabel[VectorState.Off] }</span>
-                        <th>{ sectionName }</th>
+                <span className='mx_UserNotifSettings_gridColumnLabel'>{ VectorStateToLabel[VectorState.On] }</span>
-                        <th>{ VectorStateToLabel[VectorState.Off] }</th>
+                <span className='mx_UserNotifSettings_gridColumnLabel'>{ VectorStateToLabel[VectorState.Loud] }</span>
-                        <th>{ VectorStateToLabel[VectorState.On] }</th>
+                { fieldsetRows }
-                        <th>{ VectorStateToLabel[VectorState.Loud] }</th>
+            </div>
-                    </tr>
-                </thead>
-                <tbody>
-                    { rows }
-                </tbody>
-            </table>
             { clearNotifsButton }
             { keywordComposer }
         </>;

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'
+            // 4 notification rules with class 'global'
-            expect(globalSection.find('td').length).toEqual(16);
+            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');