From 4bb4f3a86f81c7dc61e5fe5b506981f2630421c9 Mon Sep 17 00:00:00 2001 From: Avinash Dwarapu Date: Fri, 25 Nov 2022 11:36:11 +0100 Subject: [PATCH] fix: Allow popovers in charts to be dismissed by pressing Esc (#514) --- src/area-chart/__integ__/area-chart.test.ts | 21 ++++++++++++++ src/area-chart/internal.tsx | 6 ++++ src/area-chart/model/index.ts | 1 + src/area-chart/model/use-chart-model.ts | 8 +++++ .../__integ__/mixed-line-bar-chart.test.ts | 29 +++++++++++++++++++ src/mixed-line-bar-chart/chart-container.tsx | 22 ++++++++++---- src/pie-chart/__integ__/pie-chart.test.ts | 21 ++++++++++++++ src/pie-chart/pie-chart.tsx | 13 ++++++++- 8 files changed, 114 insertions(+), 7 deletions(-) diff --git a/src/area-chart/__integ__/area-chart.test.ts b/src/area-chart/__integ__/area-chart.test.ts index b4e915bc76..f27d0ef2ae 100644 --- a/src/area-chart/__integ__/area-chart.test.ts +++ b/src/area-chart/__integ__/area-chart.test.ts @@ -192,6 +192,27 @@ describe('Popover', () => { await expect(page.getPopoverTitle()).resolves.toBe('5s'); }) ); + + test( + 'popover can be closed when Escape is pressed after hover', + setupTest('#/light/area-chart/test', 'Linear latency chart', async page => { + await expect(page.hasPopover()).resolves.toBe(false); + await page.hoverElement(page.chart.toSelector()); + await expect(page.hasPopover()).resolves.toBe(true); + await page.keys(['Escape']); + await expect(page.hasPopover()).resolves.toBe(false); + }) + ); + + test( + 'popover can be closed when Escape is pressed after chart plot is focused', + setupTest('#/light/area-chart/test', 'Linear latency chart', async page => { + await page.focusPlot(); + await expect(page.hasPopover()).resolves.toBe(true); + await page.keys(['Escape']); + await expect(page.hasPopover()).resolves.toBe(false); + }) + ); }); describe('Keyboard navigation', () => { diff --git a/src/area-chart/internal.tsx b/src/area-chart/internal.tsx index 80635dd979..d82a40562a 100644 --- a/src/area-chart/internal.tsx +++ b/src/area-chart/internal.tsx @@ -114,6 +114,12 @@ export default function InternalAreaChart({ const reserveLegendSpace = !showChart && !hideLegend; const reserveFilterSpace = !showChart && !isNoMatch && (!hideFilter || additionalFilters); + useEffect(() => { + const onKeyDown = model.handlers.onDocumentKeyDown; + document.addEventListener('keydown', onKeyDown); + return () => document.removeEventListener('keydown', onKeyDown); + }, [model.handlers.onDocumentKeyDown]); + const onBlur = (event: React.FocusEvent) => { if (event.relatedTarget && !nodeContains(containerRef.current, event.relatedTarget)) { model.handlers.onContainerBlur(); diff --git a/src/area-chart/model/index.ts b/src/area-chart/model/index.ts index d51cdb6a7c..23132734a1 100644 --- a/src/area-chart/model/index.ts +++ b/src/area-chart/model/index.ts @@ -26,6 +26,7 @@ export interface ChartModel { onLegendHighlight: (series: null | AreaChartProps.Series) => void; onPopoverDismiss: (outsideClick?: boolean) => void; onContainerBlur: () => void; + onDocumentKeyDown: (event: KeyboardEvent) => void; }; interactions: ReadonlyAsyncStore>; refs: { diff --git a/src/area-chart/model/use-chart-model.ts b/src/area-chart/model/use-chart-model.ts index 4df3b775e2..ec8134ec69 100644 --- a/src/area-chart/model/use-chart-model.ts +++ b/src/area-chart/model/use-chart-model.ts @@ -241,6 +241,13 @@ export default function useChartModel({ interactions.clearState(); }; + const onDocumentKeyDown = (event: KeyboardEvent) => { + if (event.key === 'Escape') { + interactions.clearHighlight(); + interactions.clearHighlightedLegend(); + } + }; + return { width, height, @@ -260,6 +267,7 @@ export default function useChartModel({ onLegendHighlight, onPopoverDismiss, onContainerBlur, + onDocumentKeyDown, }, refs: { plot: plotRef, diff --git a/src/mixed-line-bar-chart/__integ__/mixed-line-bar-chart.test.ts b/src/mixed-line-bar-chart/__integ__/mixed-line-bar-chart.test.ts index 061da886ff..2692d20bae 100644 --- a/src/mixed-line-bar-chart/__integ__/mixed-line-bar-chart.test.ts +++ b/src/mixed-line-bar-chart/__integ__/mixed-line-bar-chart.test.ts @@ -336,6 +336,35 @@ describe('Details popover', () => { }) ); + test( + 'can be hidden after hover by pressing Escape', + setupTest('#/light/mixed-line-bar-chart/test', async page => { + // Hover over first group + await page.hoverElement(chartWrapper.findBarGroups().get(1).toSelector()); + await expect(page.getText(popoverHeaderSelector())).resolves.toContain('Potatoes'); + + // Pressing escape should hide the popover + await page.keys(['Escape']); + await expect(page.isDisplayed(popoverDismissSelector())).resolves.toBe(false); + await expect(page.isDisplayed(popoverHeaderSelector())).resolves.toBe(false); + }) + ); + + test( + 'can be hidden after keyboard navigation by pressing Escape', + setupTest('#/light/bar-chart/test', async page => { + // Navigate first group in the first chart + await page.click('#focus-target'); + await page.keys(['Tab', 'Tab', 'ArrowRight']); + await expect(page.getText(popoverHeaderSelector())).resolves.toContain('Potatoes'); + + // Pressing Escape should close the popover + await page.keys(['Escape']); + await expect(page.isDisplayed(popoverHeaderSelector())).resolves.toBe(false); + await expect(page.isDisplayed(popoverDismissSelector())).resolves.toBe(false); + }) + ); + test( 'can be pinned by clicking on chart background and dismissed by clicking outside chart area in line chart', setupTest('#/light/line-chart/test', async page => { diff --git a/src/mixed-line-bar-chart/chart-container.tsx b/src/mixed-line-bar-chart/chart-container.tsx index 86dd0356cd..f86590db48 100644 --- a/src/mixed-line-bar-chart/chart-container.tsx +++ b/src/mixed-line-bar-chart/chart-container.tsx @@ -263,6 +263,22 @@ export default function ChartContainer({ return null; }, [highlightedPoint, verticalMarkerLeft, highlightedGroupIndex, scaledSeries, barGroups]); + useEffect(() => { + const onKeyDown = (event: KeyboardEvent) => { + if (event.key === 'Escape') { + dismissPopover(); + } + }; + document.addEventListener('keydown', onKeyDown); + return () => document.removeEventListener('keydown', onKeyDown); + }, [dismissPopover]); + + useLayoutEffect(() => { + if (highlightedX !== null || highlightedPoint !== null) { + showPopover(); + } + }, [highlightedX, highlightedPoint, showPopover]); + const onPopoverDismiss = (outsideClick?: boolean) => { dismissPopover(); @@ -278,12 +294,6 @@ export default function ChartContainer({ } }; - useLayoutEffect(() => { - if (highlightedX !== null) { - showPopover(); - } - }, [highlightedX, showPopover]); - const onSVGMouseDown = (e: React.MouseEvent) => { if (isPopoverOpen) { if (isPopoverPinned) { diff --git a/src/pie-chart/__integ__/pie-chart.test.ts b/src/pie-chart/__integ__/pie-chart.test.ts index eeba95a6e6..76a783091c 100644 --- a/src/pie-chart/__integ__/pie-chart.test.ts +++ b/src/pie-chart/__integ__/pie-chart.test.ts @@ -299,6 +299,27 @@ describe('Detail popover', () => { await expect(page.isDisplayed(detailsPopoverSelector)).resolves.toBe(false); }) ); + + test( + 'can be dismissed after hovering on the segment by pressing Escape', + setupTest(async page => { + await page.hoverElement(pieWrapper.findSegments().get(1).toSelector()); + await page.waitForVisible(detailsPopoverSelector); + await page.keys(['Escape']); + await expect(page.isDisplayed(detailsPopoverSelector)).resolves.toBe(false); + }) + ); + + test( + 'can be dismissed after navigating to the segment with keyboard by pressing Escape', + setupTest(async page => { + await page.click('#focus-target'); + await page.keys(['Tab', 'Tab', 'Enter']); + await page.waitForVisible(detailsPopoverSelector); + await page.keys(['Escape']); + await expect(page.isDisplayed(detailsPopoverSelector)).resolves.toBe(false); + }) + ); }); describe('Focus outline', () => { diff --git a/src/pie-chart/pie-chart.tsx b/src/pie-chart/pie-chart.tsx index 588e8d5869..29ffff54b7 100644 --- a/src/pie-chart/pie-chart.tsx +++ b/src/pie-chart/pie-chart.tsx @@ -1,6 +1,6 @@ // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 -import React, { useCallback, useMemo, useRef, useState } from 'react'; +import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import clsx from 'clsx'; import { pie } from 'd3-shape'; @@ -164,11 +164,22 @@ export default ({ }, [highlightedSegment, setTooltipOpen, onHighlightChange] ); + const clearHighlightedSegment = useCallback(() => { setTooltipOpen(false); onHighlightChange(null); }, [onHighlightChange, setTooltipOpen]); + useEffect(() => { + const onKeyDown = (event: KeyboardEvent) => { + if (event.key === 'Escape') { + clearHighlightedSegment(); + } + }; + document.addEventListener('keydown', onKeyDown); + return () => document.removeEventListener('keydown', onKeyDown); + }, [clearHighlightedSegment]); + const onMouseDown = useCallback( (internalDatum: InternalChartDatum) => { if (pinnedSegment === internalDatum.datum) {