chore(Dashboard): Accessibility filters Popover (#28015)

This commit is contained in:
Geido 2024-04-15 17:54:53 +02:00 committed by GitHub
parent 08aaebbf7c
commit 2a62c40526
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 239 additions and 53 deletions

View File

@ -17,12 +17,28 @@
* under the License. * under the License.
*/ */
import userEvent from '@testing-library/user-event'; import userEvent from '@testing-library/user-event';
import React from 'react'; import React, { RefObject } from 'react';
import { render, screen } from 'spec/helpers/testing-library'; import { render, screen, fireEvent } from 'spec/helpers/testing-library';
import { Indicator } from 'src/dashboard/components/nativeFilters/selectors'; import { Indicator } from 'src/dashboard/components/nativeFilters/selectors';
import DetailsPanel from '.'; 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 = () => ({ const createProps = () => ({
popoverVisible: true,
popoverContentRef: mockPopoverContentRef,
popoverTriggerRef: mockPopoverTriggerRef,
setPopoverVisible: jest.fn(),
appliedCrossFilterIndicators: [ appliedCrossFilterIndicators: [
{ {
column: 'clinical_stage', column: 'clinical_stage',
@ -168,3 +184,75 @@ test('Should render empty', () => {
userEvent.click(screen.getByTestId('details-panel-content')); userEvent.click(screen.getByTestId('details-panel-content'));
expect(screen.queryByRole('button')).not.toBeInTheDocument(); 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();
});

View File

@ -16,7 +16,7 @@
* specific language governing permissions and limitations * specific language governing permissions and limitations
* under the License. * under the License.
*/ */
import React, { useEffect, useState } from 'react'; import React, { RefObject, useEffect, useRef } from 'react';
import { useSelector } from 'react-redux'; import { useSelector } from 'react-redux';
import { Global, css } from '@emotion/react'; import { Global, css } from '@emotion/react';
import { t } from '@superset-ui/core'; import { t } from '@superset-ui/core';
@ -36,6 +36,10 @@ export interface DetailsPanelProps {
appliedIndicators: Indicator[]; appliedIndicators: Indicator[];
onHighlightFilterSource: (path: string[]) => void; onHighlightFilterSource: (path: string[]) => void;
children: JSX.Element; children: JSX.Element;
popoverVisible: boolean;
popoverContentRef: RefObject<HTMLDivElement>;
popoverTriggerRef: RefObject<HTMLDivElement>;
setPopoverVisible: (visible: boolean) => void;
} }
const DetailsPanelPopover = ({ const DetailsPanelPopover = ({
@ -43,35 +47,87 @@ const DetailsPanelPopover = ({
appliedIndicators = [], appliedIndicators = [],
onHighlightFilterSource, onHighlightFilterSource,
children, children,
popoverVisible,
popoverContentRef,
popoverTriggerRef,
setPopoverVisible,
}: DetailsPanelProps) => { }: DetailsPanelProps) => {
const [visible, setVisible] = useState(false);
const activeTabs = useSelector<RootState>( const activeTabs = useSelector<RootState>(
state => state.dashboardState?.activeTabs, 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 // we don't need to clean up useEffect, setting { once: true } removes the event listener after handle function is called
useEffect(() => { useEffect(() => {
if (visible) { if (popoverVisible) {
window.addEventListener('resize', () => setVisible(false), { window.addEventListener('resize', () => setPopoverVisible(false), {
once: true, once: true,
}); });
} }
}, [visible]); }, [popoverVisible]);
// if tabs change, popover doesn't close automatically // if tabs change, popover doesn't close automatically
useEffect(() => { useEffect(() => {
setVisible(false); setPopoverVisible(false);
}, [activeTabs]); }, [activeTabs]);
function handlePopoverStatus(isOpen: boolean) {
setVisible(isOpen);
}
const indicatorKey = (indicator: Indicator): string => const indicatorKey = (indicator: Indicator): string =>
`${indicator.column} - ${indicator.name}`; `${indicator.column} - ${indicator.name}`;
const content = ( const content = (
<FiltersDetailsContainer> <FiltersDetailsContainer
ref={popoverContentRef}
tabIndex={-1}
onMouseLeave={() => setPopoverVisible(false)}
onKeyDown={handleKeyDown}
role="menu"
>
<Global <Global
styles={theme => css` styles={theme => css`
.filterStatusPopover { .filterStatusPopover {
@ -132,6 +188,7 @@ const DetailsPanelPopover = ({
<FiltersContainer> <FiltersContainer>
{appliedCrossFilterIndicators.map(indicator => ( {appliedCrossFilterIndicators.map(indicator => (
<FilterIndicator <FilterIndicator
ref={el => indicatorRefs.current.push(el)}
key={indicatorKey(indicator)} key={indicatorKey(indicator)}
indicator={indicator} indicator={indicator}
onClick={onHighlightFilterSource} onClick={onHighlightFilterSource}
@ -151,6 +208,7 @@ const DetailsPanelPopover = ({
<FiltersContainer> <FiltersContainer>
{appliedIndicators.map(indicator => ( {appliedIndicators.map(indicator => (
<FilterIndicator <FilterIndicator
ref={el => indicatorRefs.current.push(el)}
key={indicatorKey(indicator)} key={indicatorKey(indicator)}
indicator={indicator} indicator={indicator}
onClick={onHighlightFilterSource} onClick={onHighlightFilterSource}
@ -167,10 +225,10 @@ const DetailsPanelPopover = ({
<Popover <Popover
overlayClassName="filterStatusPopover" overlayClassName="filterStatusPopover"
content={content} content={content}
visible={visible} visible={popoverVisible}
onVisibleChange={handlePopoverStatus} onVisibleChange={handleVisibility}
placement="bottomRight" placement="bottomRight"
trigger="hover" trigger={['hover']}
> >
{children} {children}
</Popover> </Popover>

View File

@ -17,7 +17,7 @@
* under the License. * under the License.
*/ */
import React, { FC } from 'react'; import React from 'react';
import { css } from '@superset-ui/core'; import { css } from '@superset-ui/core';
import Icons from 'src/components/Icons'; import Icons from 'src/components/Icons';
import { getFilterValueForDisplay } from 'src/dashboard/components/nativeFilters/utils'; import { getFilterValueForDisplay } from 'src/dashboard/components/nativeFilters/utils';
@ -33,38 +33,39 @@ export interface IndicatorProps {
onClick?: (path: string[]) => void; onClick?: (path: string[]) => void;
} }
const FilterIndicator: FC<IndicatorProps> = ({ const FilterIndicator = React.forwardRef<HTMLButtonElement, IndicatorProps>(
indicator: { column, name, value, path = [] }, ({ indicator: { column, name, value, path = [] }, onClick }, ref) => {
onClick, const resultValue = getFilterValueForDisplay(value);
}) => { return (
const resultValue = getFilterValueForDisplay(value); <FilterItem
return ( ref={ref}
<FilterItem onClick={
onClick={ onClick ? () => onClick([...path, `LABEL-${column}`]) : undefined
onClick ? () => onClick([...path, `LABEL-${column}`]) : undefined }
} tabIndex={-1}
> >
{onClick && ( {onClick && (
<i> <i>
<Icons.SearchOutlined <Icons.SearchOutlined
iconSize="m" iconSize="m"
css={css` css={css`
span { span {
vertical-align: 0; vertical-align: 0;
} }
`} `}
/> />
</i> </i>
)} )}
<div> <div>
<FilterName> <FilterName>
{name} {name}
{resultValue ? ': ' : ''} {resultValue ? ': ' : ''}
</FilterName> </FilterName>
<FilterValue>{resultValue}</FilterValue> <FilterValue>{resultValue}</FilterValue>
</div> </div>
</FilterItem> </FilterItem>
); );
}; },
);
export default FilterIndicator; export default FilterIndicator;

View File

@ -93,7 +93,8 @@ export const FilterItem = styled.button`
transition: opacity ease-in-out ${theme.transitionTiming}; transition: opacity ease-in-out ${theme.transitionTiming};
} }
&:hover i svg { &:hover i svg,
&:focus-visible i svg {
opacity: 1; opacity: 1;
} }
`} `}

View File

@ -16,7 +16,13 @@
* specific language governing permissions and limitations * specific language governing permissions and limitations
* under the License. * 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 { useDispatch, useSelector } from 'react-redux';
import { uniqWith } from 'lodash'; import { uniqWith } from 'lodash';
import cx from 'classnames'; import cx from 'classnames';
@ -25,6 +31,7 @@ import {
Filters, Filters,
JsonObject, JsonObject,
styled, styled,
t,
usePrevious, usePrevious,
} from '@superset-ui/core'; } from '@superset-ui/core';
import Icons from 'src/components/Icons'; import Icons from 'src/components/Icons';
@ -126,6 +133,9 @@ export const FiltersBadge = ({ chartId }: FiltersBadgeProps) => {
const [dashboardIndicators, setDashboardIndicators] = useState<Indicator[]>( const [dashboardIndicators, setDashboardIndicators] = useState<Indicator[]>(
indicatorsInitialState, indicatorsInitialState,
); );
const [popoverVisible, setPopoverVisible] = useState(false);
const popoverContentRef = useRef<HTMLDivElement>(null);
const popoverTriggerRef = useRef<HTMLDivElement>(null);
const onHighlightFilterSource = useCallback( const onHighlightFilterSource = useCallback(
(path: string[]) => { (path: string[]) => {
@ -134,6 +144,12 @@ export const FiltersBadge = ({ chartId }: FiltersBadgeProps) => {
[dispatch], [dispatch],
); );
const handleKeyDown = (event: React.KeyboardEvent<HTMLDivElement>) => {
if (event.key === 'Enter') {
setPopoverVisible(true);
}
};
const prevChart = usePrevious(chart); const prevChart = usePrevious(chart);
const prevChartStatus = prevChart?.chartStatus; const prevChartStatus = prevChart?.chartStatus;
const prevDashboardFilters = usePrevious(dashboardFilters); const prevDashboardFilters = usePrevious(dashboardFilters);
@ -141,6 +157,14 @@ export const FiltersBadge = ({ chartId }: FiltersBadgeProps) => {
const showIndicators = const showIndicators =
chart?.chartStatus && ['rendered', 'success'].includes(chart.chartStatus); chart?.chartStatus && ['rendered', 'success'].includes(chart.chartStatus);
useEffect(() => {
if (popoverVisible) {
setTimeout(() => {
popoverContentRef?.current?.focus();
});
}
}, [popoverVisible]);
useEffect(() => { useEffect(() => {
if (!showIndicators && dashboardIndicators.length > 0) { if (!showIndicators && dashboardIndicators.length > 0) {
setDashboardIndicators(indicatorsInitialState); setDashboardIndicators(indicatorsInitialState);
@ -180,6 +204,7 @@ export const FiltersBadge = ({ chartId }: FiltersBadgeProps) => {
const prevDashboardLayout = usePrevious(present); const prevDashboardLayout = usePrevious(present);
const prevDataMask = usePrevious(dataMask); const prevDataMask = usePrevious(dataMask);
const prevChartConfig = usePrevious(chartConfiguration); const prevChartConfig = usePrevious(chartConfiguration);
useEffect(() => { useEffect(() => {
if (!showIndicators && nativeIndicators.length > 0) { if (!showIndicators && nativeIndicators.length > 0) {
setNativeIndicators(indicatorsInitialState); setNativeIndicators(indicatorsInitialState);
@ -250,6 +275,8 @@ export const FiltersBadge = ({ chartId }: FiltersBadgeProps) => {
), ),
[indicators], [indicators],
); );
const filterCount =
appliedIndicators.length + appliedCrossFilterIndicators.length;
if (!appliedCrossFilterIndicators.length && !appliedIndicators.length) { if (!appliedCrossFilterIndicators.length && !appliedIndicators.length) {
return null; return null;
@ -260,18 +287,28 @@ export const FiltersBadge = ({ chartId }: FiltersBadgeProps) => {
appliedCrossFilterIndicators={appliedCrossFilterIndicators} appliedCrossFilterIndicators={appliedCrossFilterIndicators}
appliedIndicators={appliedIndicators} appliedIndicators={appliedIndicators}
onHighlightFilterSource={onHighlightFilterSource} onHighlightFilterSource={onHighlightFilterSource}
setPopoverVisible={setPopoverVisible}
popoverVisible={popoverVisible}
popoverContentRef={popoverContentRef}
popoverTriggerRef={popoverTriggerRef}
> >
<StyledFilterCount <StyledFilterCount
aria-label={t('Applied filters (%s)', filterCount)}
aria-haspopup="true"
role="button"
ref={popoverTriggerRef}
className={cx( className={cx(
'filter-counts', 'filter-counts',
!!appliedCrossFilterIndicators.length && 'has-cross-filters', !!appliedCrossFilterIndicators.length && 'has-cross-filters',
)} )}
tabIndex={0}
onKeyDown={handleKeyDown}
> >
<Icons.Filter iconSize="m" /> <Icons.Filter iconSize="m" />
<StyledBadge <StyledBadge
data-test="applied-filter-count" data-test="applied-filter-count"
className="applied-count" className="applied-count"
count={appliedIndicators.length + appliedCrossFilterIndicators.length} count={filterCount}
showZero showZero
/> />
</StyledFilterCount> </StyledFilterCount>

View File

@ -936,6 +936,7 @@ const SliceHeaderControls = (props: SliceHeaderControlsPropsWithRouter) => {
id={`slice_${slice.slice_id}-controls`} id={`slice_${slice.slice_id}-controls`}
role="button" role="button"
aria-label="More Options" aria-label="More Options"
aria-haspopup="true"
tabIndex={0} tabIndex={0}
> >
<VerticalDotsTrigger /> <VerticalDotsTrigger />