From 163db7d6316d003780e3637074dffdf1cddd7c59 Mon Sep 17 00:00:00 2001 From: Noah Waldner Date: Fri, 16 Aug 2024 15:14:58 +0200 Subject: [PATCH 1/4] handle focusing of interactive elements inside of a flyout --- .../src/components/Flyout/Flyout.stories.tsx | 26 +++++- .../src/components/Flyout/Flyout.tsx | 23 +++++- .../components/Flyout/__tests__/Flyout.ct.tsx | 82 +++++++++++++++++++ 3 files changed, 129 insertions(+), 2 deletions(-) diff --git a/packages/components/src/components/Flyout/Flyout.stories.tsx b/packages/components/src/components/Flyout/Flyout.stories.tsx index 396357f9c0..53511478bd 100644 --- a/packages/components/src/components/Flyout/Flyout.stories.tsx +++ b/packages/components/src/components/Flyout/Flyout.stories.tsx @@ -3,8 +3,9 @@ import { type Meta, type StoryObj } from '@storybook/react'; import { Button } from '../Button/Button'; +import { TextInput } from '../TextInput/TextInput'; -import { Flyout, FlyoutContent, FlyoutRoot, FlyoutTrigger, FlyoutHeader, FlyoutBody, FlyoutFooter } from './Flyout'; +import { Flyout, FlyoutBody, FlyoutContent, FlyoutFooter, FlyoutHeader, FlyoutRoot, FlyoutTrigger } from './Flyout'; type Story = StoryObj; const meta: Meta = { @@ -104,6 +105,29 @@ export const WithFooter: Story = { }, }; +export const WithFocusableContent: Story = { + args: { + children: 'Hello World', + }, + render: ({ ...args }) => { + return ( + + + + + + Header + + + + + + + + ); + }, +}; + export const WithHeaderAndFooter: Story = {}; export const WithCloseButton: Story = { diff --git a/packages/components/src/components/Flyout/Flyout.tsx b/packages/components/src/components/Flyout/Flyout.tsx index db011543b6..e5f516e4e4 100644 --- a/packages/components/src/components/Flyout/Flyout.tsx +++ b/packages/components/src/components/Flyout/Flyout.tsx @@ -80,7 +80,18 @@ export const FlyoutTrigger = ( ref: ForwardedRef, ) => { return ( - + { + console.log('mouse'); + + mouseEvent.currentTarget.dataset.autoFocusVisible = 'false'; + }} + data-auto-focus-visible="true" + data-flyout-trigger + data-test-id={dataTestId} + asChild + ref={ref} + > {children} ); @@ -113,6 +124,16 @@ export const FlyoutContent = ( className={flyoutContentStyles({ ...props })} data-flyout-spacing={padding} data-test-id={dataTestId} + onFocus={(e) => { + const triggerElement = e.relatedTarget as HTMLElement; + if (triggerElement && triggerElement.dataset.flyoutTrigger) { + const focusVisible = triggerElement.dataset.autoFocusVisible === 'true'; + triggerElement.dataset.autoFocusVisible = 'true'; + if (!focusVisible) { + e.target.dataset.showFocusRing = 'false'; + } + } + }} {...props} > {children} diff --git a/packages/components/src/components/Flyout/__tests__/Flyout.ct.tsx b/packages/components/src/components/Flyout/__tests__/Flyout.ct.tsx index 81141e0989..a8caee678b 100644 --- a/packages/components/src/components/Flyout/__tests__/Flyout.ct.tsx +++ b/packages/components/src/components/Flyout/__tests__/Flyout.ct.tsx @@ -4,6 +4,7 @@ import { expect, test } from '@playwright/experimental-ct-react'; import sinon from 'sinon'; import { Button } from '#/components/Button/Button'; +import { TextInput } from '#/components/TextInput/TextInput'; import { Flyout } from '../Flyout'; @@ -16,6 +17,9 @@ const FLYOUT_HEADER_TEST_ID = 'fondue-flyout-header'; const FLYOUT_HEADER_TEXT = 'Flyout Header'; const FLYOUT_FOOTER_TEST_ID = 'fondue-flyout-footer'; const FLYOUT_FOOTER_TEXT = 'Flyout Footer'; +const TEXT_INPUT_TEST_ID_1 = 'fondue-text-input-1'; +const TEXT_INPUT_TEST_ID_2 = 'fondue-text-input-2'; +const TEXT_INPUT_TEST_ID_3 = 'fondue-text-input-3'; test('should render trigger', async ({ mount, page }) => { const component = await mount( @@ -364,3 +368,81 @@ test('should trigger callback on open and close', async ({ mount, page }) => { await page.click('body'); expect(onOpenChange.calledWith(false)).toBe(true); }); + +test('should render focus visible input on enter press', async ({ mount, page }) => { + const component = await mount( + + + + + + + + + + + + , + ); + const tooltipTrigger = page.getByTestId(FLYOUT_TRIGGER_TEST_ID); + const textInput1 = page.getByTestId(TEXT_INPUT_TEST_ID_1); + const textInput2 = page.getByTestId(TEXT_INPUT_TEST_ID_2); + const textInput3 = page.getByTestId(TEXT_INPUT_TEST_ID_3); + await expect(component).toBeVisible(); + await expect(tooltipTrigger).toBeVisible(); + await tooltipTrigger.focus(); + await page.keyboard.press('Enter'); + await expect(textInput1).toHaveCSS( + 'box-shadow', + 'rgb(255, 255, 255) 0px 0px 0px 2px, rgb(65, 150, 251) 0px 0px 0px 6px', + ); + await page.keyboard.press('Tab'); + await expect(textInput2).toHaveCSS( + 'box-shadow', + 'rgb(255, 255, 255) 0px 0px 0px 2px, rgb(65, 150, 251) 0px 0px 0px 6px', + ); + await textInput3.click(); + await expect(textInput3).not.toHaveCSS( + 'box-shadow', + 'rgb(255, 255, 255) 0px 0px 0px 2px, rgb(65, 150, 251) 0px 0px 0px 6px', + ); +}); + +test('should not render focus visible input on click', async ({ mount, page }) => { + const component = await mount( + + + + + + + + + + + + , + ); + const tooltipTrigger = page.getByTestId(FLYOUT_TRIGGER_TEST_ID); + const textInput1 = page.getByTestId(TEXT_INPUT_TEST_ID_1); + const textInput2 = page.getByTestId(TEXT_INPUT_TEST_ID_2); + const textInput3 = page.getByTestId(TEXT_INPUT_TEST_ID_3); + await expect(component).toBeVisible(); + await expect(tooltipTrigger).toBeVisible(); + await tooltipTrigger.focus(); + await tooltipTrigger.click(); + await expect(textInput1).not.toHaveCSS( + 'box-shadow', + 'rgb(255, 255, 255) 0px 0px 0px 2px, rgb(65, 150, 251) 0px 0px 0px 6px', + ); + await textInput2.click(); + await expect(textInput2).not.toHaveCSS( + 'box-shadow', + 'rgb(255, 255, 255) 0px 0px 0px 2px, rgb(65, 150, 251) 0px 0px 0px 6px', + ); + await page.keyboard.press('Tab'); + await expect(textInput3).toHaveCSS( + 'box-shadow', + 'rgb(255, 255, 255) 0px 0px 0px 2px, rgb(65, 150, 251) 0px 0px 0px 6px', + ); +}); From 5fb83208b4ddc3e06365e95e6934037129cf2b6a Mon Sep 17 00:00:00 2001 From: Noah Waldner Date: Fri, 16 Aug 2024 15:15:30 +0200 Subject: [PATCH 2/4] add changeset --- .changeset/blue-doors-hammer.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/blue-doors-hammer.md diff --git a/.changeset/blue-doors-hammer.md b/.changeset/blue-doors-hammer.md new file mode 100644 index 0000000000..4be551f61e --- /dev/null +++ b/.changeset/blue-doors-hammer.md @@ -0,0 +1,5 @@ +--- +"@frontify/fondue-components": patch +--- + +feat: add handling for autofocus in flyout From 7feb2255056d3e00e3ad16a0026488030577ed9f Mon Sep 17 00:00:00 2001 From: Noah Waldner Date: Fri, 16 Aug 2024 15:16:37 +0200 Subject: [PATCH 3/4] remove log --- packages/components/src/components/Flyout/Flyout.tsx | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/components/src/components/Flyout/Flyout.tsx b/packages/components/src/components/Flyout/Flyout.tsx index e5f516e4e4..1d6871ce21 100644 --- a/packages/components/src/components/Flyout/Flyout.tsx +++ b/packages/components/src/components/Flyout/Flyout.tsx @@ -82,8 +82,6 @@ export const FlyoutTrigger = ( return ( { - console.log('mouse'); - mouseEvent.currentTarget.dataset.autoFocusVisible = 'false'; }} data-auto-focus-visible="true" From 573706187e51dcaa307a32280ae5b662ccc0d2a1 Mon Sep 17 00:00:00 2001 From: Noah Waldner Date: Fri, 16 Aug 2024 15:50:16 +0200 Subject: [PATCH 4/4] added helpers --- .../src/components/Flyout/Flyout.tsx | 6 +-- .../components/Flyout/__tests__/Flyout.ct.tsx | 37 +++++++------------ 2 files changed, 16 insertions(+), 27 deletions(-) diff --git a/packages/components/src/components/Flyout/Flyout.tsx b/packages/components/src/components/Flyout/Flyout.tsx index 1d6871ce21..ae4b1f7c88 100644 --- a/packages/components/src/components/Flyout/Flyout.tsx +++ b/packages/components/src/components/Flyout/Flyout.tsx @@ -122,13 +122,13 @@ export const FlyoutContent = ( className={flyoutContentStyles({ ...props })} data-flyout-spacing={padding} data-test-id={dataTestId} - onFocus={(e) => { - const triggerElement = e.relatedTarget as HTMLElement; + onFocus={(event) => { + const triggerElement = event.relatedTarget as HTMLElement; if (triggerElement && triggerElement.dataset.flyoutTrigger) { const focusVisible = triggerElement.dataset.autoFocusVisible === 'true'; triggerElement.dataset.autoFocusVisible = 'true'; if (!focusVisible) { - e.target.dataset.showFocusRing = 'false'; + event.target.dataset.showFocusRing = 'false'; } } }} diff --git a/packages/components/src/components/Flyout/__tests__/Flyout.ct.tsx b/packages/components/src/components/Flyout/__tests__/Flyout.ct.tsx index a8caee678b..e49e30907d 100644 --- a/packages/components/src/components/Flyout/__tests__/Flyout.ct.tsx +++ b/packages/components/src/components/Flyout/__tests__/Flyout.ct.tsx @@ -5,6 +5,7 @@ import sinon from 'sinon'; import { Button } from '#/components/Button/Button'; import { TextInput } from '#/components/TextInput/TextInput'; +import { FOCUS_BORDER_CSS, FOCUS_OUTLINE_CSS } from '#/helpers/constants'; import { Flyout } from '../Flyout'; @@ -392,20 +393,14 @@ test('should render focus visible input on enter press', async ({ mount, page }) await expect(tooltipTrigger).toBeVisible(); await tooltipTrigger.focus(); await page.keyboard.press('Enter'); - await expect(textInput1).toHaveCSS( - 'box-shadow', - 'rgb(255, 255, 255) 0px 0px 0px 2px, rgb(65, 150, 251) 0px 0px 0px 6px', - ); + await expect(textInput1).toHaveCSS(...FOCUS_OUTLINE_CSS); + await expect(textInput1).not.toHaveCSS(...FOCUS_BORDER_CSS); await page.keyboard.press('Tab'); - await expect(textInput2).toHaveCSS( - 'box-shadow', - 'rgb(255, 255, 255) 0px 0px 0px 2px, rgb(65, 150, 251) 0px 0px 0px 6px', - ); + await expect(textInput2).toHaveCSS(...FOCUS_OUTLINE_CSS); + await expect(textInput2).not.toHaveCSS(...FOCUS_BORDER_CSS); await textInput3.click(); - await expect(textInput3).not.toHaveCSS( - 'box-shadow', - 'rgb(255, 255, 255) 0px 0px 0px 2px, rgb(65, 150, 251) 0px 0px 0px 6px', - ); + await expect(textInput3).not.toHaveCSS(...FOCUS_OUTLINE_CSS); + await expect(textInput3).toHaveCSS(...FOCUS_BORDER_CSS); }); test('should not render focus visible input on click', async ({ mount, page }) => { @@ -431,18 +426,12 @@ test('should not render focus visible input on click', async ({ mount, page }) = await expect(tooltipTrigger).toBeVisible(); await tooltipTrigger.focus(); await tooltipTrigger.click(); - await expect(textInput1).not.toHaveCSS( - 'box-shadow', - 'rgb(255, 255, 255) 0px 0px 0px 2px, rgb(65, 150, 251) 0px 0px 0px 6px', - ); + await expect(textInput1).not.toHaveCSS(...FOCUS_OUTLINE_CSS); + await expect(textInput1).toHaveCSS(...FOCUS_BORDER_CSS); await textInput2.click(); - await expect(textInput2).not.toHaveCSS( - 'box-shadow', - 'rgb(255, 255, 255) 0px 0px 0px 2px, rgb(65, 150, 251) 0px 0px 0px 6px', - ); + await expect(textInput2).not.toHaveCSS(...FOCUS_OUTLINE_CSS); + await expect(textInput2).toHaveCSS(...FOCUS_BORDER_CSS); await page.keyboard.press('Tab'); - await expect(textInput3).toHaveCSS( - 'box-shadow', - 'rgb(255, 255, 255) 0px 0px 0px 2px, rgb(65, 150, 251) 0px 0px 0px 6px', - ); + await expect(textInput3).toHaveCSS(...FOCUS_OUTLINE_CSS); + await expect(textInput3).not.toHaveCSS(...FOCUS_BORDER_CSS); });