Skip to content

Commit

Permalink
fix: backfill should not affected by mouse (#549)
Browse files Browse the repository at this point in the history
* test: test driven

* fix backfill logic
  • Loading branch information
zombieJ committed Sep 18, 2020
1 parent 5bb7de6 commit 8c32273
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 18 deletions.
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

1 comment on commit 8c32273

@vercel
Copy link

@vercel vercel bot commented on 8c32273 Sep 18, 2020

Choose a reason for hiding this comment

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

Please sign in to comment.