villebro commented on a change in pull request #14933:
URL: https://github.com/apache/superset/pull/14933#discussion_r643855899
##########
File path:
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControls.tsx
##########
@@ -54,21 +73,86 @@ const FilterControls: FC<FilterControlsProps> = ({
return buildCascadeFiltersTree(filtersWithValue);
}, [filterValues, dataMaskSelected]);
+ let filtersInScope: CascadeFilter[] = [];
+ const filtersOutOfScope: CascadeFilter[] = [];
+ const dashboardHasTabs = Object.values(dashboardLayout).some(
+ element => element.type === TAB_TYPE,
+ );
+ const showCollapsePanel = dashboardHasTabs && cascadeFilters.length > 0;
+ if (!lastFocusedTabId || !dashboardHasTabs) {
+ filtersInScope = cascadeFilters;
+ } else {
+ cascadeFilters.forEach((filter, index) => {
+ if (cascadeFilters[index].tabsInScope?.includes(lastFocusedTabId)) {
+ filtersInScope.push(filter);
+ } else {
+ filtersOutOfScope.push(filter);
+ }
+ });
+ }
+
return (
<Wrapper>
- {cascadeFilters.map(filter => (
- <CascadePopover
- data-test="cascade-filters-control"
- key={filter.id}
- visible={visiblePopoverId === filter.id}
- onVisibleChange={visible =>
- setVisiblePopoverId(visible ? filter.id : null)
- }
- filter={filter}
- onFilterSelectionChange={onFilterSelectionChange}
- directPathToChild={directPathToChild}
- />
+ {portalNodes.map((node, index) => (
+ <portals.InPortal node={node}>
+ <CascadePopover
+ data-test="cascade-filters-control"
+ key={cascadeFilters[index].id}
+ visible={visiblePopoverId === cascadeFilters[index].id}
+ onVisibleChange={visible =>
+ setVisiblePopoverId(visible ? cascadeFilters[index].id : null)
+ }
+ filter={cascadeFilters[index]}
+ onFilterSelectionChange={onFilterSelectionChange}
+ directPathToChild={directPathToChild}
+ />
+ </portals.InPortal>
))}
+ {filtersInScope.map(filter => {
+ const index = cascadeFilters.findIndex(f => f.id === filter.id);
+ return <portals.OutPortal node={portalNodes[index]} />;
+ })}
+ {showCollapsePanel && (
+ <Collapse
+ ghost
+ bordered
+ expandIconPosition="right"
+ collapsible={filtersOutOfScope.length === 0 ? 'disabled' : undefined}
+ css={theme => css`
+ &.ant-collapse {
+ margin-top: ${filtersInScope.length > 0
+ ? theme.gridUnit * 6
+ : 0}px;
+ & > .ant-collapse-item {
+ & > .ant-collapse-header {
+ padding-left: 0;
+ padding-bottom: ${theme.gridUnit * 2}px;
+
+ & > .ant-collapse-arrow {
+ right: ${theme.gridUnit}px;
+ }
+ }
+
+ & .ant-collapse-content-box {
+ padding: ${theme.gridUnit * 4}px 0 0;
+ }
+ }
+ }
+ `}
+ >
+ <Collapse.Panel
+ header={`${t('Filters out of scope')} (${
+ filtersOutOfScope.length
+ })`}
+ key="1"
+ >
+ {filtersOutOfScope.map(filter => {
+ const index = cascadeFilters.findIndex(f => f.id === filter.id);
+ return <portals.OutPortal node={portalNodes[index]} />;
+ })}
+ </Collapse.Panel>
+ </Collapse>
Review comment:
I think you misunderstood what I was getting at. What I meant was that
as long as the "Out of scope" collapsible is collapsed, the requests for those
filters would not go out. So in this case:
the request for that last filter would not yet have gone out, but once it's
expanded
The charts would render for the first time, triggering the requests. If the
user moved to one of the tabs that are in scope for that filter, it would also
trigger that query, but of course only once (the next time it goes out and in
scope, it would already have been loaded hence no need to retrigger the query).
But yes, this was more a thought for future improvement, no need to delay
this PR with this now.
--
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
