Skip to content

Commit

Permalink
Dropdown: VR changes and overall fixes (#3967)
Browse files Browse the repository at this point in the history
  • Loading branch information
AlbertCarreras authored Jan 31, 2025
1 parent 54bfd86 commit 4febcb3
Show file tree
Hide file tree
Showing 10 changed files with 164 additions and 65 deletions.
8 changes: 8 additions & 0 deletions packages/gestalt/src/Dropdown.css
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,11 @@
.VRDropdownSection:not(:first-child) {
margin-top: var(--sema-space-200);
}

.hovered:hover {
background-color: var(--sema-color-hover-background-elevation);
}

.hovered:active {
background-color: var(--sema-color-pressed-background-elevation);
}
5 changes: 5 additions & 0 deletions packages/gestalt/src/Dropdown.jsdom.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -632,6 +632,11 @@ describe('Dropdown', () => {
fireEvent.keyDown(window.document, {
keyCode: DOWN_ARROW,
});

fireEvent.keyDown(window.document, {
keyCode: ENTER,
});

// eslint-disable-next-line testing-library/no-node-access -- Please fix the next time this file is touched!
expect(document.activeElement).toHaveAttribute('href', 'https://pinterest.com/today');

Expand Down
26 changes: 21 additions & 5 deletions packages/gestalt/src/Dropdown.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -176,14 +176,20 @@ export default function Dropdown({
const [isPopoverPositioned, setIsPopoverPositioned] = useState(false);
const deviceType = useDeviceType();
const isMobile = deviceType === 'mobile';

const isInVRExperiment = useInExperiment({
webExperimentName: 'web_gestalt_visualrefresh',
mwebExperimentName: 'web_gestalt_visualrefresh',
});

const [hoveredItemIndex, setHoveredItemIndex] = useState<number | null | undefined>(
isMobile ? undefined : 0,
);

const [focusedItemIndex, setFocusedItemIndex] = useState<number | null | undefined>(
isMobile ? undefined : 0,
);

// @ts-expect-error - TS2558 - Expected 0 type arguments, but got 1.
const dropdownChildrenArray = Children.toArray<ReactNode>(children);
const allowedChildrenOptions = getChildrenOptions(dropdownChildrenArray);
Expand Down Expand Up @@ -227,7 +233,8 @@ export default function Dropdown({
if (cursorOption) {
const item = cursorOption.option;

setHoveredItemIndex(cursorIndex);
setFocusedItemIndex(cursorIndex);
setHoveredItemIndex(null);

if (direction === KEYS.ENTER && !cursorOption.disabled) {
cursorOption.onSelect?.({
Expand All @@ -241,14 +248,14 @@ export default function Dropdown({
const onKeyDown = ({ event }: { event: React.KeyboardEvent<HTMLElement> }) => {
const { keyCode } = event;
if (keyCode === UP_ARROW) {
handleKeyNavigation(event, KEYS.UP, hoveredItemIndex);
handleKeyNavigation(event, KEYS.UP, focusedItemIndex);
event.preventDefault();
} else if (keyCode === DOWN_ARROW) {
handleKeyNavigation(event, KEYS.DOWN, hoveredItemIndex);
handleKeyNavigation(event, KEYS.DOWN, focusedItemIndex);
event.preventDefault();
} else if (keyCode === ENTER) {
event.stopPropagation();
handleKeyNavigation(event, KEYS.ENTER, hoveredItemIndex);
handleKeyNavigation(event, KEYS.ENTER, focusedItemIndex);
} else if ([ESCAPE, TAB].includes(keyCode)) {
anchor?.focus();
onDismiss?.();
Expand Down Expand Up @@ -276,7 +283,9 @@ export default function Dropdown({
<DropdownContextProvider
value={{
id,
focusedItemIndex,
hoveredItemIndex,
setFocusedItemIndex,
setHoveredItemIndex,
setOptionRef,
}}
Expand Down Expand Up @@ -318,7 +327,14 @@ export default function Dropdown({
{Boolean(headerContent) && <Box padding={2}>{headerContent}</Box>}

<DropdownContextProvider
value={{ id, hoveredItemIndex, setHoveredItemIndex, setOptionRef }}
value={{
id,
hoveredItemIndex,
setHoveredItemIndex,
setFocusedItemIndex,
setOptionRef,
focusedItemIndex,
}}
>
{renderChildrenWithIndex(dropdownChildrenArray)}
</DropdownContextProvider>
Expand Down
6 changes: 5 additions & 1 deletion packages/gestalt/src/Dropdown/Context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,19 @@ import { Context, createContext } from 'react';
type DropdownContextType = {
id: string;
hoveredItemIndex: number | null | undefined;
setHoveredItemIndex: (n: number) => void;
setHoveredItemIndex: (n: number | null | undefined) => void;
setOptionRef: (arg1?: HTMLElement | null | undefined) => void;
focusedItemIndex: number | null | undefined;
setFocusedItemIndex: (n: number | null | undefined) => void;
};

const initialContextState = {
id: '',
hoveredItemIndex: -1,
setHoveredItemIndex: () => {},
setFocusedItemIndex: () => {},
setOptionRef: () => {},
focusedItemIndex: -1,
} as const;

const context: Context<DropdownContextType> =
Expand Down
3 changes: 3 additions & 0 deletions packages/gestalt/src/Dropdown/OptionItem.jsdom.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import OptionItem from './OptionItem';
describe('OptionItem', () => {
const props = {
hoveredItemIndex: null,
focusedItemIndex: null,
id: 'optionItemId',
index: 0,
option: {
Expand All @@ -12,6 +13,8 @@ describe('OptionItem', () => {
},
// @ts-expect-error - TS2344 - Type 'undefined' does not satisfy the constraint 'any[]'.
setHoveredItemIndex: jest.fn<[number], undefined>(),
// @ts-expect-error - TS2344 - Type 'undefined' does not satisfy the constraint 'any[]'.
setFocusedItemIndex: jest.fn<[number], undefined>(),
} as const;

it('Should render correctly', () => {
Expand Down
73 changes: 38 additions & 35 deletions packages/gestalt/src/Dropdown/OptionItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,13 @@ import { useRequestAnimationFrame } from '../animation/RequestAnimationFrameCont
import Badge from '../Badge';
import Box from '../Box';
import { useDeviceType } from '../contexts/DeviceTypeProvider';
import styles from '../Dropdown.css';
import Flex from '../Flex';
import focusStyles from '../Focus.css';
import getRoundingClassName from '../getRoundingClassName';
import Icon from '../Icon';
import Link from '../Link';
import styles from '../TapArea.css';
import tapAreaStyles from '../TapArea.css';
import TapAreaLink from '../TapAreaLink';
import Text from '../Text';
import { FontWeight } from '../textTypes';
import TextUI from '../TextUI';
Expand Down Expand Up @@ -43,6 +44,7 @@ type Props = {
children?: ReactNode;
dataTestId?: string;
disabled?: boolean;
focusedItemIndex: number | null | undefined;
hoveredItemIndex: number | null | undefined;
href?: string;
id: string;
Expand All @@ -56,7 +58,8 @@ type Props = {
onSelect?: (arg1: { item: OptionItemType; event: React.ChangeEvent<HTMLInputElement> }) => void;
option: OptionItemType;
selected?: OptionItemType | ReadonlyArray<OptionItemType> | null;
setHoveredItemIndex: (arg1: number) => void;
setHoveredItemIndex: (arg1: number | null | undefined) => void;
setFocusedItemIndex: (arg1: number | null | undefined) => void;
textWeight?: FontWeight;
};

Expand All @@ -68,7 +71,9 @@ const OptionItemWithForwardRef = forwardRef<HTMLElement | null | undefined, Prop
dataTestId,
disabled,
onSelect,
focusedItemIndex,
hoveredItemIndex,
setFocusedItemIndex,
href,
id,
index,
Expand All @@ -93,31 +98,25 @@ const OptionItemWithForwardRef = forwardRef<HTMLElement | null | undefined, Prop
const isSelectedItem =
matches.length > 0 || JSON.stringify(option) === JSON.stringify(selected);

const handleOnTap = (event: React.ChangeEvent<HTMLInputElement>) => {
if (!href && !children) {
event.preventDefault();
}

if (disabled) return;
onSelect?.({ event, item: option });
};

const { isFocusVisible } = useFocusVisible();

const className = classnames(getRoundingClassName(2), {
[focusStyles.hideOutline]: !isFocusVisible,
[focusStyles.accessibilityOutline]: isFocusVisible,
[focusStyles.accessibilityOutlineFocusWithin]: isFocusVisible,
[styles.fullWidth]: true,
[styles.pointer]: !disabled,
[styles.noDrop]: disabled,
});

const isInVRExperiment = useInExperiment({
webExperimentName: 'web_gestalt_visualrefresh',
mwebExperimentName: 'web_gestalt_visualrefresh',
});

const className = classnames(getRoundingClassName(2), focusStyles.hideOutlineWithin, {
[styles.hovered]: isInVRExperiment && index === hoveredItemIndex,
[focusStyles.hideOutline]: index !== focusedItemIndex || index === hoveredItemIndex,
[focusStyles.accessibilityOutlineFocus]:
!isInVRExperiment && index === focusedItemIndex && isFocusVisible,
[focusStyles.accessibilityVROutlineFocus]:
isInVRExperiment && index === focusedItemIndex && isFocusVisible,
[tapAreaStyles.fullWidth]: true,
[tapAreaStyles.pointer]: !disabled,
[tapAreaStyles.noDrop]: disabled,
});

const textColor = disabled ? 'disabled' : 'default';

const optionItemContent = (
Expand Down Expand Up @@ -148,14 +147,10 @@ const OptionItemWithForwardRef = forwardRef<HTMLElement | null | undefined, Prop
</Text>
)}
{badge && !disabled && (
<Box marginStart={2} marginTop={1}>
<Box marginStart={2} marginTop={isInVRExperiment ? undefined : 1}>
{/* Adds a pause for screen reader users between the text content */}
<Box display="visuallyHidden">{`, `}</Box>
<Badge
position={isInVRExperiment ? 'top' : undefined}
text={badge.text}
type={badge.type || 'info'}
/>
<Badge text={badge.text} type={badge.type || 'info'} />
</Box>
)}
</Fragment>
Expand Down Expand Up @@ -188,7 +183,7 @@ const OptionItemWithForwardRef = forwardRef<HTMLElement | null | undefined, Prop
size={isInVRExperiment ? 16 : 12}
/>
) : (
<Box minWidth={12} />
<Box minWidth={isInVRExperiment ? 16 : 12} />
)}
</Box>
{iconEnd && (
Expand All @@ -213,15 +208,19 @@ const OptionItemWithForwardRef = forwardRef<HTMLElement | null | undefined, Prop
return (
<div
// @ts-expect-error - TS2322 - Type 'ForwardedRef<HTMLElement | null | undefined>' is not assignable to type 'LegacyRef<HTMLDivElement> | undefined'.
ref={index === hoveredItemIndex ? ref : null}
ref={index === hoveredItemIndex || index === focusedItemIndex ? ref : null}
aria-disabled={disabled}
className={className}
data-test-id={dataTestId}
// These event.stopPropagation are important so interactive anchors don't receive the onFocus/onBlur event
id={`${id}-item-${index}`}
onBlur={(event) => event.stopPropagation()}
// @ts-expect-error - TS2322 - Type '(event: React.ChangeEvent<HTMLInputElement>) => void' is not assignable to type 'MouseEventHandler<HTMLDivElement>'.
onClick={handleOnTap}
onClick={(event: React.ChangeEvent<HTMLInputElement>) => {
if (!href && !children) event.preventDefault();
if (disabled) return;
onSelect?.({ event, item: option });
}}
// This event.stopPropagation is important so interactive anchors don't compress with the onMouseDown event
onFocus={(event) => event.stopPropagation()}
onKeyPress={(event) => {
Expand All @@ -233,6 +232,7 @@ const OptionItemWithForwardRef = forwardRef<HTMLElement | null | undefined, Prop
}}
onMouseEnter={() => {
if (!disabled) {
setFocusedItemIndex(null);
setHoveredItemIndex(index);
}
}}
Expand All @@ -241,28 +241,31 @@ const OptionItemWithForwardRef = forwardRef<HTMLElement | null | undefined, Prop
tabIndex={isMobile && !disabled ? 0 : -1}
>
<Box
color={index === hoveredItemIndex && !disabled ? 'secondary' : 'transparent'}
color={
index === hoveredItemIndex && !disabled && !isInVRExperiment
? 'secondary'
: 'transparent'
}
direction="column"
display="flex"
paddingX={isInVRExperiment ? 3 : 2}
paddingY={isInVRExperiment ? 2 : 2}
rounding={2}
>
{href && !disabled ? (
<Link
<TapAreaLink
href={href}
onClick={({ event, dangerouslyDisableOnNavigation }) =>
onTap={({ event, dangerouslyDisableOnNavigation }) =>
onClick?.({
event,
dangerouslyDisableOnNavigation,
mobileOnDismissStart: isMobile ? onExternalDismiss : () => {},
})
}
target={iconEnd === 'visit' ? 'blank' : 'self'}
underline="none"
>
{optionItemContent}
</Link>
</TapAreaLink>
) : (
optionItemContent
)}
Expand Down
11 changes: 10 additions & 1 deletion packages/gestalt/src/DropdownItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -88,19 +88,28 @@ export default function DropdownItem({
}: Props) {
return (
<DropdownContextConsumer>
{({ id, hoveredItemIndex, setHoveredItemIndex, setOptionRef }) => (
{({
id,
hoveredItemIndex,
setHoveredItemIndex,
setOptionRef,
focusedItemIndex,
setFocusedItemIndex,
}) => (
<OptionItem
key={`${option.value + _index}`}
ref={setOptionRef}
badge={badge}
dataTestId={dataTestId}
disabled={disabled}
focusedItemIndex={focusedItemIndex}
hoveredItemIndex={hoveredItemIndex}
id={id}
index={_index}
onSelect={onSelect}
option={option}
selected={selected}
setFocusedItemIndex={setFocusedItemIndex}
setHoveredItemIndex={setHoveredItemIndex}
textWeight="bold"
>
Expand Down
11 changes: 10 additions & 1 deletion packages/gestalt/src/DropdownLink.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -80,20 +80,29 @@ export default function DropdownLink({
}: Props) {
return (
<DropdownContextConsumer>
{({ id, hoveredItemIndex, setHoveredItemIndex, setOptionRef }) => (
{({
id,
hoveredItemIndex,
setHoveredItemIndex,
setOptionRef,
focusedItemIndex,
setFocusedItemIndex,
}) => (
<OptionItem
key={`${option.value + _index}`}
ref={setOptionRef}
badge={badge}
dataTestId={dataTestId}
disabled={disabled}
focusedItemIndex={focusedItemIndex}
hoveredItemIndex={hoveredItemIndex}
href={href}
iconEnd={iconEnd}
id={id}
index={_index}
onClick={onClick}
option={option}
setFocusedItemIndex={setFocusedItemIndex}
setHoveredItemIndex={setHoveredItemIndex}
textWeight="bold"
>
Expand Down
Loading

0 comments on commit 4febcb3

Please sign in to comment.