geido commented on code in PR #32514:
URL: https://github.com/apache/superset/pull/32514#discussion_r2032982272
##########
superset-frontend/src/components/Select/types.ts:
##########
@@ -171,9 +175,19 @@ export interface SelectProps extends BaseSelectProps {
* The options can be static, an array of options.
*/
options: SelectOptionsType;
+
+ /**
+ * It defines the threshold for select options to be virtual
+ * The threshold default is 20, meaning that if the number of options is
greater than 20,
+ * the options will be virtualized.
+ * If the virtual prop is set this will be ignored.
+ */
+ virtualThreshold?: number;
Review Comment:
As I wrote above I don't think we need a prop for this
##########
superset-frontend/src/explore/components/controls/SelectAsyncControl/index.tsx:
##########
@@ -97,10 +100,12 @@ const SelectAsyncControl = ({
endpoint: dataEndpoint,
})
.then(response => {
- const data = mutator
- ? mutator(response.json, value)
- : response.json.result;
- setOptions(data);
+ if (value) {
Review Comment:
@msyavuz not sure if this got missed but it seems we might still ignoring
`null` values with this check which should be valid.
##########
superset-frontend/src/components/Select/Select.tsx:
##########
@@ -121,9 +124,11 @@ const Select = forwardRef(
getPopupContainer,
oneLine,
maxTagCount: propsMaxTagCount,
+ virtual = undefined,
+ virtualThreshold = VIRTUAL_THRESHOLD,
Review Comment:
Any reasons to pass this as a prop? I think if someone wanted to enable the
`virtual` mode they can just do it through the `virtual` prop.
##########
superset-frontend/src/components/Select/Select.tsx:
##########
@@ -19,44 +19,46 @@
import {
forwardRef,
FocusEvent,
- ReactElement,
RefObject,
useEffect,
useMemo,
useState,
useCallback,
ClipboardEvent,
+ Ref,
+ ReactElement,
} from 'react';
import {
ensureIsArray,
+ FAST_DEBOUNCE,
formatNumber,
NumberFormats,
t,
usePrevious,
} from '@superset-ui/core';
-// eslint-disable-next-line no-restricted-imports
-import AntdSelect, { LabeledValue as AntdLabeledValue } from
'antd/lib/select'; // TODO: Remove antd
+import {
+ LabeledValue as AntdLabeledValue,
+ RefSelectProps,
+} from 'antd-v5/es/select';
import { debounce, isEqual, uniq } from 'lodash';
-import { FAST_DEBOUNCE } from 'src/constants';
import {
getValue,
hasOption,
isLabeledValue,
- renderSelectOptions,
- sortSelectedFirstHelper,
- sortComparatorWithSearchHelper,
- handleFilterOptionHelper,
- dropDownRenderHelper,
- getSuffixIcon,
SELECT_ALL_VALUE,
selectAllOption,
mapValues,
mapOptions,
- hasCustomLabels,
+ isEqual as utilsIsEqual,
+ handleFilterOptionHelper,
+ dropDownRenderHelper,
+ sortSelectedFirstHelper,
getOption,
isObject,
- isEqual as utilsIsEqual,
+ getSuffixIcon,
+ sortComparatorWithSearchHelper,
+ VIRTUAL_THRESHOLD,
Review Comment:
Should this part of constants? And maybe `SELECT_ALL_VALUE` too?
--
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]
