Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions src/BaseSelect/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ export interface BaseSelectProps
tokenSeparators?: string[] | ((input: string) => string[]);

// >>> Icons
allowClear?: boolean | { clearIcon?: React.ReactNode };
allowClear?: boolean | { clearIcon?: React.ReactNode; label?: string };
prefix?: React.ReactNode;
/** @deprecated Please use `suffix` instead. */
suffixIcon?: RenderNode;
Expand Down Expand Up @@ -720,7 +720,11 @@ const BaseSelect = React.forwardRef<BaseSelectRef, BaseSelectProps>((props, ref)
onInternalSearch('', false, false);
};

const { allowClear: mergedAllowClear, clearIcon: clearNode } = useAllowClear(
const {
allowClear: mergedAllowClear,
clearIcon: clearNode,
label: clearLabel,
} = useAllowClear(
prefixCls,
displayValues,
allowClear,
Expand Down Expand Up @@ -762,6 +766,7 @@ const BaseSelect = React.forwardRef<BaseSelectRef, BaseSelectProps>((props, ref)
prefix={prefix}
suffix={mergedSuffixIcon}
clearIcon={clearNode}
clearLabel={clearLabel}
// Type or mode
multiple={multiple}
mode={mode}
Expand Down
18 changes: 14 additions & 4 deletions src/SelectInput/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export interface SelectInputProps extends Omit<React.HTMLAttributes<HTMLDivEleme
prefix?: React.ReactNode;
suffix?: React.ReactNode;
clearIcon?: React.ReactNode;
clearLabel?: string;
removeIcon?: RenderNode;
multiple?: boolean;
displayValues: DisplayValueType[];
Expand Down Expand Up @@ -77,6 +78,7 @@ export default React.forwardRef<SelectInputRef, SelectInputProps>(function Selec
prefix,
suffix,
clearIcon,
clearLabel,
children,

// Data
Expand Down Expand Up @@ -283,17 +285,25 @@ export default React.forwardRef<SelectInputRef, SelectInputProps>(function Selec
</Affix>
{/* Clear Icon */}
{clearIcon && (
<Affix
<button
type="button"
aria-label={clearLabel}
className={clsx(`${prefixCls}-clear`, classNames?.clear)}
style={styles?.clear}
onMouseDown={(e) => {
// Mark to tell not trigger open or focus
// Keep focus on the input and mark the native event so the root
// `onInternalMouseDown` handler does not open the dropdown.
// This must run on mousedown because the root handler fires
// before the button's onClick.
e.preventDefault();
(e.nativeEvent as any)._select_lazy = true;
onClearMouseDown?.(e);
}}
// Clearing happens on click so it works for both pointer and
// keyboard (Enter/Space) activation.
onClick={onClearMouseDown}
>
{clearIcon}
</Affix>
</button>
)}
{children}
</div>
Expand Down
4 changes: 3 additions & 1 deletion src/hooks/useAllowClear.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,13 @@ import { useMemo } from 'react';
export interface AllowClearConfig {
allowClear: boolean;
clearIcon: React.ReactNode;
label: string;
}

export const useAllowClear = (
prefixCls: string,
displayValues: DisplayValueType[],
allowClear?: boolean | { clearIcon?: React.ReactNode },
allowClear?: boolean | { clearIcon?: React.ReactNode; label?: string },
clearIcon?: React.ReactNode,
disabled: boolean = false,
mergedSearchValue?: string,
Expand All @@ -37,6 +38,7 @@ export const useAllowClear = (
return {
allowClear: mergedAllowClear,
clearIcon: mergedAllowClear ? allowClearConfig.clearIcon || clearIcon || '×' : null,
label: mergedAllowClear ? (allowClearConfig.label ?? 'Clear') : '',
};
}, [allowClearConfig, clearIcon, disabled, displayValues.length, mergedSearchValue, mode]);
};
64 changes: 60 additions & 4 deletions tests/Select.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2526,7 +2526,7 @@ describe('Select.Basic', () => {
it('should be focused when click clear button', () => {
jest.useFakeTimers();

const mouseDownPreventDefault = jest.fn();
const clickPreventDefault = jest.fn();
const { container } = render(
Comment on lines +2529 to 2530

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The clickPreventDefault mock is unused because the component does not call preventDefault() on the click event (it only calls it on the mousedown event). Removing this definition avoids unused variable warnings and simplifies the test setup.

    const { container } = render(

<Select allowClear value="1">
<Option value="1">1</Option>
Expand All @@ -2536,15 +2536,71 @@ describe('Select.Basic', () => {

expect(container.querySelector('.rc-select-clear')).toBeTruthy();

const mouseDownEvent = createEvent.mouseDown(container.querySelector('.rc-select-clear'));
mouseDownEvent.preventDefault = mouseDownPreventDefault;
fireEvent(container.querySelector('.rc-select-clear'), mouseDownEvent);
const clickEvent = createEvent.click(container.querySelector('.rc-select-clear'));
clickEvent.preventDefault = clickPreventDefault;
fireEvent(container.querySelector('.rc-select-clear'), clickEvent);
Comment on lines +2539 to +2541

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Since we don't need to mock preventDefault on the click event, we can simplify the event dispatching by directly calling fireEvent.click.

Suggested change
const clickEvent = createEvent.click(container.querySelector('.rc-select-clear'));
clickEvent.preventDefault = clickPreventDefault;
fireEvent(container.querySelector('.rc-select-clear'), clickEvent);
fireEvent.click(container.querySelector('.rc-select-clear'));

jest.runAllTimers();

expect(container.querySelector('.rc-select').className).toContain('-focused');
jest.useRealTimers();
});

it('renders clear as an accessible button with label', () => {
const { container } = render(
<Select allowClear={{ label: 'Clear all' }} value="1">
<Option value="1">1</Option>
<Option value="2">2</Option>
</Select>,
);

const clear = container.querySelector('.rc-select-clear');
expect(clear.tagName).toBe('BUTTON');
expect(clear).toHaveAttribute('type', 'button');
expect(clear).toHaveAttribute('aria-label', 'Clear all');
});

it('clicking clear does not open the dropdown', () => {
const onPopupVisibleChange = jest.fn();
const { container } = render(
<Select value="1" allowClear onPopupVisibleChange={onPopupVisibleChange}>
<Option value="1">One</Option>
<Option value="2">Two</Option>
</Select>,
);

// mousedown should be prevented (keeps focus on input) and must not open
const mouseDownEvent = createEvent.mouseDown(container.querySelector('.rc-select-clear'));
fireEvent(container.querySelector('.rc-select-clear'), mouseDownEvent);
expect(mouseDownEvent.defaultPrevented).toBe(true);

fireEvent.click(container.querySelector('.rc-select-clear'));

expectOpen(container, false);
expect(onPopupVisibleChange).not.toHaveBeenCalledWith(true);
});

it('clears value via keyboard activation', () => {
const onChange = jest.fn();
const onClear = jest.fn();
const { container } = render(
<Select defaultValue="1" allowClear onChange={onChange} onClear={onClear}>
<Option value="1">1</Option>
<Option value="2">2</Option>
</Select>,
);

const clear = container.querySelector<HTMLButtonElement>('.rc-select-clear');

clear.focus();
expect(clear).toHaveFocus();
// Enter/Space on a native button dispatch a click event
fireEvent.click(clear);

expect(onChange).toHaveBeenCalledWith(undefined, undefined);
expect(onClear).toHaveBeenCalled();
expect(container.querySelector('input').value).toBe('');
});

it('should support title', () => {
const { container: container1 } = render(<Select defaultValue="lucy" options={[]} />);
expect(container1.querySelector('.rc-select').getAttribute('title')).toBeFalsy();
Expand Down
24 changes: 16 additions & 8 deletions tests/__snapshots__/Select.test.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -131,11 +131,13 @@ exports[`Select.Basic render renders aria-attributes correctly 1`] = `
value=""
/>
</div>
<div
<button
aria-label="Clear"
class="antd-clear"
type="button"
>
×
</div>
</button>
</div>
`;

Expand All @@ -160,11 +162,13 @@ exports[`Select.Basic render renders correctly 1`] = `
value=""
/>
</div>
<div
<button
aria-label="Clear"
class="antd-clear"
type="button"
>
×
</div>
</button>
</div>
`;

Expand All @@ -191,11 +195,13 @@ exports[`Select.Basic render renders data-attributes correctly 1`] = `
value=""
/>
</div>
<div
<button
aria-label="Clear"
class="antd-clear"
type="button"
>
×
</div>
</button>
</div>
`;

Expand Down Expand Up @@ -247,11 +253,13 @@ exports[`Select.Basic render renders role prop correctly 1`] = `
value=""
/>
</div>
<div
<button
aria-label="Clear"
class="antd-clear"
type="button"
>
×
</div>
</button>
</div>
`;

Expand Down
19 changes: 17 additions & 2 deletions tests/shared/allowClearTest.tsx
Original file line number Diff line number Diff line change
@@ -1,13 +1,28 @@
import * as React from 'react';
import Select, { Option } from '../../src';
import { fireEvent, render } from '@testing-library/react';
import { createEvent, fireEvent, render } from '@testing-library/react';

export default function allowClearTest(mode: any, value: any) {
describe('allowClear', () => {
it('renders correctly', () => {
const { container } = render(<Select mode={mode} value={value} allowClear />);
expect(container.querySelector('.rc-select-clear')).toBeTruthy();
});

it('renders clear as a button', () => {
const { container } = render(<Select mode={mode} value={value} allowClear />);
const clear = container.querySelector('.rc-select-clear');
expect(clear.tagName).toBe('BUTTON');
expect(clear).toHaveAttribute('type', 'button');
});

it('prevents default on mousedown to keep focus on input', () => {
const { container } = render(<Select mode={mode} value={value} allowClear />);
const clear = container.querySelector('.rc-select-clear');
const mouseDownEvent = createEvent.mouseDown(clear);
fireEvent(clear, mouseDownEvent);
expect(mouseDownEvent.defaultPrevented).toBe(true);
});
it('clears value', () => {
const onClear = jest.fn();
const onChange = jest.fn();
Expand Down Expand Up @@ -37,7 +52,7 @@ export default function allowClearTest(mode: any, value: any) {

// enabled
rerender(renderDemo(false));
fireEvent.mouseDown(container.querySelector('.rc-select-clear'));
fireEvent.click(container.querySelector('.rc-select-clear'));
if (useArrayValue) {
expect(onChange).toHaveBeenCalledWith([], []);
} else {
Expand Down
Loading