[GitHub] [superset] geido commented on a change in pull request #15799: chore: Adds lazy loading and fetchOnlyOnSearch to the Select component

[GitBox <gi...@apache.org>]

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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org