From 69171a75a74f150408485cc35a90d38b1fce79ba Mon Sep 17 00:00:00 2001
From: olav <mail@olav.io>
Date: Fri, 17 Jun 2022 11:27:54 +0200
Subject: [PATCH] refactor: improve button label markup (#1091)

* refactor: improve button label markup

* refactor: remove misused tooltip roles

* refactor: simplify FeatureStrategyIcon labelling

* refactor: simplify labels for disabled buttons

* refactor: add missing switch input labels
---
 .../component/common/HelpIcon/HelpIcon.tsx    |  1 -
 .../PermissionButton/PermissionButton.tsx     |  2 +-
 .../PermissionIconButton.tsx                  |  2 +-
 .../cells/FeatureSeenCell/FeatureSeenCell.tsx |  1 -
 .../EnvironmentActionCell.tsx                 | 22 ++++----
 .../FeatureStrategyIcon.tsx                   |  6 +--
 .../FeatureStrategyMenu.tsx                   |  2 +-
 .../StrategyDeleteButton.tsx                  |  7 ++-
 .../StrategyEditButton/StrategyEditButton.tsx | 51 ++++++++++---------
 .../StrategySwitch/StrategySwitch.tsx         |  3 +-
 .../__snapshots__/TagTypeList.test.tsx.snap   | 12 +----
 11 files changed, 53 insertions(+), 56 deletions(-)

diff --git a/frontend/src/component/common/HelpIcon/HelpIcon.tsx b/frontend/src/component/common/HelpIcon/HelpIcon.tsx
index a3da995f80..502be077c5 100644
--- a/frontend/src/component/common/HelpIcon/HelpIcon.tsx
+++ b/frontend/src/component/common/HelpIcon/HelpIcon.tsx
@@ -17,7 +17,6 @@ export const HelpIcon = ({ tooltip, style }: IHelpIconProps) => {
                 className={styles.container}
                 style={style}
                 tabIndex={0}
-                role="tooltip"
                 aria-label="Help"
             >
                 <Info className={styles.icon} />
diff --git a/frontend/src/component/common/PermissionButton/PermissionButton.tsx b/frontend/src/component/common/PermissionButton/PermissionButton.tsx
index 4e2b03ddb5..0882d40a7b 100644
--- a/frontend/src/component/common/PermissionButton/PermissionButton.tsx
+++ b/frontend/src/component/common/PermissionButton/PermissionButton.tsx
@@ -72,7 +72,7 @@ const PermissionButton: React.FC<IPermissionButtonProps> = ({
                 <Button
                     onClick={onClick}
                     disabled={disabled || !access}
-                    aria-describedby={id}
+                    aria-labelledby={id}
                     variant={variant}
                     color={color}
                     {...rest}
diff --git a/frontend/src/component/common/PermissionIconButton/PermissionIconButton.tsx b/frontend/src/component/common/PermissionIconButton/PermissionIconButton.tsx
index 7b5768e174..aa3c44c3c0 100644
--- a/frontend/src/component/common/PermissionIconButton/PermissionIconButton.tsx
+++ b/frontend/src/component/common/PermissionIconButton/PermissionIconButton.tsx
@@ -61,7 +61,7 @@ const PermissionIconButton = ({
             arrow
             onClick={e => e.preventDefault()}
         >
-            <div id={id} role="tooltip">
+            <div id={id}>
                 <IconButton
                     {...rest}
                     disabled={!access || disabled}
diff --git a/frontend/src/component/common/Table/cells/FeatureSeenCell/FeatureSeenCell.tsx b/frontend/src/component/common/Table/cells/FeatureSeenCell/FeatureSeenCell.tsx
index b3f9be9e6a..9c8f83abc5 100644
--- a/frontend/src/component/common/Table/cells/FeatureSeenCell/FeatureSeenCell.tsx
+++ b/frontend/src/component/common/Table/cells/FeatureSeenCell/FeatureSeenCell.tsx
@@ -66,7 +66,6 @@ const Wrapper: FC<{ unit?: string; tooltip: string }> = ({
         <div className={styles.container}>
             <Tooltip title={tooltip} arrow describeChild>
                 <div
-                    role="tooltip"
                     className={styles.box}
                     style={{ background: getColor(unit) }}
                     data-loading
diff --git a/frontend/src/component/environments/EnvironmentTable/EnvironmentActionCell/EnvironmentActionCell.tsx b/frontend/src/component/environments/EnvironmentTable/EnvironmentActionCell/EnvironmentActionCell.tsx
index baf4ca0b9c..5fbd3716c4 100644
--- a/frontend/src/component/environments/EnvironmentTable/EnvironmentActionCell/EnvironmentActionCell.tsx
+++ b/frontend/src/component/environments/EnvironmentTable/EnvironmentActionCell/EnvironmentActionCell.tsx
@@ -99,6 +99,7 @@ export const EnvironmentActionCell = ({
         ? `Disable environment ${environment.name}`
         : `Enable environment ${environment.name}`;
 
+    const toggleId = useId();
     const editId = useId();
     const deleteId = useId();
 
@@ -108,13 +109,16 @@ export const EnvironmentActionCell = ({
                 condition={updatePermission}
                 show={
                     <>
-                        <Tooltip title={toggleIconTooltip} arrow describeChild>
-                            <PermissionSwitch
-                                permission={UPDATE_ENVIRONMENT}
-                                checked={environment.enabled}
-                                onClick={() => setToggleModal(true)}
-                                disabled={environment.protected}
-                            />
+                        <Tooltip title={toggleIconTooltip} arrow>
+                            <div id={toggleId}>
+                                <PermissionSwitch
+                                    permission={UPDATE_ENVIRONMENT}
+                                    checked={environment.enabled}
+                                    onClick={() => setToggleModal(true)}
+                                    disabled={environment.protected}
+                                    inputProps={{ 'aria-labelledby': toggleId }}
+                                />
+                            </div>
                         </Tooltip>
                         <ActionCell.Divider />
                     </>
@@ -133,7 +137,7 @@ export const EnvironmentActionCell = ({
                     >
                         <span id={editId}>
                             <IconButton
-                                aria-describedby={editId}
+                                aria-labelledby={editId}
                                 disabled={environment.protected}
                                 onClick={() => {
                                     navigate(
@@ -162,7 +166,7 @@ export const EnvironmentActionCell = ({
                     >
                         <span id={deleteId}>
                             <IconButton
-                                aria-describedby={deleteId}
+                                aria-labelledby={deleteId}
                                 disabled={environment.protected}
                                 onClick={() => setDeleteModal(true)}
                                 size="large"
diff --git a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyIcon/FeatureStrategyIcon.tsx b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyIcon/FeatureStrategyIcon.tsx
index a6db5c779d..1665665354 100644
--- a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyIcon/FeatureStrategyIcon.tsx
+++ b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyIcon/FeatureStrategyIcon.tsx
@@ -3,7 +3,6 @@ import {
     formatStrategyName,
 } from 'utils/strategyNames';
 import { styled, Tooltip } from '@mui/material';
-import { useId } from 'hooks/useId';
 
 interface IFeatureStrategyIconProps {
     strategyName: string;
@@ -13,14 +12,11 @@ export const FeatureStrategyIcon = ({
     strategyName,
 }: IFeatureStrategyIconProps) => {
     const Icon = getFeatureStrategyIcon(strategyName);
-    const id = useId();
 
     return (
         <StyledIcon>
             <Tooltip title={formatStrategyName(strategyName)} arrow>
-                <div id={id} role="tooltip">
-                    <Icon aria-labelledby={id} />
-                </div>
+                <Icon />
             </Tooltip>
         </StyledIcon>
     );
diff --git a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyMenu/FeatureStrategyMenu.tsx b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyMenu/FeatureStrategyMenu.tsx
index 754e17ffb7..bb8aa10114 100644
--- a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyMenu/FeatureStrategyMenu.tsx
+++ b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyMenu/FeatureStrategyMenu.tsx
@@ -40,7 +40,7 @@ export const FeatureStrategyMenu = ({
                 projectId={projectId}
                 environmentId={environmentId}
                 onClick={onClick}
-                aria-describedby={popoverId}
+                aria-labelledby={popoverId}
                 variant={variant}
             >
                 {label}
diff --git a/frontend/src/component/strategies/StrategiesList/StrategyDeleteButton/StrategyDeleteButton.tsx b/frontend/src/component/strategies/StrategiesList/StrategyDeleteButton/StrategyDeleteButton.tsx
index 6e7ffd092d..0e9291fef3 100644
--- a/frontend/src/component/strategies/StrategiesList/StrategyDeleteButton/StrategyDeleteButton.tsx
+++ b/frontend/src/component/strategies/StrategiesList/StrategyDeleteButton/StrategyDeleteButton.tsx
@@ -5,6 +5,7 @@ import { Delete } from '@mui/icons-material';
 import { IconButton, Tooltip } from '@mui/material';
 import { IStrategy } from 'interfaces/strategy';
 import { DELETE_STRATEGY } from 'component/providers/AccessProvider/permissions';
+import { useId } from 'hooks/useId';
 
 interface IStrategyDeleteButtonProps {
     strategy: IStrategy;
@@ -15,6 +16,8 @@ export const StrategyDeleteButton: VFC<IStrategyDeleteButtonProps> = ({
     strategy,
     onClick,
 }) => {
+    const id = useId();
+
     return (
         <ConditionallyRender
             condition={strategy?.editable}
@@ -29,9 +32,9 @@ export const StrategyDeleteButton: VFC<IStrategyDeleteButtonProps> = ({
             }
             elseShow={
                 <Tooltip title="You cannot delete a built-in strategy" arrow>
-                    <div>
+                    <div id={id}>
                         <IconButton disabled size="large">
-                            <Delete titleAccess="Delete strategy" />
+                            <Delete aria-labelledby={id} />
                         </IconButton>
                     </div>
                 </Tooltip>
diff --git a/frontend/src/component/strategies/StrategiesList/StrategyEditButton/StrategyEditButton.tsx b/frontend/src/component/strategies/StrategiesList/StrategyEditButton/StrategyEditButton.tsx
index a6cad7129c..80dda57bd8 100644
--- a/frontend/src/component/strategies/StrategiesList/StrategyEditButton/StrategyEditButton.tsx
+++ b/frontend/src/component/strategies/StrategiesList/StrategyEditButton/StrategyEditButton.tsx
@@ -5,6 +5,7 @@ import { Edit } from '@mui/icons-material';
 import { IconButton, Tooltip } from '@mui/material';
 import { UPDATE_STRATEGY } from 'component/providers/AccessProvider/permissions';
 import { IStrategy } from 'interfaces/strategy';
+import { useId } from 'hooks/useId';
 
 interface IStrategyEditButtonProps {
     strategy: IStrategy;
@@ -14,26 +15,30 @@ interface IStrategyEditButtonProps {
 export const StrategyEditButton: VFC<IStrategyEditButtonProps> = ({
     strategy,
     onClick,
-}) => (
-    <ConditionallyRender
-        condition={strategy?.editable}
-        show={
-            <PermissionIconButton
-                onClick={onClick}
-                permission={UPDATE_STRATEGY}
-                tooltipProps={{ title: 'Edit strategy' }}
-            >
-                <Edit />
-            </PermissionIconButton>
-        }
-        elseShow={
-            <Tooltip title="You cannot edit a built-in strategy" arrow>
-                <div>
-                    <IconButton disabled size="large">
-                        <Edit titleAccess="Edit strategy" />
-                    </IconButton>
-                </div>
-            </Tooltip>
-        }
-    />
-);
+}) => {
+    const id = useId();
+
+    return (
+        <ConditionallyRender
+            condition={strategy?.editable}
+            show={
+                <PermissionIconButton
+                    onClick={onClick}
+                    permission={UPDATE_STRATEGY}
+                    tooltipProps={{ title: 'Edit strategy' }}
+                >
+                    <Edit />
+                </PermissionIconButton>
+            }
+            elseShow={
+                <Tooltip title="You cannot edit a built-in strategy" arrow>
+                    <div id={id}>
+                        <IconButton disabled size="large">
+                            <Edit aria-labelledby={id} />
+                        </IconButton>
+                    </div>
+                </Tooltip>
+            }
+        />
+    );
+};
diff --git a/frontend/src/component/strategies/StrategiesList/StrategySwitch/StrategySwitch.tsx b/frontend/src/component/strategies/StrategiesList/StrategySwitch/StrategySwitch.tsx
index 21ef30bba9..58c9c394f4 100644
--- a/frontend/src/component/strategies/StrategiesList/StrategySwitch/StrategySwitch.tsx
+++ b/frontend/src/component/strategies/StrategiesList/StrategySwitch/StrategySwitch.tsx
@@ -30,12 +30,13 @@ export const StrategySwitch: VFC<IStrategySwitchProps> = ({
             describeChild
             arrow
         >
-            <div id={id} role="tooltip">
+            <div id={id}>
                 <PermissionSwitch
                     checked={!deprecated}
                     permission={UPDATE_STRATEGY}
                     onClick={onClick}
                     disabled={disabled}
+                    inputProps={{ 'aria-labelledby': id }}
                 />
             </div>
         </Tooltip>
diff --git a/frontend/src/component/tags/TagTypeList/__tests__/__snapshots__/TagTypeList.test.tsx.snap b/frontend/src/component/tags/TagTypeList/__tests__/__snapshots__/TagTypeList.test.tsx.snap
index 110b246b70..81db1439f5 100644
--- a/frontend/src/component/tags/TagTypeList/__tests__/__snapshots__/TagTypeList.test.tsx.snap
+++ b/frontend/src/component/tags/TagTypeList/__tests__/__snapshots__/TagTypeList.test.tsx.snap
@@ -76,7 +76,7 @@ exports[`renders an empty list correctly 1`] = `
                 id="useId-0"
               >
                 <button
-                  aria-describedby="useId-0"
+                  aria-labelledby="useId-0"
                   className="MuiButton-root MuiButton-contained MuiButton-containedPrimary MuiButton-sizeMedium MuiButton-containedSizeMedium MuiButtonBase-root  mui-1aw3qf3-MuiButtonBase-root-MuiButton-root"
                   disabled={false}
                   onBlur={[Function]}
@@ -313,7 +313,6 @@ exports[`renders an empty list correctly 1`] = `
                     onMouseOver={[Function]}
                     onTouchEnd={[Function]}
                     onTouchStart={[Function]}
-                    role="tooltip"
                   >
                     <button
                       aria-labelledby="useId-1"
@@ -364,7 +363,6 @@ exports[`renders an empty list correctly 1`] = `
                     onMouseOver={[Function]}
                     onTouchEnd={[Function]}
                     onTouchStart={[Function]}
-                    role="tooltip"
                   >
                     <button
                       aria-labelledby="useId-2"
@@ -485,7 +483,6 @@ exports[`renders an empty list correctly 1`] = `
                     onMouseOver={[Function]}
                     onTouchEnd={[Function]}
                     onTouchStart={[Function]}
-                    role="tooltip"
                   >
                     <button
                       aria-labelledby="useId-3"
@@ -536,7 +533,6 @@ exports[`renders an empty list correctly 1`] = `
                     onMouseOver={[Function]}
                     onTouchEnd={[Function]}
                     onTouchStart={[Function]}
-                    role="tooltip"
                   >
                     <button
                       aria-labelledby="useId-4"
@@ -657,7 +653,6 @@ exports[`renders an empty list correctly 1`] = `
                     onMouseOver={[Function]}
                     onTouchEnd={[Function]}
                     onTouchStart={[Function]}
-                    role="tooltip"
                   >
                     <button
                       aria-labelledby="useId-5"
@@ -708,7 +703,6 @@ exports[`renders an empty list correctly 1`] = `
                     onMouseOver={[Function]}
                     onTouchEnd={[Function]}
                     onTouchStart={[Function]}
-                    role="tooltip"
                   >
                     <button
                       aria-labelledby="useId-6"
@@ -829,7 +823,6 @@ exports[`renders an empty list correctly 1`] = `
                     onMouseOver={[Function]}
                     onTouchEnd={[Function]}
                     onTouchStart={[Function]}
-                    role="tooltip"
                   >
                     <button
                       aria-labelledby="useId-7"
@@ -880,7 +873,6 @@ exports[`renders an empty list correctly 1`] = `
                     onMouseOver={[Function]}
                     onTouchEnd={[Function]}
                     onTouchStart={[Function]}
-                    role="tooltip"
                   >
                     <button
                       aria-labelledby="useId-8"
@@ -1001,7 +993,6 @@ exports[`renders an empty list correctly 1`] = `
                     onMouseOver={[Function]}
                     onTouchEnd={[Function]}
                     onTouchStart={[Function]}
-                    role="tooltip"
                   >
                     <button
                       aria-labelledby="useId-9"
@@ -1052,7 +1043,6 @@ exports[`renders an empty list correctly 1`] = `
                     onMouseOver={[Function]}
                     onTouchEnd={[Function]}
                     onTouchStart={[Function]}
-                    role="tooltip"
                   >
                     <button
                       aria-labelledby="useId-10"