ktmud commented on a change in pull request #18856:
URL: https://github.com/apache/superset/pull/18856#discussion_r820835806
##########
File path: superset-frontend/src/components/Select/Select.tsx
##########
@@ -321,81 +330,41 @@ const Select = (
: allowNewOptions
? 'tags'
: 'multiple';
- const allowFetch = !fetchOnlyOnSearch || searchedValue;
-
- // TODO: Don't assume that isAsync is always labelInValue
- const handleTopOptions = useCallback(
- (selectedValue: AntdSelectValue | undefined) => {
- // bringing selected options to the top of the list
- if (selectedValue !== undefined && selectedValue !== null) {
- const isLabeledValue = isAsync || labelInValue;
- const topOptions: OptionsType = [];
- const otherOptions: OptionsType = [];
-
- selectOptions.forEach(opt => {
- let found = false;
- if (Array.isArray(selectedValue)) {
- if (isLabeledValue) {
- found =
- (selectedValue as AntdLabeledValue[]).find(
- element => element.value === opt.value,
- ) !== undefined;
- } else {
- found = selectedValue.includes(opt.value);
- }
- } else {
- found = isLabeledValue
- ? (selectedValue as AntdLabeledValue).value === opt.value
- : selectedValue === opt.value;
- }
-
- if (found) {
- topOptions.push(opt);
- } else {
- otherOptions.push(opt);
- }
- });
-
- // fallback for custom options in tags mode as they
- // do not appear in the selectOptions state
- if (!isSingleMode && Array.isArray(selectedValue)) {
- selectedValue.forEach((val: string | number | AntdLabeledValue) => {
- if (
- !topOptions.find(
- tOpt =>
- tOpt.value ===
- (isLabeledValue ? (val as AntdLabeledValue)?.value : val),
- )
- ) {
- if (isLabeledValue) {
- const labelValue = val as AntdLabeledValue;
- topOptions.push({
- label: labelValue.label,
- value: labelValue.value,
- });
- } else {
- const value = val as string | number;
- topOptions.push({ label: String(value), value });
- }
- }
- });
- }
- const sortedOptions = [
- ...topOptions.sort(sortComparator),
- ...otherOptions.sort(sortComparator),
- ];
- if (!isEqual(sortedOptions, selectOptions)) {
- setSelectOptions(sortedOptions);
- }
- } else {
- const sortedOptions = [...selectOptions].sort(sortComparator);
- if (!isEqual(sortedOptions, selectOptions)) {
- setSelectOptions(sortedOptions);
- }
- }
- },
- [isAsync, isSingleMode, labelInValue, selectOptions, sortComparator],
+ const allowFetch = !fetchOnlyOnSearch || inputValue;
+
+ const sortSelectedFirst = useCallback(
Review comment:
I'm not even sure why we added this behavior in the first place. Neither
antd Select or react-select or the native select do this, actually making this
behavior inconsistent with a select component users see in other places.
If this is about making multi-select more usable, then multi select should
have used tags mode anyway.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]