-
Notifications
You must be signed in to change notification settings - Fork 14k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: dropdown placement for cascading filters popover #17046
Conversation
Codecov Report
@@ Coverage Diff @@
## master #17046 +/- ##
==========================================
+ Coverage 76.91% 76.93% +0.02%
==========================================
Files 1038 1039 +1
Lines 55557 55575 +18
Branches 7567 7576 +9
==========================================
+ Hits 42731 42759 +28
+ Misses 12576 12566 -10
Partials 250 250
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
/testenv up FEATURE_DASHBOARD_NATIVE_FILTERS=true FEATURE_DASHBOARD_CROSS_FILTERS=true FEATURE_DASHBOARD_NATIVE_FILTERS_SET=true FEATURE_DASHBOARD_FILTERS_EXPERIMENTAL=true |
@geido Ephemeral environment spinning up at http://35.161.112.24:8080. Credentials are |
...end/src/dashboard/components/nativeFilters/FilterBar/CascadeFilters/CascadePopover/index.tsx
Outdated
Show resolved
Hide resolved
@pkdotson The Screen.Recording.2021-10-11.at.7.30.55.AM.mov |
/testenv up FEATURE_DASHBOARD_NATIVE_FILTERS=true FEATURE_DASHBOARD_CROSS_FILTERS=true FEATURE_DASHBOARD_NATIVE_FILTERS_SET=true FEATURE_DASHBOARD_FILTERS_EXPERIMENTAL=true |
@rusackas Ephemeral environment spinning up at http://54.245.53.165:8080. Credentials are |
Ephemeral environment shutdown and build artifacts deleted. |
...c/dashboard/components/nativeFilters/FilterBar/CascadeFilters/CascadeFilterControl/index.tsx
Show resolved
Hide resolved
...c/dashboard/components/nativeFilters/FilterBar/CascadeFilters/CascadeFilterControl/index.tsx
Outdated
Show resolved
Hide resolved
...end/src/dashboard/components/nativeFilters/FilterBar/CascadeFilters/CascadePopover/index.tsx
Outdated
Show resolved
Hide resolved
/testenv up FEATURE_DASHBOARD_NATIVE_FILTERS=true FEATURE_DASHBOARD_CROSS_FILTERS=true FEATURE_DASHBOARD_NATIVE_FILTERS_SET=true FEATURE_DASHBOARD_FILTERS_EXPERIMENTAL=true |
@pkdotson Ephemeral environment spinning up at http://54.191.179.117:8080. Credentials are |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks for the fix. ready to go if the code looks good. cc @graceguo-supercat
superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx
Outdated
Show resolved
Hide resolved
superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx
Outdated
Show resolved
Hide resolved
/testenv up FEATURE_DASHBOARD_NATIVE_FILTERS=true FEATURE_DASHBOARD_CROSS_FILTERS=true FEATURE_DASHBOARD_NATIVE_FILTERS_SET=true FEATURE_DASHBOARD_FILTERS_EXPERIMENTAL=true |
@pkdotson Ephemeral environment spinning up at http://54.202.234.224:8080. Credentials are |
...end/src/dashboard/components/nativeFilters/FilterBar/CascadeFilters/CascadePopover/index.tsx
Outdated
Show resolved
Hide resolved
@@ -285,6 +287,7 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) { | |||
// @ts-ignore | |||
value={filterState.value || []} | |||
disabled={isDisabled} | |||
getPopupContainer={showOverflow ? () => parentRef : undefined} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't that be parentRef.current
?
…uperset into fix-popover-dropdown
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm
Ephemeral environment shutdown and build artifacts deleted. |
SUMMARY
This pr fixes an issue where the dropdown select will fill the container of the popover causing ui confusion for the user. This fix enables the drop down to drop in respect to the select
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
before
#16521 (comment)
after
Screen.Recording.2021-10-08.at.4.37.51.PM.mov
TESTING INSTRUCTIONS
Go to dashboard and create cascading filter and ensure the the dropdown does not fill the popover container.
ADDITIONAL INFORMATION