[GitHub] [superset] geido commented on a change in pull request #12410: fix: Popover closes on change of dropdowns values

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

geido commented on a change in pull request #12410:
URL: https://github.com/apache/superset/pull/12410#discussion_r555865323



##########
File path: superset-frontend/src/components/Select/SupersetStyledSelect.tsx
##########
@@ -259,36 +257,43 @@ function styled<
     };
 
     const theme = useTheme();
+    const menuPortalTargetRef = React.useRef() as React.MutableRefObject<HTMLDivElement>;
 
     return (
-      <MaybeSortableSelect
-        ref={setRef}
-        className={className}
-        classNamePrefix={classNamePrefix}
-        isMulti={isMulti}
-        isClearable={isClearable}
-        options={options}
-        value={value}
-        minMenuHeight={minMenuHeight}
-        maxMenuHeight={maxMenuHeight}
-        filterOption={
-          // filterOption may be NULL
-          filterOption !== undefined
-            ? filterOption
-            : createFilter({ ignoreAccents })
-        }
-        styles={{ ...DEFAULT_STYLES, ...stylesConfig } as SelectProps['styles']}
-        // merge default theme from `react-select`, default theme for Superset,
-        // and the theme from props.
-        theme={reactSelectTheme =>
-          merge(reactSelectTheme, defaultTheme(theme), themeConfig)
-        }
-        formatOptionLabel={formatOptionLabel}
-        getOptionLabel={getOptionLabel}
-        getOptionValue={getOptionValue}
-        components={components}
-        {...restProps}
-      />
+      <div ref={menuPortalTargetRef}>

Review comment:
       Hi. @kkucharc thanks for your feedback. The `menuPortalTarget` to `document.body` will create the issue #12388 as the click event will be targeted outside of the Popover container. This is actually one of the main reasons why I am implementing this change. However, I think we can make this optional and just use it as a fallback, while managing all the other cases individually with ad hoc solutions where possible. I am looking into that.




----------------------------------------------------------------
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.

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