From 2a62c40526510f23edd2ac032effd7e93081078d Mon Sep 17 00:00:00 2001
From: Geido <60598000+geido@users.noreply.github.com>
Date: Mon, 15 Apr 2024 17:54:53 +0200
Subject: [PATCH] chore(Dashboard): Accessibility filters Popover (#28015)

---
 .../DetailsPanel/DetailsPanel.test.tsx        | 92 ++++++++++++++++++-
 .../FiltersBadge/DetailsPanel/index.tsx       | 86 ++++++++++++++---
 .../FiltersBadge/FilterIndicator/index.tsx    | 69 +++++++-------
 .../components/FiltersBadge/Styles.tsx        |  3 +-
 .../components/FiltersBadge/index.tsx         | 41 ++++++++-
 .../components/SliceHeaderControls/index.tsx  |  1 +
 6 files changed, 239 insertions(+), 53 deletions(-)

diff --git a/superset-frontend/src/dashboard/components/FiltersBadge/DetailsPanel/DetailsPanel.test.tsx b/superset-frontend/src/dashboard/components/FiltersBadge/DetailsPanel/DetailsPanel.test.tsx
index ef4b49d21..82b8f3cb3 100644
--- a/superset-frontend/src/dashboard/components/FiltersBadge/DetailsPanel/DetailsPanel.test.tsx
+++ b/superset-frontend/src/dashboard/components/FiltersBadge/DetailsPanel/DetailsPanel.test.tsx
@@ -17,12 +17,28 @@
  * under the License.
  */
 import userEvent from '@testing-library/user-event';
-import React from 'react';
-import { render, screen } from 'spec/helpers/testing-library';
+import React, { RefObject } from 'react';
+import { render, screen, fireEvent } from 'spec/helpers/testing-library';
 import { Indicator } from 'src/dashboard/components/nativeFilters/selectors';
 import DetailsPanel from '.';
 
+const mockPopoverContentRef = {
+  current: {
+    focus: jest.fn(),
+  },
+} as unknown as RefObject<HTMLDivElement>;
+
+const mockPopoverTriggerRef = {
+  current: {
+    focus: jest.fn(),
+  },
+} as unknown as RefObject<HTMLDivElement>;
+
 const createProps = () => ({
+  popoverVisible: true,
+  popoverContentRef: mockPopoverContentRef,
+  popoverTriggerRef: mockPopoverTriggerRef,
+  setPopoverVisible: jest.fn(),
   appliedCrossFilterIndicators: [
     {
       column: 'clinical_stage',
@@ -168,3 +184,75 @@ test('Should render empty', () => {
   userEvent.click(screen.getByTestId('details-panel-content'));
   expect(screen.queryByRole('button')).not.toBeInTheDocument();
 });
+
+test('Close popover with ESC or ENTER', async () => {
+  const props = createProps();
+  render(
+    <DetailsPanel {...props}>
+      <div>Content</div>
+    </DetailsPanel>,
+    { useRedux: true },
+  );
+
+  const activeElement = document.activeElement as Element;
+
+  // Close popover with Escape key
+  fireEvent.keyDown(activeElement, { key: 'Escape', code: 'Escape' });
+  expect(props.setPopoverVisible).toHaveBeenCalledWith(false);
+
+  // Open the popover for this test
+  props.popoverVisible = true;
+
+  // Close with Enter
+  fireEvent.keyDown(activeElement, { key: 'Enter', code: 'Enter' });
+  expect(props.setPopoverVisible).toHaveBeenCalledWith(false);
+});
+
+test('Arrow key navigation switches focus between indicators', () => {
+  // Prepare props with two indicators
+  const props = createProps();
+
+  props.appliedCrossFilterIndicators = [
+    {
+      column: 'clinical_stage',
+      name: 'Clinical Stage',
+      value: [],
+      path: ['PATH_TO_CLINICAL_STAGE'],
+    },
+    {
+      column: 'age_group',
+      name: 'Age Group',
+      value: [],
+      path: ['PATH_TO_AGE_GROUP'],
+    },
+  ];
+
+  render(
+    <DetailsPanel {...props}>
+      <div>Content</div>
+    </DetailsPanel>,
+    { useRedux: true },
+  );
+
+  // Query the indicators
+  const firstIndicator = screen.getByRole('button', { name: 'Clinical Stage' });
+  const secondIndicator = screen.getByRole('button', { name: 'Age Group' });
+
+  // Focus the first indicator
+  firstIndicator.focus();
+  expect(firstIndicator).toHaveFocus();
+
+  // Simulate ArrowDown key press
+  fireEvent.keyDown(document.activeElement as Element, {
+    key: 'ArrowDown',
+    code: 'ArrowDown',
+  });
+  expect(secondIndicator).toHaveFocus();
+
+  // Simulate ArrowUp key press
+  fireEvent.keyDown(document.activeElement as Element, {
+    key: 'ArrowUp',
+    code: 'ArrowUp',
+  });
+  expect(firstIndicator).toHaveFocus();
+});
diff --git a/superset-frontend/src/dashboard/components/FiltersBadge/DetailsPanel/index.tsx b/superset-frontend/src/dashboard/components/FiltersBadge/DetailsPanel/index.tsx
index 53d20cd1f..482dc47e9 100644
--- a/superset-frontend/src/dashboard/components/FiltersBadge/DetailsPanel/index.tsx
+++ b/superset-frontend/src/dashboard/components/FiltersBadge/DetailsPanel/index.tsx
@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import React, { useEffect, useState } from 'react';
+import React, { RefObject, useEffect, useRef } from 'react';
 import { useSelector } from 'react-redux';
 import { Global, css } from '@emotion/react';
 import { t } from '@superset-ui/core';
@@ -36,6 +36,10 @@ export interface DetailsPanelProps {
   appliedIndicators: Indicator[];
   onHighlightFilterSource: (path: string[]) => void;
   children: JSX.Element;
+  popoverVisible: boolean;
+  popoverContentRef: RefObject<HTMLDivElement>;
+  popoverTriggerRef: RefObject<HTMLDivElement>;
+  setPopoverVisible: (visible: boolean) => void;
 }
 
 const DetailsPanelPopover = ({
@@ -43,35 +47,87 @@ const DetailsPanelPopover = ({
   appliedIndicators = [],
   onHighlightFilterSource,
   children,
+  popoverVisible,
+  popoverContentRef,
+  popoverTriggerRef,
+  setPopoverVisible,
 }: DetailsPanelProps) => {
-  const [visible, setVisible] = useState(false);
   const activeTabs = useSelector<RootState>(
     state => state.dashboardState?.activeTabs,
   );
+  // Combined ref array for all filter indicator elements
+  const indicatorRefs = useRef<(HTMLButtonElement | null)[]>([]);
+
+  const handleKeyDown = (event: React.KeyboardEvent<HTMLDivElement>) => {
+    switch (event.key) {
+      case 'Escape':
+      case 'Enter':
+        // timing out to allow for filter selection to happen first
+        setTimeout(() => {
+          // move back to the popover trigger element
+          popoverTriggerRef?.current?.focus();
+          // Close popover on ESC or ENTER
+          setPopoverVisible(false);
+        });
+        break;
+      case 'ArrowDown':
+      case 'ArrowUp': {
+        event.preventDefault(); // Prevent scrolling
+        // Navigate through filters with arrows up/down
+        const currentFocusIndex = indicatorRefs.current.findIndex(
+          ref => ref === document.activeElement,
+        );
+        const maxIndex = indicatorRefs.current.length - 1;
+        let nextFocusIndex = 0;
+
+        if (event.key === 'ArrowDown') {
+          nextFocusIndex =
+            currentFocusIndex >= maxIndex ? 0 : currentFocusIndex + 1;
+        } else if (event.key === 'ArrowUp') {
+          nextFocusIndex =
+            currentFocusIndex <= 0 ? maxIndex : currentFocusIndex - 1;
+        }
+        indicatorRefs.current[nextFocusIndex]?.focus();
+        break;
+      }
+      case 'Tab':
+        // forcing popover context until ESC or ENTER are pressed
+        event.preventDefault();
+        break;
+      default:
+        break;
+    }
+  };
+
+  const handleVisibility = (isOpen: boolean) => {
+    setPopoverVisible(isOpen);
+  };
 
   // we don't need to clean up useEffect, setting { once: true } removes the event listener after handle function is called
   useEffect(() => {
-    if (visible) {
-      window.addEventListener('resize', () => setVisible(false), {
+    if (popoverVisible) {
+      window.addEventListener('resize', () => setPopoverVisible(false), {
         once: true,
       });
     }
-  }, [visible]);
+  }, [popoverVisible]);
 
   // if tabs change, popover doesn't close automatically
   useEffect(() => {
-    setVisible(false);
+    setPopoverVisible(false);
   }, [activeTabs]);
 
-  function handlePopoverStatus(isOpen: boolean) {
-    setVisible(isOpen);
-  }
-
   const indicatorKey = (indicator: Indicator): string =>
     `${indicator.column} - ${indicator.name}`;
 
   const content = (
-    <FiltersDetailsContainer>
+    <FiltersDetailsContainer
+      ref={popoverContentRef}
+      tabIndex={-1}
+      onMouseLeave={() => setPopoverVisible(false)}
+      onKeyDown={handleKeyDown}
+      role="menu"
+    >
       <Global
         styles={theme => css`
           .filterStatusPopover {
@@ -132,6 +188,7 @@ const DetailsPanelPopover = ({
             <FiltersContainer>
               {appliedCrossFilterIndicators.map(indicator => (
                 <FilterIndicator
+                  ref={el => indicatorRefs.current.push(el)}
                   key={indicatorKey(indicator)}
                   indicator={indicator}
                   onClick={onHighlightFilterSource}
@@ -151,6 +208,7 @@ const DetailsPanelPopover = ({
             <FiltersContainer>
               {appliedIndicators.map(indicator => (
                 <FilterIndicator
+                  ref={el => indicatorRefs.current.push(el)}
                   key={indicatorKey(indicator)}
                   indicator={indicator}
                   onClick={onHighlightFilterSource}
@@ -167,10 +225,10 @@ const DetailsPanelPopover = ({
     <Popover
       overlayClassName="filterStatusPopover"
       content={content}
-      visible={visible}
-      onVisibleChange={handlePopoverStatus}
+      visible={popoverVisible}
+      onVisibleChange={handleVisibility}
       placement="bottomRight"
-      trigger="hover"
+      trigger={['hover']}
     >
       {children}
     </Popover>
diff --git a/superset-frontend/src/dashboard/components/FiltersBadge/FilterIndicator/index.tsx b/superset-frontend/src/dashboard/components/FiltersBadge/FilterIndicator/index.tsx
index 2f8fd5af1..2be5cb743 100644
--- a/superset-frontend/src/dashboard/components/FiltersBadge/FilterIndicator/index.tsx
+++ b/superset-frontend/src/dashboard/components/FiltersBadge/FilterIndicator/index.tsx
@@ -17,7 +17,7 @@
  * under the License.
  */
 
-import React, { FC } from 'react';
+import React from 'react';
 import { css } from '@superset-ui/core';
 import Icons from 'src/components/Icons';
 import { getFilterValueForDisplay } from 'src/dashboard/components/nativeFilters/utils';
@@ -33,38 +33,39 @@ export interface IndicatorProps {
   onClick?: (path: string[]) => void;
 }
 
-const FilterIndicator: FC<IndicatorProps> = ({
-  indicator: { column, name, value, path = [] },
-  onClick,
-}) => {
-  const resultValue = getFilterValueForDisplay(value);
-  return (
-    <FilterItem
-      onClick={
-        onClick ? () => onClick([...path, `LABEL-${column}`]) : undefined
-      }
-    >
-      {onClick && (
-        <i>
-          <Icons.SearchOutlined
-            iconSize="m"
-            css={css`
-              span {
-                vertical-align: 0;
-              }
-            `}
-          />
-        </i>
-      )}
-      <div>
-        <FilterName>
-          {name}
-          {resultValue ? ': ' : ''}
-        </FilterName>
-        <FilterValue>{resultValue}</FilterValue>
-      </div>
-    </FilterItem>
-  );
-};
+const FilterIndicator = React.forwardRef<HTMLButtonElement, IndicatorProps>(
+  ({ indicator: { column, name, value, path = [] }, onClick }, ref) => {
+    const resultValue = getFilterValueForDisplay(value);
+    return (
+      <FilterItem
+        ref={ref}
+        onClick={
+          onClick ? () => onClick([...path, `LABEL-${column}`]) : undefined
+        }
+        tabIndex={-1}
+      >
+        {onClick && (
+          <i>
+            <Icons.SearchOutlined
+              iconSize="m"
+              css={css`
+                span {
+                  vertical-align: 0;
+                }
+              `}
+            />
+          </i>
+        )}
+        <div>
+          <FilterName>
+            {name}
+            {resultValue ? ': ' : ''}
+          </FilterName>
+          <FilterValue>{resultValue}</FilterValue>
+        </div>
+      </FilterItem>
+    );
+  },
+);
 
 export default FilterIndicator;
diff --git a/superset-frontend/src/dashboard/components/FiltersBadge/Styles.tsx b/superset-frontend/src/dashboard/components/FiltersBadge/Styles.tsx
index 1b3be657a..9abd2c9ea 100644
--- a/superset-frontend/src/dashboard/components/FiltersBadge/Styles.tsx
+++ b/superset-frontend/src/dashboard/components/FiltersBadge/Styles.tsx
@@ -93,7 +93,8 @@ export const FilterItem = styled.button`
       transition: opacity ease-in-out ${theme.transitionTiming};
     }
 
-    &:hover i svg {
+    &:hover i svg,
+    &:focus-visible i svg {
       opacity: 1;
     }
   `}
diff --git a/superset-frontend/src/dashboard/components/FiltersBadge/index.tsx b/superset-frontend/src/dashboard/components/FiltersBadge/index.tsx
index 6dba29c66..3f771740a 100644
--- a/superset-frontend/src/dashboard/components/FiltersBadge/index.tsx
+++ b/superset-frontend/src/dashboard/components/FiltersBadge/index.tsx
@@ -16,7 +16,13 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import React, { useCallback, useEffect, useMemo, useState } from 'react';
+import React, {
+  useCallback,
+  useEffect,
+  useMemo,
+  useRef,
+  useState,
+} from 'react';
 import { useDispatch, useSelector } from 'react-redux';
 import { uniqWith } from 'lodash';
 import cx from 'classnames';
@@ -25,6 +31,7 @@ import {
   Filters,
   JsonObject,
   styled,
+  t,
   usePrevious,
 } from '@superset-ui/core';
 import Icons from 'src/components/Icons';
@@ -126,6 +133,9 @@ export const FiltersBadge = ({ chartId }: FiltersBadgeProps) => {
   const [dashboardIndicators, setDashboardIndicators] = useState<Indicator[]>(
     indicatorsInitialState,
   );
+  const [popoverVisible, setPopoverVisible] = useState(false);
+  const popoverContentRef = useRef<HTMLDivElement>(null);
+  const popoverTriggerRef = useRef<HTMLDivElement>(null);
 
   const onHighlightFilterSource = useCallback(
     (path: string[]) => {
@@ -134,6 +144,12 @@ export const FiltersBadge = ({ chartId }: FiltersBadgeProps) => {
     [dispatch],
   );
 
+  const handleKeyDown = (event: React.KeyboardEvent<HTMLDivElement>) => {
+    if (event.key === 'Enter') {
+      setPopoverVisible(true);
+    }
+  };
+
   const prevChart = usePrevious(chart);
   const prevChartStatus = prevChart?.chartStatus;
   const prevDashboardFilters = usePrevious(dashboardFilters);
@@ -141,6 +157,14 @@ export const FiltersBadge = ({ chartId }: FiltersBadgeProps) => {
   const showIndicators =
     chart?.chartStatus && ['rendered', 'success'].includes(chart.chartStatus);
 
+  useEffect(() => {
+    if (popoverVisible) {
+      setTimeout(() => {
+        popoverContentRef?.current?.focus();
+      });
+    }
+  }, [popoverVisible]);
+
   useEffect(() => {
     if (!showIndicators && dashboardIndicators.length > 0) {
       setDashboardIndicators(indicatorsInitialState);
@@ -180,6 +204,7 @@ export const FiltersBadge = ({ chartId }: FiltersBadgeProps) => {
   const prevDashboardLayout = usePrevious(present);
   const prevDataMask = usePrevious(dataMask);
   const prevChartConfig = usePrevious(chartConfiguration);
+
   useEffect(() => {
     if (!showIndicators && nativeIndicators.length > 0) {
       setNativeIndicators(indicatorsInitialState);
@@ -250,6 +275,8 @@ export const FiltersBadge = ({ chartId }: FiltersBadgeProps) => {
       ),
     [indicators],
   );
+  const filterCount =
+    appliedIndicators.length + appliedCrossFilterIndicators.length;
 
   if (!appliedCrossFilterIndicators.length && !appliedIndicators.length) {
     return null;
@@ -260,18 +287,28 @@ export const FiltersBadge = ({ chartId }: FiltersBadgeProps) => {
       appliedCrossFilterIndicators={appliedCrossFilterIndicators}
       appliedIndicators={appliedIndicators}
       onHighlightFilterSource={onHighlightFilterSource}
+      setPopoverVisible={setPopoverVisible}
+      popoverVisible={popoverVisible}
+      popoverContentRef={popoverContentRef}
+      popoverTriggerRef={popoverTriggerRef}
     >
       <StyledFilterCount
+        aria-label={t('Applied filters (%s)', filterCount)}
+        aria-haspopup="true"
+        role="button"
+        ref={popoverTriggerRef}
         className={cx(
           'filter-counts',
           !!appliedCrossFilterIndicators.length && 'has-cross-filters',
         )}
+        tabIndex={0}
+        onKeyDown={handleKeyDown}
       >
         <Icons.Filter iconSize="m" />
         <StyledBadge
           data-test="applied-filter-count"
           className="applied-count"
-          count={appliedIndicators.length + appliedCrossFilterIndicators.length}
+          count={filterCount}
           showZero
         />
       </StyledFilterCount>
diff --git a/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx b/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx
index 49a13362f..3890c596e 100644
--- a/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx
+++ b/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx
@@ -936,6 +936,7 @@ const SliceHeaderControls = (props: SliceHeaderControlsPropsWithRouter) => {
           id={`slice_${slice.slice_id}-controls`}
           role="button"
           aria-label="More Options"
+          aria-haspopup="true"
           tabIndex={0}
         >
           <VerticalDotsTrigger />