Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: backfill should not affected by mouse #549

Merged
merged 2 commits into from Sep 18, 2020
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 8 additions & 5 deletions src/OptionList.tsx
Expand Up @@ -10,6 +10,7 @@ import {
FlattenOptionData as SelectFlattenOptionData,
OptionData,
RenderNode,
OnActiveValue,
} from './interface';
import { RawValueType, FlattenOptionsType } from './interface/generator';

Expand All @@ -33,7 +34,7 @@ export interface OptionListProps<OptionsType extends object[]> {
onSelect: (value: RawValueType, option: { selected: boolean }) => void;
onToggleOpen: (open?: boolean) => void;
/** Tell Select that some value is now active to make accessibility work */
onActiveValue: (value: RawValueType, index: number) => void;
onActiveValue: OnActiveValue;
onScroll: React.UIEventHandler<HTMLDivElement>;

/** Tell Select that mouse enter the popup to force re-render */
Expand Down Expand Up @@ -115,17 +116,19 @@ const OptionList: React.RefForwardingComponent<
};

const [activeIndex, setActiveIndex] = React.useState(() => getEnabledActiveIndex(0));
const setActive = (index: number) => {
const setActive = (index: number, fromKeyboard = false) => {
setActiveIndex(index);

const info = { source: fromKeyboard ? ('keyboard' as const) : ('mouse' as const) };

// Trigger active event
const flattenItem = memoFlattenOptions[index];
if (!flattenItem) {
onActiveValue(null, -1);
onActiveValue(null, -1, info);
return;
}

onActiveValue((flattenItem.data as OptionData).value, index);
onActiveValue((flattenItem.data as OptionData).value, index, info);
};

// Auto active first item when list length or searchValue changed
Expand Down Expand Up @@ -184,7 +187,7 @@ const OptionList: React.RefForwardingComponent<
if (offset !== 0) {
const nextActiveIndex = getEnabledActiveIndex(activeIndex + offset, offset);
scrollIntoView(nextActiveIndex);
setActive(nextActiveIndex);
setActive(nextActiveIndex, true);
}

break;
Expand Down
15 changes: 7 additions & 8 deletions src/generate.tsx
Expand Up @@ -14,7 +14,7 @@ import classNames from 'classnames';
import useMergedState from 'rc-util/lib/hooks/useMergedState';
import Selector, { RefSelectorProps } from './Selector';
import SelectTrigger, { RefTriggerProps } from './SelectTrigger';
import { RenderNode, Mode, RenderDOMFunc } from './interface';
import { RenderNode, Mode, RenderDOMFunc, OnActiveValue } from './interface';
import {
GetLabeledValue,
FilterOptions,
Expand Down Expand Up @@ -187,11 +187,10 @@ export interface GenerateConfig<OptionsType extends object[]> {
/** Convert single raw value into { label, value } format. Will be called by each value */
getLabeledValue: GetLabeledValue<FlattenOptionsType<OptionsType>>;
filterOptions: FilterOptions<OptionsType>;
findValueOption:
| (// Need still support legacy ts api
(values: RawValueType[], options: FlattenOptionsType<OptionsType>) => OptionsType)
| (// New API add prevValueOptions support
(
findValueOption: // Need still support legacy ts api
| ((values: RawValueType[], options: FlattenOptionsType<OptionsType>) => OptionsType)
// New API add prevValueOptions support
| ((
values: RawValueType[],
options: FlattenOptionsType<OptionsType>,
info?: { prevValueOptions?: OptionsType[] },
Expand Down Expand Up @@ -874,10 +873,10 @@ export default function generateSelector<
const mergedDefaultActiveFirstOption =
defaultActiveFirstOption !== undefined ? defaultActiveFirstOption : mode !== 'combobox';

const onActiveValue = (active: RawValueType, index: number) => {
const onActiveValue: OnActiveValue = (active, index, { source = 'keyboard' } = {}) => {
setAccessibilityIndex(index);

if (backfill && mode === 'combobox' && active !== null) {
if (backfill && mode === 'combobox' && active !== null && source === 'keyboard') {
setActiveValue(String(active));
}
};
Expand Down
8 changes: 7 additions & 1 deletion src/interface/index.ts
@@ -1,5 +1,5 @@
import * as React from 'react';
import { Key } from './generator';
import { Key, RawValueType } from './generator';

export type RenderDOMFunc = (props: any) => HTMLElement;

Expand All @@ -8,6 +8,12 @@ export type RenderNode = React.ReactNode | ((props: any) => React.ReactNode);
export type Mode = 'multiple' | 'tags' | 'combobox';

// ======================== Option ========================
export type OnActiveValue = (
active: RawValueType,
index: number,
info?: { source?: 'keyboard' | 'mouse' },
) => void;

export interface OptionCoreData {
key?: Key;
disabled?: boolean;
Expand Down
16 changes: 16 additions & 0 deletions tests/Combobox.test.tsx
Expand Up @@ -235,6 +235,22 @@ describe('Select.Combobox', () => {
expect(handleSelect).toHaveBeenCalledWith('One', expect.objectContaining({ value: 'One' }));
});

it('mouse should not trigger', () => {
const wrapper = mount(
<Select mode="combobox" backfill open>
<Option value="One">One</Option>
<Option value="Two">Two</Option>
</Select>,
);

wrapper
.find('.rc-select-item-option')
.first()
.simulate('mouseMove');

expect(wrapper.find('input').props().value).toBeFalsy();
});

// https://github.com/ant-design/ant-design/issues/25345
it('dynamic options', () => {
const onChange = jest.fn();
Expand Down
20 changes: 16 additions & 4 deletions tests/OptionList.test.tsx
Expand Up @@ -68,7 +68,7 @@ describe('OptionList', () => {
}),
);

expect(onActiveValue).toHaveBeenCalledWith('1', expect.anything());
expect(onActiveValue).toHaveBeenCalledWith('1', expect.anything(), expect.anything());
});

it('key operation', () => {
Expand All @@ -86,13 +86,21 @@ describe('OptionList', () => {
act(() => {
listRef.current.onKeyDown({ which: KeyCode.DOWN } as any);
});
expect(onActiveValue).toHaveBeenCalledWith('2', expect.anything());
expect(onActiveValue).toHaveBeenCalledWith(
'2',
expect.anything(),
expect.objectContaining({ source: 'keyboard' }),
);

onActiveValue.mockReset();
act(() => {
listRef.current.onKeyDown({ which: KeyCode.UP } as any);
});
expect(onActiveValue).toHaveBeenCalledWith('1', expect.anything());
expect(onActiveValue).toHaveBeenCalledWith(
'1',
expect.anything(),
expect.objectContaining({ source: 'keyboard' }),
);
});

it('hover to active', () => {
Expand All @@ -109,7 +117,11 @@ describe('OptionList', () => {
.find('.rc-select-item-option')
.last()
.simulate('mouseMove');
expect(onActiveValue).toHaveBeenCalledWith('2', expect.anything());
expect(onActiveValue).toHaveBeenCalledWith(
'2',
expect.anything(),
expect.objectContaining({ source: 'mouse' }),
);

// Same item not repeat trigger
onActiveValue.mockReset();
Expand Down