geido commented on a change in pull request #15799:
URL: https://github.com/apache/superset/pull/15799#discussion_r673230573
##########
File path: superset-frontend/src/components/Select/Select.tsx
##########
@@ -137,15 +143,16 @@ const Error = ({ error }: { error: string }) => (
const Select = ({
allowNewOptions = false,
ariaLabel,
+ fetchOnlyOnSearch,
Review comment:
What do you think of changing this prop to `fetchOnFocus`? I think it is
more accurate as you are not typing anything but only focusing the input.
##########
File path: superset-frontend/src/components/Select/Select.tsx
##########
@@ -361,13 +376,20 @@ const Select = ({
};
useEffect(() => {
- const foundOption = hasOption(searchedValue, selectOptions);
- if (isAsync && !foundOption) {
+ const allowFetch = !fetchOnlyOnSearch || searchedValue;
+ if (isAsync && loadingEnabled && allowFetch) {
Review comment:
Do we really need to check whether `loadingEnabled` is `true` here?
##########
File path: superset-frontend/src/components/Select/Select.tsx
##########
@@ -361,13 +376,20 @@ const Select = ({
};
useEffect(() => {
- const foundOption = hasOption(searchedValue, selectOptions);
Review comment:
Are you sure that we should remove this one? As far as I remember, this
should check whether the option exists to avoid unnecessary requests against
the backend.
--
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]
