[GitHub] [superset] villebro commented on a change in pull request #14933: feat(dashboard/native-filters): Hide filters out of scope of current tab

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

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:
users@infra.apache.org



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