From 7e03d33be05b8de36a1cab8747a5a6733d20e280 Mon Sep 17 00:00:00 2001
From: daproclaima <sebastien@devedanos.com>
Date: Thu, 18 Jul 2024 17:21:45 +0200
Subject: [PATCH] =?UTF-8?q?=F0=9F=9A=B8(frontend)=20improve=20keyboard=20n?=
 =?UTF-8?q?avigation?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- add css rules to highlight focused-visible navigable elements
- update drop down components to make it keyboard navigable
- add e2e keyboard navigation tests asserting it navigates through
all focusable elements from top to bottom on groups index view
when one group exists
---
 .../apps/desk/src/components/DropButton.tsx   | 42 ++++++---
 .../desk/src/cunningham/cunningham-style.css  | 21 ++++-
 .../src/features/language/LanguagePicker.tsx  |  5 ++
 .../components/panel/PanelItem.tsx            |  2 +-
 .../apps/desk/src/features/menu/MenuItems.tsx | 10 ++-
 .../components/CardCreateTeam.tsx             |  4 +-
 .../teams-panel/components/PanelActions.tsx   |  1 +
 .../apps/desk/src/pages/teams/index.tsx       |  2 +-
 .../app-desk/keyboard-navigation.spec.ts      | 86 +++++++++++++++++++
 9 files changed, 157 insertions(+), 16 deletions(-)
 create mode 100644 src/frontend/apps/e2e/__tests__/app-desk/keyboard-navigation.spec.ts

diff --git a/src/frontend/apps/desk/src/components/DropButton.tsx b/src/frontend/apps/desk/src/components/DropButton.tsx
index f79b78e..2b81887 100644
--- a/src/frontend/apps/desk/src/components/DropButton.tsx
+++ b/src/frontend/apps/desk/src/components/DropButton.tsx
@@ -2,10 +2,11 @@ import React, {
   PropsWithChildren,
   ReactNode,
   useEffect,
+  useRef,
   useState,
 } from 'react';
 import { Button, DialogTrigger, Popover } from 'react-aria-components';
-import styled from 'styled-components';
+import styled, { createGlobalStyle } from 'styled-components';
 
 const StyledPopover = styled(Popover)`
   background-color: white;
@@ -29,6 +30,12 @@ const StyledButton = styled(Button)`
   text-wrap: nowrap;
 `;
 
+const GlobalStyle = createGlobalStyle`
+  &:focus-visible {
+    outline: none;
+  }
+`;
+
 interface DropButtonProps {
   button: ReactNode;
   isOpen?: boolean;
@@ -44,10 +51,18 @@ export const DropButton = ({
   const [opacity, setOpacity] = useState(false);
   const [isLocalOpen, setIsLocalOpen] = useState(isOpen);
 
+  const ref = useRef<HTMLDivElement>(null);
+
   useEffect(() => {
     setIsLocalOpen(isOpen);
   }, [isOpen]);
 
+  useEffect(() => {
+    if (ref.current) {
+      ref.current[isLocalOpen ? 'focus' : 'blur']();
+    }
+  }, [isLocalOpen]);
+
   const onOpenChangeHandler = (isOpen: boolean) => {
     setIsLocalOpen(isOpen);
     onOpenChange?.(isOpen);
@@ -57,15 +72,20 @@ export const DropButton = ({
   };
 
   return (
-    <DialogTrigger onOpenChange={onOpenChangeHandler} isOpen={isLocalOpen}>
-      <StyledButton>{button}</StyledButton>
-      <StyledPopover
-        style={{ opacity: opacity ? 1 : 0 }}
-        isOpen={isLocalOpen}
-        onOpenChange={onOpenChangeHandler}
-      >
-        {children}
-      </StyledPopover>
-    </DialogTrigger>
+    <>
+      <GlobalStyle />
+      <DialogTrigger onOpenChange={onOpenChangeHandler} isOpen={isLocalOpen}>
+        <StyledButton>{button}</StyledButton>
+        <StyledPopover
+          style={{ opacity: opacity ? 1 : 0 }}
+          isOpen={isLocalOpen}
+          onOpenChange={onOpenChangeHandler}
+        >
+          <div ref={ref} tabIndex={-1}>
+            {children}
+          </div>
+        </StyledPopover>
+      </DialogTrigger>
+    </>
   );
 };
diff --git a/src/frontend/apps/desk/src/cunningham/cunningham-style.css b/src/frontend/apps/desk/src/cunningham/cunningham-style.css
index 97c1ea3..5b5d147 100644
--- a/src/frontend/apps/desk/src/cunningham/cunningham-style.css
+++ b/src/frontend/apps/desk/src/cunningham/cunningham-style.css
@@ -5,6 +5,15 @@
 @import url('./cunningham-custom-tokens.css');
 @import url('../assets/fonts/Marianne/Marianne-font.css');
 
+a:focus-visible,
+button:focus-visible,
+.c__button:focus-visible,
+.c__select__wrapper:focus-visible,
+.c__datagrid__header:focus-visible {
+  outline: var(--c--theme--colors--primary-600) solid 2px;
+  border-radius: var(--c--components--button--border-radius--focus);
+}
+
 .c__input,
 .c__field,
 .c__select,
@@ -314,6 +323,10 @@ input:-webkit-autofill:focus {
   transition: all 0.8s ease-in-out;
 }
 
+.c__radio input:focus-visible {
+  outline: var(--c--theme--colors--primary-600) solid 2px;
+}
+
 /**
  * Button
 */
@@ -340,7 +353,8 @@ input:-webkit-autofill:focus {
     var(--c--theme--spacings--s);
 }
 
-.c__button--primary {
+.c__button--primary,
+.c__button--primary:focus-visible {
   background-color: var(--c--components--button--primary--background--color);
   color: var(--c--components--button--primary--color);
 }
@@ -470,6 +484,11 @@ input:-webkit-autofill:focus {
 /**
  * Modal
 */
+.c__modal:focus-visible {
+  outline: none;
+  box-shadow: none;
+}
+
 .c__modal__backdrop {
   z-index: 1000;
 }
diff --git a/src/frontend/apps/desk/src/features/language/LanguagePicker.tsx b/src/frontend/apps/desk/src/features/language/LanguagePicker.tsx
index 990cd77..feafcd1 100644
--- a/src/frontend/apps/desk/src/features/language/LanguagePicker.tsx
+++ b/src/frontend/apps/desk/src/features/language/LanguagePicker.tsx
@@ -30,6 +30,11 @@ const SelectStyled = styled(Select)<{ $isSmall?: boolean }>`
     &:hover {
       border-color: var(--c--theme--colors--primary-500);
     }
+
+    .c__button--tertiary-text:focus-visible {
+      outline: var(--c--theme--colors--primary-600) solid 2px;
+      border-radius: var(--c--components--button--border-radius--focus);
+    }
   }
 `;
 
diff --git a/src/frontend/apps/desk/src/features/mail-domains/components/panel/PanelItem.tsx b/src/frontend/apps/desk/src/features/mail-domains/components/panel/PanelItem.tsx
index c9db2e4..442252c 100644
--- a/src/frontend/apps/desk/src/features/mail-domains/components/panel/PanelItem.tsx
+++ b/src/frontend/apps/desk/src/features/mail-domains/components/panel/PanelItem.tsx
@@ -41,7 +41,7 @@ export const PanelMailDomains = ({ mailDomain }: MailDomainProps) => {
 
   return (
     <Box
-      $margin={{ all: 'none' }}
+      $margin="none"
       as="li"
       $css={`
         transition: all 0.2s ease-in; 
diff --git a/src/frontend/apps/desk/src/features/menu/MenuItems.tsx b/src/frontend/apps/desk/src/features/menu/MenuItems.tsx
index 4421c9f..4bf1e70 100644
--- a/src/frontend/apps/desk/src/features/menu/MenuItems.tsx
+++ b/src/frontend/apps/desk/src/features/menu/MenuItems.tsx
@@ -50,6 +50,10 @@ const MenuItem = ({ Icon, label, href, alias }: MenuItemProps) => {
       onMouseOver={() => setIsTooltipOpen(true)}
       onMouseLeave={() => setIsTooltipOpen(false)}
       style={{ display: 'block' }}
+      $css={`
+        &:focus-visible {
+          outline: #fff solid 2px;
+        }`}
     >
       <Box
         $margin="xtiny"
@@ -63,7 +67,11 @@ const MenuItem = ({ Icon, label, href, alias }: MenuItemProps) => {
         $background={background}
         $radius="10px"
       >
-        <BoxButton aria-label={t(`{{label}} button`, { label })} $color={color}>
+        <BoxButton
+          aria-label={t(`{{label}} button`, { label })}
+          $color={color}
+          tabIndex={-1}
+        >
           <Icon
             width="2.375rem"
             aria-label={t(`{{label}} icon`, { label })}
diff --git a/src/frontend/apps/desk/src/features/teams/team-management/components/CardCreateTeam.tsx b/src/frontend/apps/desk/src/features/teams/team-management/components/CardCreateTeam.tsx
index 066fd6e..30b9cc3 100644
--- a/src/frontend/apps/desk/src/features/teams/team-management/components/CardCreateTeam.tsx
+++ b/src/frontend/apps/desk/src/features/teams/team-management/components/CardCreateTeam.tsx
@@ -55,7 +55,9 @@ export const CardCreateTeam = () => {
       </Box>
       <Box $justify="space-between" $direction="row" $align="center">
         <StyledLink href="/">
-          <Button color="secondary">{t('Cancel')}</Button>
+          <Button color="secondary" tabIndex={-1}>
+            {t('Cancel')}
+          </Button>
         </StyledLink>
         <Button
           onClick={() => createTeam(teamName)}
diff --git a/src/frontend/apps/desk/src/features/teams/teams-panel/components/PanelActions.tsx b/src/frontend/apps/desk/src/features/teams/teams-panel/components/PanelActions.tsx
index e69ff0f..277c629 100644
--- a/src/frontend/apps/desk/src/features/teams/teams-panel/components/PanelActions.tsx
+++ b/src/frontend/apps/desk/src/features/teams/teams-panel/components/PanelActions.tsx
@@ -47,6 +47,7 @@ export const PanelActions = () => {
         <BoxButton
           aria-label={t('Add a team')}
           $color={colorsTokens()['primary-600']}
+          tabIndex={-1}
         >
           <IconAdd width={30} height={30} aria-label={t('Add team icon')} />
         </BoxButton>
diff --git a/src/frontend/apps/desk/src/pages/teams/index.tsx b/src/frontend/apps/desk/src/pages/teams/index.tsx
index 05c1eb4..28e9c9a 100644
--- a/src/frontend/apps/desk/src/pages/teams/index.tsx
+++ b/src/frontend/apps/desk/src/pages/teams/index.tsx
@@ -17,7 +17,7 @@ const Page: NextPageWithLayout = () => {
   return (
     <Box $align="center" $justify="center" $height="inherit">
       <StyledLink href="/teams/create">
-        <StyledButton>{t('Create a new team')}</StyledButton>
+        <StyledButton tabIndex={-1}>{t('Create a new team')}</StyledButton>
       </StyledLink>
     </Box>
   );
diff --git a/src/frontend/apps/e2e/__tests__/app-desk/keyboard-navigation.spec.ts b/src/frontend/apps/e2e/__tests__/app-desk/keyboard-navigation.spec.ts
new file mode 100644
index 0000000..fa79b0f
--- /dev/null
+++ b/src/frontend/apps/e2e/__tests__/app-desk/keyboard-navigation.spec.ts
@@ -0,0 +1,86 @@
+import { Page, expect, test } from '@playwright/test';
+
+import { keyCloakSignIn } from './common';
+
+const payloadGetTeams = {
+  count: 4,
+  next: null,
+  previous: null,
+  results: [
+    {
+      id: 'b2224958-ac22-4863-9330-4322add64ebe',
+      name: 'Test Group',
+      accesses: ['1a1ba2ba-a58c-4593-8adb-1da6ee9cd3a3'],
+      abilities: {
+        get: true,
+        patch: true,
+        put: true,
+        delete: true,
+        manage_accesses: true,
+      },
+      slug: 'test-group',
+      created_at: '2024-07-17T19:41:11.404763Z',
+      updated_at: '2024-07-17T19:41:11.404763Z',
+    },
+  ],
+};
+
+const mockApiRequests = (page: Page) => {
+  void page.route('**/teams/?page=1&ordering=-created_at', (route) => {
+    void route.fulfill({
+      json: payloadGetTeams,
+    });
+  });
+};
+
+test.describe('Keyboard navigation', () => {
+  test('navigates through all focusable elements from top to bottom on groups index view when one group exists', async ({
+    browser,
+    browserName,
+  }) => {
+    const page = await browser.newPage();
+
+    await page.goto('/');
+    await keyCloakSignIn(page, browserName);
+
+    void mockApiRequests(page);
+
+    const header = page.locator('header');
+
+    // La Gauffre button is loaded asynchronously, so we wait for it to be visible
+    await expect(
+      header.getByRole('button', {
+        name: 'Les services de La Suite numérique',
+      }),
+    ).toBeVisible();
+
+    // necessary to begin the keyboard navigation directly from first button on the app and only select its elements
+    await header.click();
+
+    // ensure ignoring elements (like tanstack query button) that are not part of the app
+    const focusableElements = await page
+      .locator(
+        '.c__app a:not([tabindex="-1"]), .c__app button:not([tabindex="-1"]), ' +
+          '.c__app [tabindex]:not([tabindex="-1"])',
+      )
+      .all();
+
+    expect(focusableElements.length).toEqual(20);
+
+    for (let i = 0; i < focusableElements.length; i++) {
+      await page.keyboard.press('Tab');
+
+      // check language picker language option navigation. 4th element is inner language picker arrow button
+      // eslint-disable-next-line playwright/no-conditional-in-test
+      if (i === 3) {
+        await page.keyboard.press('Enter');
+
+        // eslint-disable-next-line playwright/no-conditional-expect
+        await expect(focusableElements[i]).toBeFocused();
+        continue;
+      }
+
+      await expect(focusableElements[i]).toBeFocused();
+    }
+  });
+});