fix: prevent focus styles duplication (#2009)

.changeset/ten-clocks-lie.md

@@ -0,0 +1,5 @@
+---
+"@frontify/fondue-components": patch
+---
+
+fix: prevent outline style duplication (input and select)

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 (
         <RadixPopover.Root open={isOpen}>
-            <RadixPopover.Anchor asChild>
+            <RadixPopover.Anchor
+                asChild
+                onMouseDown={(mouseEvent) => {
+                    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;
+                }}
+            >
                 <div
                     className={styles.root}
                     data-status={status}

packages/components/src/components/Select/__tests__/Select.ct.tsx

@@ -4,11 +4,13 @@ import { IconIcon } from '@frontify/fondue-icons';
 import { expect, test } from '@playwright/experimental-ct-react';
 import * as sinon from 'sinon';
 
+import { FOCUS_BORDER_CSS, FOCUS_OUTLINE_CSS } from '#/helpers/constants';
 import { MAX_HEIGHT_MARGIN } from '#/utilities/domUtilities';
 
 import { Select } from '../Select';
 
 const SELECT_TEST_ID = 'test-select';
+const SELECT_MENU_TEST_ID = 'fondue-select-menu';
 const GROUP_TEST_ID = 'test-group';
 const ITEM_TEST_ID1 = 'test-item1';
 const ITEM_TEST_ID2 = 'test-item2';
@@ -370,7 +372,7 @@ test('should have max height equal to available space', async ({ mount, page })
     await expect(component).toBeVisible();
     await component.click();
 
-    const dialog = page.getByTestId('fondue-select-menu');
+    const dialog = page.getByTestId(SELECT_MENU_TEST_ID);
     await expect(dialog).toBeVisible();
     await page.setViewportSize({ width: 800, height: 300 });
     const windowHeight = page.viewportSize()?.height || 0;
@@ -379,3 +381,56 @@ test('should have max height equal to available space', async ({ mount, page })
     const actualMaxHeight = await dialog.evaluate((node) => parseFloat(window.getComputedStyle(node).maxHeight));
     expect(actualMaxHeight).toBe(expectedMaxHeight);
 });
+
+test('render focus ring and no border when keyboard focused', async ({ mount, page }) => {
+    await mount(
+        <Select aria-label="test" data-test-id={SELECT_TEST_ID} placeholder={PLACEHOLDER_TEXT}>
+            <Select.Slot name="menu">
+                <Select.Item value="test1">{ITEM_TEXT1}</Select.Item>
+            </Select.Slot>
+        </Select>,
+    );
+
+    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(
+        <Select aria-label="test" data-test-id={SELECT_TEST_ID} placeholder={PLACEHOLDER_TEXT}>
+            <Select.Slot name="menu">
+                <Select.Item value="test1">{ITEM_TEXT1}</Select.Item>
+            </Select.Slot>
+        </Select>,
+    );
+    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);
+});

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);
     }
 

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(<TextInput value={TEXT_INPUT_TEXT} />);
+    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(<TextInput value={TEXT_INPUT_TEXT} />);
+    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);
 });

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);
     }
 

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)'];