villebro commented on a change in pull request #14933:
URL: https://github.com/apache/superset/pull/14933#discussion_r643721227
##########
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 wonder if it would be possible to postpone rendering of the out of
scope components to the time they become visible for the first time, similar to
how charts in tabs only load up when switching tabs? Especially with dashboards
with lots of filters and tabs that only have a few filters in scope per tab,
this could have a performance boost as we would only need to render the filters
as they are needed.
I checked that it's possible to pass default props in the `InPortal` and can
then be changed/added to in the `OutPortal`, but it seems this may add quite a
bit of additional logic that might obfuscate the original code, so this
additional feature probably shouldn't be in scope for this PR..
--
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]
