Skip to content

Commit

Permalink
Merge pull request #147 from the-deep/fix/select-input-options-sorting
Browse files Browse the repository at this point in the history
Only sort/filter options when sortFunction is defined
  • Loading branch information
AdityaKhatri authored Apr 19, 2022
2 parents 14af098 + 615461e commit e7d6364
Show file tree
Hide file tree
Showing 7 changed files with 70 additions and 76 deletions.
5 changes: 2 additions & 3 deletions src/components/MultiSelectInput/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ type Def = { containerClassName?: string };
type NameType = string | number | undefined;
type OptionKey = string | number;

// FIXME: the omissions is not correct
// we need multiple omission for SearchMultiSelectInputProps
export type Props<
T extends OptionKey,
K extends NameType,
Expand All @@ -21,16 +23,13 @@ function MultiSelectInput<T extends OptionKey, K extends NameType, O extends obj
props: Props<T, K, O, P>,
) {
const {
name,
options,
totalOptionsCount, // eslint-disable-line no-unused-vars, @typescript-eslint/no-unused-vars
...otherProps
} = props;

return (
<SearchMultiSelectInput
{...otherProps}
name={name}
options={options}
sortFunction={rankedSearchOnList}
searchOptions={options}
Expand Down
51 changes: 31 additions & 20 deletions src/components/SearchMultiSelectInput/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@ import React, { useCallback, useMemo, useState } from 'react';
import {
_cs,
listToMap,
isDefined,
unique,
isDefined,
} from '@togglecorp/fujs';
import SelectInputContainer, {
Props as SelectInputContainerProps,
} from '../SelectInputContainer';
import DismissibleList from '../DismissibleList';
import { rankedSearchOnList, genericMemo } from '../../utils';
import { genericMemo } from '../../utils';
import Option from './Option';

import styles from './styles.css';
Expand Down Expand Up @@ -42,6 +42,8 @@ export type Props<
onShowDropdownChange?: (value: boolean) => void;
selectedOptionContainerClassName?: string;
selectionListShown?: boolean;
selectedOptionsAtTop?: boolean;
ellipsizeOptions?: boolean;
}, OMISSION> & (
SelectInputContainerProps<T, K, O, P,
'name'
Expand All @@ -65,9 +67,7 @@ export type Props<
| 'onFocusedKeyChange'
| 'hasValue'
>
) & {
ellipsizeOptions?: boolean;
};
);

const emptyList: unknown[] = [];

Expand Down Expand Up @@ -100,6 +100,7 @@ function SearchMultiSelectInput<
readOnly,
inputDescription,
ellipsizeOptions,
selectedOptionsAtTop = true,
...otherProps
} = props;

Expand All @@ -118,13 +119,6 @@ function SearchMultiSelectInput<
[key: string]: boolean,
}>({});

const optionsMap = useMemo(
() => (
listToMap(options, keySelector, (i) => i)
),
[options, keySelector],
);

const optionsLabelMap = useMemo(
() => (
listToMap(options, keySelector, labelSelector)
Expand All @@ -139,6 +133,13 @@ function SearchMultiSelectInput<
[value, optionsLabelMap],
);

const optionsMap = useMemo(
() => (
listToMap(options, keySelector, (i) => i)
),
[options, keySelector],
);

// NOTE: we can skip this calculation if optionsShowInitially is false
const selectedOptions = useMemo(
() => value.map((valueKey) => optionsMap[valueKey]).filter(isDefined),
Expand All @@ -147,10 +148,18 @@ function SearchMultiSelectInput<

const realOptions = useMemo(
() => {
const allOptions = unique(
[...searchOptions, ...selectedOptions],
keySelector,
);
const allOptions = searchInputValue
? searchOptions
: unique(
[...searchOptions, ...selectedOptions],
keySelector,
);

if (!selectedOptionsAtTop) {
return sortFunction
? sortFunction(allOptions, searchInputValue, labelSelector)
: allOptions;
}

const initiallySelected = allOptions
.filter((item) => selectedKeys[keySelector(item)]);
Expand All @@ -159,13 +168,13 @@ function SearchMultiSelectInput<

if (sortFunction) {
return [
...rankedSearchOnList(initiallySelected, searchInputValue, labelSelector),
...sortFunction(initiallySelected, searchInputValue, labelSelector),
...sortFunction(initiallyNotSelected, searchInputValue, labelSelector),
];
}

return [
...rankedSearchOnList(initiallySelected, searchInputValue, labelSelector),
...initiallySelected,
...initiallyNotSelected,
];
},
Expand All @@ -177,6 +186,7 @@ function SearchMultiSelectInput<
selectedKeys,
selectedOptions,
sortFunction,
selectedOptionsAtTop,
],
);

Expand Down Expand Up @@ -218,6 +228,7 @@ function SearchMultiSelectInput<

const optionRendererParams = useCallback(
(key: OptionKey, option: O) => {
// FIXME: use map
const isActive = value.findIndex((item) => item === key) !== -1;

return {
Expand All @@ -228,7 +239,7 @@ function SearchMultiSelectInput<
ellipsize: ellipsizeOptions,
};
},
[labelSelector, value, optionLabelSelector, ellipsizeOptions],
[value, labelSelector, optionLabelSelector, ellipsizeOptions],
);

// FIXME: value should not be on dependency list
Expand Down Expand Up @@ -287,9 +298,9 @@ function SearchMultiSelectInput<
onFocusedChange={setFocused}
focusedKey={focusedKey}
onFocusedKeyChange={setFocusedKey}
hasValue={isDefined(value) && value.length > 0}
persistentOptionPopup
nonClearable={false}
hasValue={isDefined(value) && value.length > 0}
disabled={disabled}
readOnly={readOnly}
inputDescription={(
Expand Down
30 changes: 20 additions & 10 deletions src/components/SearchSelectInput/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import ElementFragments from '../ElementFragments';
import SelectInputContainer, {
Props as SelectInputContainerProps,
} from '../SelectInputContainer';
import { rankedSearchOnList, genericMemo } from '../../utils';
import { genericMemo } from '../../utils';

import styles from './styles.css';

Expand Down Expand Up @@ -64,6 +64,8 @@ export type Props<
sortFunction?: (options: O[], search: string, labelSelector: (option: O) => string) => O[];
onSearchValueChange?: (value: string) => void;
onShowDropdownChange?: (value: boolean) => void;
selectedOptionsAtTop?: boolean;
ellipsizeOptions?: boolean;
}, OMISSION> & (
SelectInputContainerProps<T, K, O, P,
'name'
Expand Down Expand Up @@ -95,9 +97,7 @@ export type Props<
nonClearable?: false;
onChange: (newValue: T | undefined, name: K) => void;
}
) & {
ellipsizeOptions?: boolean;
};
);

const emptyList: unknown[] = [];

Expand Down Expand Up @@ -125,6 +125,7 @@ function SearchSelectInput<
onSearchValueChange,
onShowDropdownChange,
ellipsizeOptions,
selectedOptionsAtTop = true,
...otherProps
} = props;

Expand Down Expand Up @@ -162,10 +163,18 @@ function SearchSelectInput<

const realOptions = useMemo(
() => {
const allOptions = unique(
[...searchOptions, ...selectedOptions],
keySelector,
);
const allOptions = searchInputValue
? searchOptions
: unique(
[...searchOptions, ...selectedOptions],
keySelector,
);

if (!selectedOptionsAtTop) {
return sortFunction
? sortFunction(allOptions, searchInputValue, labelSelector)
: allOptions;
}

const initiallySelected = allOptions
.filter((item) => selectedKeys[keySelector(item)]);
Expand All @@ -174,13 +183,13 @@ function SearchSelectInput<

if (sortFunction) {
return [
...rankedSearchOnList(initiallySelected, searchInputValue, labelSelector),
...sortFunction(initiallySelected, searchInputValue, labelSelector),
...sortFunction(initiallyNotSelected, searchInputValue, labelSelector),
];
}

return [
...rankedSearchOnList(initiallySelected, searchInputValue, labelSelector),
...initiallySelected,
...initiallyNotSelected,
];
},
Expand All @@ -192,6 +201,7 @@ function SearchSelectInput<
selectedKeys,
selectedOptions,
sortFunction,
selectedOptionsAtTop,
],
);

Expand Down
47 changes: 4 additions & 43 deletions src/components/SelectInput/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ type Def = { containerClassName?: string };
type OptionKey = string | number;
type NameType = string | number | undefined;

const emptyList: unknown[] = [];

// FIXME: the omissions is not correct
// we need multiple omission for SearchMultiSelectInputProps
export type Props<
T extends OptionKey,
K extends NameType,
Expand All @@ -26,55 +26,16 @@ function SelectInput<T extends OptionKey, K extends NameType, O extends object,
props: Props<T, K, O, P>,
) {
const {
name,
options,
labelSelector,
nonClearable, // eslint-disable-line no-unused-vars, @typescript-eslint/no-unused-vars
onChange, // eslint-disable-line no-unused-vars, @typescript-eslint/no-unused-vars
totalOptionsCount, // eslint-disable-line no-unused-vars, @typescript-eslint/no-unused-vars
...otherProps
} = props;

const [searchInputValue, setSearchInputValue] = React.useState('');

const searchOptions = React.useMemo(
() => rankedSearchOnList(options ?? (emptyList as O[]), searchInputValue, labelSelector),
[options, searchInputValue, labelSelector],
);

// NOTE: this looks weird but we need to use typeguard to identify between
// different union types (for onChange and nonClearable)
// eslint-disable-next-line react/destructuring-assignment
if (props.nonClearable) {
return (
<SearchSelectInput
{...otherProps}
// eslint-disable-next-line react/destructuring-assignment
onChange={props.onChange}
// eslint-disable-next-line react/destructuring-assignment
nonClearable={props.nonClearable}
name={name}
options={options}
labelSelector={labelSelector}
onSearchValueChange={setSearchInputValue}
searchOptions={searchOptions}
// searchOptionsShownInitially
/>
);
}
return (
<SearchSelectInput
{...otherProps}
// eslint-disable-next-line react/destructuring-assignment
onChange={props.onChange}
// eslint-disable-next-line react/destructuring-assignment
nonClearable={props.nonClearable}
name={name}
options={options}
labelSelector={labelSelector}
onSearchValueChange={setSearchInputValue}
searchOptions={searchOptions}
// searchOptionsShownInitially
searchOptions={options}
sortFunction={rankedSearchOnList}
/>
);
}
Expand Down
1 change: 1 addition & 0 deletions storybook/stories/Calendar.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,5 @@ const Template: Story<CalendarProps<CalendarDateProps>> = (args) => (

export const Default = Template.bind({});
Default.args = {
initialDate: '2020-10-12',
};
6 changes: 6 additions & 0 deletions storybook/stories/SearchMultiSelectInput.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,12 @@ Default.args = {
value: ['1', '3'],
};

export const NormalSorting = Template.bind({});
NormalSorting.args = {
value: ['1', '3'],
selectedOptionsAtTop: false,
};

export const Disabled = Template.bind({});
Disabled.args = {
value: ['1', '3'],
Expand Down
6 changes: 6 additions & 0 deletions storybook/stories/SelectInput.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,12 @@ Default.args = {
ellipsizeOptions: true,
};

export const NormalSorting = Template.bind({});
NormalSorting.args = {
value: '1',
selectedOptionsAtTop: false,
};

export const Disabled = Template.bind({});
Disabled.args = {
value: '1',
Expand Down

0 comments on commit e7d6364

Please sign in to comment.