From c8fee79a0ad9602cf0cb2e97002f6b44359e4a15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Costa?= Date: Fri, 16 Aug 2024 14:18:41 +0200 Subject: [PATCH] fix: prevent focus styles duplication (#2009) --- .changeset/ten-clocks-lie.md | 5 ++ .../src/components/Select/Select.tsx | 21 ++++++- .../components/Select/__tests__/Select.ct.tsx | 57 ++++++++++++++++++- .../Select/styles/select.module.scss | 2 +- .../TextInput/__tests__/TextInput.ct.tsx | 37 +++++++----- .../TextInput/styles/text.module.scss | 2 +- packages/components/src/helpers/constants.ts | 8 +++ 7 files changed, 112 insertions(+), 20 deletions(-) create mode 100644 .changeset/ten-clocks-lie.md create mode 100644 packages/components/src/helpers/constants.ts diff --git a/.changeset/ten-clocks-lie.md b/.changeset/ten-clocks-lie.md new file mode 100644 index 0000000000..f85ab2d3cc --- /dev/null +++ b/.changeset/ten-clocks-lie.md @@ -0,0 +1,5 @@ +--- +"@frontify/fondue-components": patch +--- + +fix: prevent outline style duplication (input and select) diff --git a/packages/components/src/components/Select/Select.tsx b/packages/components/src/components/Select/Select.tsx index 093770f434..a46d75b06d 100644 --- a/packages/components/src/components/Select/Select.tsx +++ b/packages/components/src/components/Select/Select.tsx @@ -4,7 +4,7 @@ import { IconCaretDown, IconCheckMark, IconExclamationMarkTriangle } from '@fron import * as RadixPopover from '@radix-ui/react-popover'; import { Slot as RadixSlot } from '@radix-ui/react-slot'; import { useSelect } from 'downshift'; -import { forwardRef, type ForwardedRef, type ReactNode } from 'react'; +import { forwardRef, useRef, type ForwardedRef, type ReactNode } from 'react'; import { ForwardedRefCombobox } from './Combobox'; import { ForwardedRefSelectItem, ForwardedRefSelectItemGroup } from './SelectItem'; @@ -72,6 +72,8 @@ export const SelectInput = ( const defaultItem = getItemByValue(defaultValue); const activeItem = getItemByValue(value); + const wasClicked = useRef(false); + const { getToggleButtonProps, getMenuProps, getItemProps, reset, selectedItem, isOpen, highlightedIndex } = useSelect({ items, @@ -85,7 +87,22 @@ export const SelectInput = ( return ( - + { + wasClicked.current = true; + mouseEvent.currentTarget.dataset.showFocusRing = 'false'; + }} + onFocus={(focusEvent) => { + if (!wasClicked.current) { + focusEvent.target.dataset.showFocusRing = 'true'; + } + }} + onBlur={(blurEvent) => { + blurEvent.target.dataset.showFocusRing = 'false'; + wasClicked.current = false; + }} + >
parseFloat(window.getComputedStyle(node).maxHeight)); expect(actualMaxHeight).toBe(expectedMaxHeight); }); + +test('render focus ring and no border when keyboard focused', async ({ mount, page }) => { + await mount( + , + ); + + const select = page.getByTestId(SELECT_TEST_ID); + + await page.focus('body'); + + await expect(select).not.toHaveCSS(...FOCUS_OUTLINE_CSS); + await expect(select).not.toHaveCSS(...FOCUS_BORDER_CSS); + + await page.keyboard.press('Tab'); + + await expect(select).toBeFocused(); + + await expect(select).toHaveCSS(...FOCUS_OUTLINE_CSS); + await expect(select).not.toHaveCSS(...FOCUS_BORDER_CSS); +}); + +test('render border and no focus ring when mouse focused', async ({ mount, page }) => { + const component = await mount( + , + ); + const select = page.getByTestId(SELECT_TEST_ID); + + await page.focus('body'); + + await expect(select).not.toHaveCSS(...FOCUS_BORDER_CSS); + await expect(select).not.toHaveCSS(...FOCUS_OUTLINE_CSS); + + await component.hover(); + + await page.mouse.down(); + await expect(select).toHaveCSS(...FOCUS_BORDER_CSS); + await page.mouse.up(); + + await expect(select).toBeFocused(); + + const dialog = page.getByTestId(SELECT_MENU_TEST_ID); + + await expect(dialog).toHaveCSS(...FOCUS_BORDER_CSS); + await expect(select).not.toHaveCSS(...FOCUS_OUTLINE_CSS); +}); diff --git a/packages/components/src/components/Select/styles/select.module.scss b/packages/components/src/components/Select/styles/select.module.scss index fb1a0330e0..ea469b39a1 100644 --- a/packages/components/src/components/Select/styles/select.module.scss +++ b/packages/components/src/components/Select/styles/select.module.scss @@ -28,7 +28,7 @@ border-color: var(--line-color-x-strong); } - &[aria-expanded='false']:focus { + &[aria-expanded='false'][data-show-focus-ring='false']:focus { border-color: var(--line-color-xx-strong); } diff --git a/packages/components/src/components/TextInput/__tests__/TextInput.ct.tsx b/packages/components/src/components/TextInput/__tests__/TextInput.ct.tsx index be223dff95..58f551b6de 100644 --- a/packages/components/src/components/TextInput/__tests__/TextInput.ct.tsx +++ b/packages/components/src/components/TextInput/__tests__/TextInput.ct.tsx @@ -3,6 +3,8 @@ import { expect, test } from '@playwright/experimental-ct-react'; import sinon from 'sinon'; +import { FOCUS_BORDER_CSS, FOCUS_OUTLINE_CSS } from '#/helpers/constants'; + import { TextInput } from '../TextInput'; const TEXT_INPUT_TEXT = 'sample text input'; @@ -114,31 +116,36 @@ test('render slot on the right side and apply correct focus order', async ({ mou await expect(component.getByTestId('right-button-slot')).toBeFocused(); }); -test('render focus ring when keyboard focused', async ({ mount, page }) => { +test('render focus ring and no border when keyboard focused', async ({ mount, page }) => { const component = await mount(); + const input = page.getByTestId(TEXT_INPUT_TEST_ID); await page.focus('body'); - await expect(page.getByTestId(TEXT_INPUT_TEST_ID)).not.toHaveCSS('outline-width', '4px'); + + await expect(input).not.toHaveCSS(...FOCUS_OUTLINE_CSS); + await expect(input).not.toHaveCSS(...FOCUS_BORDER_CSS); + await page.keyboard.press('Tab'); + await expect(component.getByRole('textbox')).toBeFocused(); - await expect(page.getByTestId(TEXT_INPUT_TEST_ID)).toHaveCSS( - 'box-shadow', - 'rgb(255, 255, 255) 0px 0px 0px 2px, rgb(65, 150, 251) 0px 0px 0px 6px', - ); + + await expect(input).toHaveCSS(...FOCUS_OUTLINE_CSS); + await expect(input).not.toHaveCSS(...FOCUS_BORDER_CSS); }); -test('render no focus ring when keyboard focused', async ({ mount, page }) => { +test('render border and no focus ring when mouse focused', async ({ mount, page }) => { const component = await mount(); + const input = page.getByTestId(TEXT_INPUT_TEST_ID); await page.focus('body'); - await expect(page.getByTestId(TEXT_INPUT_TEST_ID)).not.toHaveCSS( - 'box-shadow', - 'rgb(255, 255, 255) 0px 0px 0px 2px, rgb(65, 150, 251) 0px 0px 0px 6px', - ); + + await expect(input).not.toHaveCSS(...FOCUS_BORDER_CSS); + await expect(input).not.toHaveCSS(...FOCUS_OUTLINE_CSS); + await component.click(); + await expect(component.getByRole('textbox')).toBeFocused(); - await expect(page.getByTestId(TEXT_INPUT_TEST_ID)).not.toHaveCSS( - 'box-shadow', - 'rgb(255, 255, 255) 0px 0px 0px 2px, rgb(65, 150, 251) 0px 0px 0px 6px', - ); + + await expect(input).toHaveCSS(...FOCUS_BORDER_CSS); + await expect(input).not.toHaveCSS(...FOCUS_OUTLINE_CSS); }); diff --git a/packages/components/src/components/TextInput/styles/text.module.scss b/packages/components/src/components/TextInput/styles/text.module.scss index 7e19f6fee2..57b6e784a9 100644 --- a/packages/components/src/components/TextInput/styles/text.module.scss +++ b/packages/components/src/components/TextInput/styles/text.module.scss @@ -26,7 +26,7 @@ border-color: var(--line-color-x-strong); } - &:has(:focus) { + &:has([data-show-focus-ring='false']:focus) { border-color: var(--line-color-xx-strong); } diff --git a/packages/components/src/helpers/constants.ts b/packages/components/src/helpers/constants.ts new file mode 100644 index 0000000000..c618cfa448 --- /dev/null +++ b/packages/components/src/helpers/constants.ts @@ -0,0 +1,8 @@ +/* (c) Copyright Frontify Ltd., all rights reserved. */ + +// ! These should be used for testing purposes only +export const FOCUS_OUTLINE_CSS: [string, string] = [ + 'box-shadow', + 'rgb(255, 255, 255) 0px 0px 0px 2px, rgb(65, 150, 251) 0px 0px 0px 6px', +]; +export const FOCUS_BORDER_CSS: [string, string] = ['border', '1px solid rgb(45, 50, 50)'];