ktmud commented on a change in pull request #18856:
URL: https://github.com/apache/superset/pull/18856#discussion_r820972137
##########
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:
This behavior only makes sense for multi-select or async select where
options are not available on the first page, because for single select you can
always just scroll to the selected value when menu opens. The github example is
also a multi-select. I'd consider a full scale multi-select with async searches
a totally different experience than single select with pre-defined values
therefore a little bit "inconsistency" should be warranted.
--
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]
