Skip to content

Commit

Permalink
fix: null can be a value (#459)
Browse files Browse the repository at this point in the history
* init

* refactor option list select logic

* add test case

* add compile ci
  • Loading branch information
zombieJ committed Mar 3, 2020
1 parent 76e8c13 commit de9d860
Show file tree
Hide file tree
Showing 6 changed files with 59 additions and 10 deletions.
15 changes: 15 additions & 0 deletions .circleci/config.yml
Expand Up @@ -29,9 +29,24 @@ jobs:
- node_modules
key: v1-dependencies-{{ checksum "package.json" }}
- run: npm test -- --coverage
compile:
docker:
- image: circleci/node:latest
steps:
- checkout
- restore_cache:
keys:
- v1-dependencies-{{ checksum "package.json" }}
- run: npm install
- save_cache:
paths:
- node_modules
key: v1-dependencies-{{ checksum "package.json" }}
- run: npm run compile
workflows:
version: 2
build_and_test:
jobs:
- lint
- test
- compile
1 change: 1 addition & 0 deletions examples/single.tsx
Expand Up @@ -79,6 +79,7 @@ class Test extends React.Component {
console.log('Scroll:', args);
}}
>
<Option value={null}>不选择</Option>
<Option value="01" text="jack" title="jack">
<b
style={{
Expand Down
4 changes: 2 additions & 2 deletions src/OptionList.tsx
Expand Up @@ -143,7 +143,7 @@ const OptionList: React.RefForwardingComponent<

// ========================== Values ==========================
const onSelectValue = (value: RawValueType) => {
if (value !== null) {
if (value !== undefined) {
onSelect(value, { selected: !values.has(value) });
}

Expand Down Expand Up @@ -187,7 +187,7 @@ const OptionList: React.RefForwardingComponent<
if (item && !(item.data as OptionData).disabled) {
onSelectValue((item.data as OptionData).value);
} else {
onSelectValue(null);
onSelectValue(undefined);
}

if (open) {
Expand Down
3 changes: 1 addition & 2 deletions src/generate.tsx
Expand Up @@ -378,8 +378,7 @@ export default function generateSelector<
const [innerValue, setInnerValue] = React.useState<ValueType>(
value || defaultValue,
);
const baseValue =
value !== undefined && value !== null ? value : innerValue;
const baseValue = value !== undefined ? value : innerValue;

// Should reset when controlled to be uncontrolled
const prevValueRef = React.useRef(value);
Expand Down
23 changes: 18 additions & 5 deletions src/utils/commonUtil.ts
Expand Up @@ -20,14 +20,16 @@ export function toInnerValue(
value: DefaultValueType,
{ labelInValue, combobox }: { labelInValue: boolean; combobox: boolean },
): RawValueType[] {
if (value === undefined || value === null || (value === '' && combobox)) {
if (value === undefined || (value === '' && combobox)) {
return [];
}

const values = Array.isArray(value) ? value : [value];

if (labelInValue) {
return values.map(({ key, value: val }: LabelValueType) => (val !== undefined ? val : key));
return (values as LabelValueType[]).map(
({ key, value: val }: LabelValueType) => (val !== undefined ? val : key),
);
}

return values as RawValueType[];
Expand Down Expand Up @@ -56,7 +58,12 @@ export function toOuterValues<FOT extends FlattenOptionsType>(

if (labelInValue) {
values = values.map(val =>
getLabeledValue(val, { options, prevValue, labelInValue, optionLabelProp }),
getLabeledValue(val, {
options,
prevValue,
labelInValue,
optionLabelProp,
}),
);
}

Expand All @@ -70,7 +77,11 @@ export function removeLastEnabledValue<
const newValues = [...values];

let removeIndex: number;
for (removeIndex = measureValues.length - 1; removeIndex >= 0; removeIndex -= 1) {
for (
removeIndex = measureValues.length - 1;
removeIndex >= 0;
removeIndex -= 1
) {
if (!measureValues[removeIndex].disabled) {
break;
}
Expand All @@ -90,7 +101,9 @@ export function removeLastEnabledValue<
}

export const isClient =
typeof window !== 'undefined' && window.document && window.document.documentElement;
typeof window !== 'undefined' &&
window.document &&
window.document.documentElement;

/** Is client side and not jsdom */
export const isBrowserClient = process.env.NODE_ENV !== 'test' && isClient;
Expand Down
23 changes: 22 additions & 1 deletion tests/Select.test.tsx
Expand Up @@ -1450,7 +1450,7 @@ describe('Select.Basic', () => {
});

describe('reset value to undefined should reset display value', () => {
[undefined, null].forEach(value => {
[undefined].forEach(value => {
it(`to ${value}`, () => {
const wrapper = mount(<Select value="light" />);
expect(wrapper.find('.rc-select-selection-item').text()).toEqual(
Expand Down Expand Up @@ -1525,4 +1525,25 @@ describe('Select.Basic', () => {
background: 'yellow',
});
});

it('`null` is a value', () => {
const onChange = jest.fn();

const wrapper = mount(
<Select onChange={onChange}>
<Option value={1}>1</Option>
<Option value={null}>No</Option>
</Select>,
);

toggleOpen(wrapper);
selectItem(wrapper, 0);
expect(onChange).toHaveBeenCalledWith(1, expect.anything());
expect(wrapper.find('.rc-select-selection-item').text()).toEqual('1');

toggleOpen(wrapper);
selectItem(wrapper, 1);
expect(onChange).toHaveBeenCalledWith(null, expect.anything());
expect(wrapper.find('.rc-select-selection-item').text()).toEqual('No');
});
});

1 comment on commit de9d860

@vercel
Copy link

@vercel vercel bot commented on de9d860 Mar 3, 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.