From 09fedc6cca690c3a197c8de478a7bfa72af943a4 Mon Sep 17 00:00:00 2001
From: Vicktor <79470910+Victor-Nyagudi@users.noreply.github.com>
Date: Thu, 23 Jan 2025 20:27:13 +0300
Subject: [PATCH] [MM-55270] fix(accessibility): "channels" button in global
 header is not selectable with keyboard (#29224)

---
 .../__snapshots__/product_menu.test.tsx.snap  | 96 +++++++++++++++----
 .../product_branding.test.tsx.snap            | 18 +++-
 .../product_branding/product_branding.tsx     | 26 ++++-
 .../product_branding_team_edition.tsx         |  8 +-
 .../product_menu/product_menu.test.tsx        |  2 +-
 .../product_menu/product_menu.tsx             | 54 +++++++----
 6 files changed, 154 insertions(+), 50 deletions(-)

diff --git a/webapp/channels/src/components/global_header/left_controls/product_menu/__snapshots__/product_menu.test.tsx.snap b/webapp/channels/src/components/global_header/left_controls/product_menu/__snapshots__/product_menu.test.tsx.snap
index dd81899c3c..7ef6eceb38 100644
--- a/webapp/channels/src/components/global_header/left_controls/product_menu/__snapshots__/product_menu.test.tsx.snap
+++ b/webapp/channels/src/components/global_header/left_controls/product_menu/__snapshots__/product_menu.test.tsx.snap
@@ -11,12 +11,22 @@ exports[`components/global/product_switcher should have an active button state w
       onClick={[Function]}
     >
       <ProductMenuButton
-        active={true}
         aria-controls="product-switcher-menu"
         aria-expanded={true}
         aria-label="Product switch menu"
-      />
-      <ProductBranding />
+        style={
+          Object {
+            "backgroundColor": "rgba(var(--sidebar-text-rgb), 0.16)",
+            "color": "rgba(var(--sidebar-text-rgb), 0.56)",
+          }
+        }
+      >
+        <ProductsIcon
+          color="rgba(var(--sidebar-text-rgb), 0.56)"
+          size={20}
+        />
+        <ProductBranding />
+      </ProductMenuButton>
     </ProductMenuContainer>
     <Menu
       ariaLabel="switcherOpen"
@@ -75,12 +85,22 @@ exports[`components/global/product_switcher should match snapshot 1`] = `
       onClick={[Function]}
     >
       <ProductMenuButton
-        active={true}
         aria-controls="product-switcher-menu"
         aria-expanded={true}
         aria-label="Product switch menu"
-      />
-      <ProductBranding />
+        style={
+          Object {
+            "backgroundColor": "rgba(var(--sidebar-text-rgb), 0.16)",
+            "color": "rgba(var(--sidebar-text-rgb), 0.56)",
+          }
+        }
+      >
+        <ProductsIcon
+          color="rgba(var(--sidebar-text-rgb), 0.56)"
+          size={20}
+        />
+        <ProductBranding />
+      </ProductMenuButton>
     </ProductMenuContainer>
     <Menu
       ariaLabel="switcherOpen"
@@ -139,12 +159,22 @@ exports[`components/global/product_switcher should match snapshot with product s
       onClick={[Function]}
     >
       <ProductMenuButton
-        active={true}
         aria-controls="product-switcher-menu"
         aria-expanded={true}
         aria-label="Product switch menu"
-      />
-      <ProductBranding />
+        style={
+          Object {
+            "backgroundColor": "rgba(var(--sidebar-text-rgb), 0.16)",
+            "color": "rgba(var(--sidebar-text-rgb), 0.56)",
+          }
+        }
+      >
+        <ProductsIcon
+          color="rgba(var(--sidebar-text-rgb), 0.56)"
+          size={20}
+        />
+        <ProductBranding />
+      </ProductMenuButton>
     </ProductMenuContainer>
     <Menu
       ariaLabel="switcherOpen"
@@ -203,12 +233,22 @@ exports[`components/global/product_switcher should match snapshot without licens
       onClick={[Function]}
     >
       <ProductMenuButton
-        active={true}
         aria-controls="product-switcher-menu"
         aria-expanded={true}
         aria-label="Product switch menu"
-      />
-      <ProductBrandingTeamEdition />
+        style={
+          Object {
+            "backgroundColor": "rgba(var(--sidebar-text-rgb), 0.16)",
+            "color": "rgba(var(--sidebar-text-rgb), 0.56)",
+          }
+        }
+      >
+        <ProductsIcon
+          color="rgba(var(--sidebar-text-rgb), 0.56)"
+          size={20}
+        />
+        <ProductBrandingTeamEdition />
+      </ProductMenuButton>
     </ProductMenuContainer>
     <Menu
       ariaLabel="switcherOpen"
@@ -267,12 +307,22 @@ exports[`components/global/product_switcher should render once when there are no
       onClick={[Function]}
     >
       <ProductMenuButton
-        active={true}
         aria-controls="product-switcher-menu"
         aria-expanded={true}
         aria-label="Product switch menu"
-      />
-      <ProductBranding />
+        style={
+          Object {
+            "backgroundColor": "rgba(var(--sidebar-text-rgb), 0.16)",
+            "color": "rgba(var(--sidebar-text-rgb), 0.56)",
+          }
+        }
+      >
+        <ProductsIcon
+          color="rgba(var(--sidebar-text-rgb), 0.56)"
+          size={20}
+        />
+        <ProductBranding />
+      </ProductMenuButton>
     </ProductMenuContainer>
     <Menu
       ariaLabel="switcherOpen"
@@ -331,12 +381,22 @@ exports[`components/global/product_switcher should render the correct amount of
       onClick={[Function]}
     >
       <ProductMenuButton
-        active={true}
         aria-controls="product-switcher-menu"
         aria-expanded={true}
         aria-label="Product switch menu"
-      />
-      <ProductBranding />
+        style={
+          Object {
+            "backgroundColor": "rgba(var(--sidebar-text-rgb), 0.16)",
+            "color": "rgba(var(--sidebar-text-rgb), 0.56)",
+          }
+        }
+      >
+        <ProductsIcon
+          color="rgba(var(--sidebar-text-rgb), 0.56)"
+          size={20}
+        />
+        <ProductBranding />
+      </ProductMenuButton>
     </ProductMenuContainer>
     <Menu
       ariaLabel="switcherOpen"
diff --git a/webapp/channels/src/components/global_header/left_controls/product_menu/product_branding/__snapshots__/product_branding.test.tsx.snap b/webapp/channels/src/components/global_header/left_controls/product_menu/product_branding/__snapshots__/product_branding.test.tsx.snap
index 8892eb2521..ac58fe9c97 100644
--- a/webapp/channels/src/components/global_header/left_controls/product_menu/product_branding/__snapshots__/product_branding.test.tsx.snap
+++ b/webapp/channels/src/components/global_header/left_controls/product_menu/product_branding/__snapshots__/product_branding.test.tsx.snap
@@ -2,51 +2,63 @@
 
 exports[`components/ProductBranding should show correct icon glyph when we are on Boards 1`] = `
 <ProductBrandingContainer
-  tabIndex={0}
+  tabIndex={-1}
 >
   <ProductBoardsIcon
     size={24}
   />
   <Heading
+    className="sr-only"
     element="h1"
     margin="none"
     size={200}
   >
     Boards
   </Heading>
+  <ProductBrandingHeading>
+    Boards
+  </ProductBrandingHeading>
 </ProductBrandingContainer>
 `;
 
 exports[`components/ProductBranding should show correct icon glyph when we are on Channels 1`] = `
 <ProductBrandingContainer
-  tabIndex={0}
+  tabIndex={-1}
 >
   <ProductChannelsIcon
     size={24}
   />
   <Heading
+    className="sr-only"
     element="h1"
     margin="none"
     size={200}
   >
     Channels
   </Heading>
+  <ProductBrandingHeading>
+    Channels
+  </ProductBrandingHeading>
 </ProductBrandingContainer>
 `;
 
 exports[`components/ProductBranding should show correct icon glyph when we are on Playbooks 1`] = `
 <ProductBrandingContainer
-  tabIndex={0}
+  tabIndex={-1}
 >
   <ProductPlaybooksIcon
     size={24}
   />
   <Heading
+    className="sr-only"
     element="h1"
     margin="none"
     size={200}
   >
     Playbooks
   </Heading>
+  <ProductBrandingHeading>
+    Playbooks
+  </ProductBrandingHeading>
 </ProductBrandingContainer>
 `;
diff --git a/webapp/channels/src/components/global_header/left_controls/product_menu/product_branding/product_branding.tsx b/webapp/channels/src/components/global_header/left_controls/product_menu/product_branding/product_branding.tsx
index d84b5fea2f..93228590af 100644
--- a/webapp/channels/src/components/global_header/left_controls/product_menu/product_branding/product_branding.tsx
+++ b/webapp/channels/src/components/global_header/left_controls/product_menu/product_branding/product_branding.tsx
@@ -9,13 +9,22 @@ import glyphMap, {ProductChannelsIcon} from '@mattermost/compass-icons/component
 
 import {useCurrentProduct} from 'utils/products';
 
-const ProductBrandingContainer = styled.div`
+const ProductBrandingContainer = styled.span`
     display: flex;
     align-items: center;
+`;
 
-    > * + * {
-        margin-left: 8px;
-    }
+// Every style here except for 'margin-left' and 'font-family'is from the deprecated 'Heading' element.
+// https://github.com/mattermost/compass-components/blob/362e96a4eb3489efc8c1852652859ef14a51eb64/src/components/heading/Heading.mixins.ts#L9-L74
+const ProductBrandingHeading = styled.span`
+    font-family: 'Metropolis';
+    font-size: 16px;
+    line-height: 24px;
+    font-weight: bold;
+    margin: 0;
+    color: inherit;
+
+    margin-left: 8px;
 `;
 
 const ProductBranding = (): JSX.Element => {
@@ -24,15 +33,22 @@ const ProductBranding = (): JSX.Element => {
     const Icon = currentProduct?.switcherIcon ? glyphMap[currentProduct.switcherIcon] : ProductChannelsIcon;
 
     return (
-        <ProductBrandingContainer tabIndex={0}>
+        <ProductBrandingContainer tabIndex={-1}>
             <Icon size={24}/>
+
+            {/* Heading for screen readers since an h1 shouldn't be inside a button */}
             <Heading
                 element='h1'
                 size={200}
                 margin='none'
+                className='sr-only'
             >
                 {currentProduct ? currentProduct.switcherText : 'Channels'}
             </Heading>
+
+            <ProductBrandingHeading>
+                {currentProduct ? currentProduct.switcherText : 'Channels'}
+            </ProductBrandingHeading>
         </ProductBrandingContainer>
     );
 };
diff --git a/webapp/channels/src/components/global_header/left_controls/product_menu/product_branding_team_edition/product_branding_team_edition.tsx b/webapp/channels/src/components/global_header/left_controls/product_menu/product_branding_team_edition/product_branding_team_edition.tsx
index 9af4fc7354..2773ba5184 100644
--- a/webapp/channels/src/components/global_header/left_controls/product_menu/product_branding_team_edition/product_branding_team_edition.tsx
+++ b/webapp/channels/src/components/global_header/left_controls/product_menu/product_branding_team_edition/product_branding_team_edition.tsx
@@ -6,7 +6,7 @@ import styled from 'styled-components';
 
 import Logo from 'components/common/svg_images_components/logo_dark_blue_svg';
 
-const ProductBrandingTeamEditionContainer = styled.div`
+const ProductBrandingTeamEditionContainer = styled.span`
     display: flex;
     align-items: center;
 
@@ -21,12 +21,10 @@ const StyledLogo = styled(Logo)`
     }
 `;
 
-const Badge = styled.div`
+const Badge = styled.span`
     display: flex;
     align-self: center;
     padding: 2px 6px;
-    position: relative;
-    top: 1px;
     border-radius: var(--radius-s);
     margin-left: 12px;
     background: rgba(var(--sidebar-text-rgb), 0.08);
@@ -40,7 +38,7 @@ const Badge = styled.div`
 
 const ProductBrandingTeamEdition = (): JSX.Element => {
     return (
-        <ProductBrandingTeamEditionContainer tabIndex={0}>
+        <ProductBrandingTeamEditionContainer tabIndex={-1}>
             <StyledLogo
                 width={116}
                 height={20}
diff --git a/webapp/channels/src/components/global_header/left_controls/product_menu/product_menu.test.tsx b/webapp/channels/src/components/global_header/left_controls/product_menu/product_menu.test.tsx
index 621e44f4e1..678f7bee47 100644
--- a/webapp/channels/src/components/global_header/left_controls/product_menu/product_menu.test.tsx
+++ b/webapp/channels/src/components/global_header/left_controls/product_menu/product_menu.test.tsx
@@ -159,7 +159,7 @@ describe('components/global/product_switcher', () => {
         useStateSpy.mockImplementation(() => [false, setState]);
 
         wrapper.find(ProductMenuContainer).simulate('click');
-        expect(wrapper.find(ProductMenuButton).props().active).toEqual(true);
+        expect(wrapper.find(ProductMenuButton).props()['aria-expanded']).toEqual(true);
         expect(wrapper).toMatchSnapshot();
     });
 
diff --git a/webapp/channels/src/components/global_header/left_controls/product_menu/product_menu.tsx b/webapp/channels/src/components/global_header/left_controls/product_menu/product_menu.tsx
index c08be00c3f..98e5832d51 100644
--- a/webapp/channels/src/components/global_header/left_controls/product_menu/product_menu.tsx
+++ b/webapp/channels/src/components/global_header/left_controls/product_menu/product_menu.tsx
@@ -6,7 +6,9 @@ import {useIntl} from 'react-intl';
 import {useDispatch, useSelector} from 'react-redux';
 import styled from 'styled-components';
 
-import IconButton from '@mattermost/compass-components/components/icon-button'; // eslint-disable-line no-restricted-imports
+import {
+    ProductsIcon,
+} from '@mattermost/compass-icons/components';
 
 import {getLicense} from 'mattermost-redux/selectors/entities/general';
 
@@ -41,21 +43,29 @@ export const ProductMenuContainer = styled.nav`
     }
 `;
 
-export const ProductMenuButton = styled(IconButton).attrs(() => ({
+export const ProductMenuButton = styled.button.attrs(() => ({
     id: 'product_switch_menu',
-    icon: 'products',
-    size: 'sm',
-
-    // we currently need this, since not passing a onClick handler is disabling the IconButton
-    // this is a known issue and is being tracked by UI platform team
-    // TODO@UI: remove the onClick, when it is not a mandatory prop anymore
-    onClick: () => {},
-    inverted: true,
-    compact: true,
+    type: 'button',
 }))`
-    > i::before {
-        font-size: 20px;
-        letter-spacing: 20px;
+    display: flex;
+    align-items: center;
+    background: transparent;
+    border: none;
+    border-radius: 4px;
+    padding: 3px 6px 3px 5px;
+
+    &:hover, &:focus {
+        color: rgba(var(--sidebar-text-rgb), 0.56);
+        background-color: rgba(var(--sidebar-text-rgb), 0.08);
+    }
+
+    &:active {
+        color: rgba(var(--sidebar-text-rgb), 0.56);
+        background-color: rgba(var(--sidebar-text-rgb), 0.16);
+    }
+
+    > * + * {
+        margin-left: 8px;
     }
 `;
 
@@ -110,13 +120,21 @@ const ProductMenu = (): JSX.Element => {
             >
                 <ProductMenuContainer onClick={handleClick}>
                     <ProductMenuButton
-                        active={switcherOpen}
                         aria-expanded={switcherOpen}
                         aria-label={formatMessage({id: 'global_header.productSwitchMenu', defaultMessage: 'Product switch menu'})}
                         aria-controls='product-switcher-menu'
-                    />
-                    {license.IsLicensed === 'false' && <ProductBrandingTeamEdition/>}
-                    {license.IsLicensed === 'true' && <ProductBranding/>}
+                        style={switcherOpen ? {
+                            backgroundColor: 'rgba(var(--sidebar-text-rgb), 0.16)',
+                            color: 'rgba(var(--sidebar-text-rgb), 0.56)',
+                        } : {}}
+                    >
+                        <ProductsIcon
+                            size={20}
+                            color='rgba(var(--sidebar-text-rgb), 0.56)'
+                        />
+                        {license.IsLicensed === 'false' && <ProductBrandingTeamEdition/>}
+                        {license.IsLicensed === 'true' && <ProductBranding/>}
+                    </ProductMenuButton>
                 </ProductMenuContainer>
                 <Menu
                     listId={'product-switcher-menu-dropdown'}