This is an automated email from the ASF dual-hosted git repository.

rusackas pushed a commit to branch filters-modal-layout-cleanup
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 60f68a3621a109f38e99d22df4154f631de4c98f
Author: Evan Rusackas <[email protected]>
AuthorDate: Fri Dec 13 14:35:39 2024 -0700

    fix(filters modal): layout cleanup, simplification, fixes
---
 superset-frontend/src/components/Modal/Modal.tsx   |   7 --
 .../FiltersConfigModal/FilterConfigurePane.tsx     |  38 +++----
 .../FiltersConfigModal/FilterTitlePane.tsx         |  32 +++---
 .../FiltersConfigForm/FiltersConfigForm.tsx        |  17 +--
 .../FiltersConfigModal/FiltersConfigModal.tsx      | 126 +++++++++------------
 5 files changed, 89 insertions(+), 131 deletions(-)

diff --git a/superset-frontend/src/components/Modal/Modal.tsx 
b/superset-frontend/src/components/Modal/Modal.tsx
index d85a5b31a1..c48e9575df 100644
--- a/superset-frontend/src/components/Modal/Modal.tsx
+++ b/superset-frontend/src/components/Modal/Modal.tsx
@@ -163,13 +163,6 @@ export const StyledModal = 
styled(BaseModal)<StyledModalProps>`
     }
   }
 
-  // styling for Tabs component
-  // Aaron note 20-11-19: this seems to be exclusively here for the Edit 
Database modal.
-  // TODO: remove this as it is a special case.
-  .ant-tabs-top {
-    margin-top: -${({ theme }) => theme.gridUnit * 4}px;
-  }
-
   &.no-content-padding .ant-modal-body {
     padding: 0;
   }
diff --git 
a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterConfigurePane.tsx
 
b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterConfigurePane.tsx
index 48c6d0b97f..b430df94b8 100644
--- 
a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterConfigurePane.tsx
+++ 
b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterConfigurePane.tsx
@@ -35,20 +35,14 @@ interface Props {
   removedFilters: Record<string, FilterRemoval>;
 }
 
-const Container = styled.div`
-  display: flex;
-  height: 100%;
-`;
-
 const ContentHolder = styled.div`
   flex-grow: 3;
   margin-left: ${({ theme }) => theme.gridUnit * -1 - 1};
 `;
 
-const TitlesContainer = styled.div`
+const FiltersListPane = styled(FilterTitlePane)`
   min-width: 300px;
   max-width: 300px;
-  border-right: 1px solid ${({ theme }) => theme.colors.grayscale.light2};
 `;
 
 const FilterConfigurePane: FC<Props> = ({
@@ -64,23 +58,21 @@ const FilterConfigurePane: FC<Props> = ({
   filters,
   removedFilters,
 }) => (
-  <Container>
-    <TitlesContainer>
-      <FilterTitlePane
-        currentFilterId={currentFilterId}
-        filters={filters}
-        removedFilters={removedFilters}
-        erroredFilters={erroredFilters}
-        getFilterTitle={getFilterTitle}
-        onChange={onChange}
-        onAdd={(type: NativeFilterType) => onAdd(type)}
-        onRearrange={onRearrange}
-        onRemove={(id: string) => onRemove(id)}
-        restoreFilter={restoreFilter}
-      />
-    </TitlesContainer>
+  <>
+    <FiltersListPane
+      currentFilterId={currentFilterId}
+      filters={filters}
+      removedFilters={removedFilters}
+      erroredFilters={erroredFilters}
+      getFilterTitle={getFilterTitle}
+      onChange={onChange}
+      onAdd={(type: NativeFilterType) => onAdd(type)}
+      onRearrange={onRearrange}
+      onRemove={(id: string) => onRemove(id)}
+      restoreFilter={restoreFilter}
+    />
     <ContentHolder>{children}</ContentHolder>
-  </Container>
+  </>
 );
 
 export default FilterConfigurePane;
diff --git 
a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterTitlePane.tsx
 
b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterTitlePane.tsx
index 0a76a5264f..22a1820964 100644
--- 
a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterTitlePane.tsx
+++ 
b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterTitlePane.tsx
@@ -44,6 +44,7 @@ const TabsContainer = styled.div`
   flex-direction: column;
   padding: ${({ theme }) => theme.gridUnit * 3}px;
   padding-top: 2px;
+  border-right: 1px solid ${({ theme }) => theme.colors.grayscale.light2};
 `;
 
 const FilterTitlePane: FC<Props> = ({
@@ -79,25 +80,18 @@ const FilterTitlePane: FC<Props> = ({
   };
   return (
     <TabsContainer>
-      <div
-        css={{
-          height: '100%',
-          overflowY: 'auto',
-        }}
-      >
-        <FilterTitleContainer
-          ref={filtersContainerRef}
-          filters={filters}
-          currentFilterId={currentFilterId}
-          removedFilters={removedFilters}
-          getFilterTitle={getFilterTitle}
-          erroredFilters={erroredFilters}
-          onChange={onChange}
-          onRemove={onRemove}
-          onRearrange={onRearrange}
-          restoreFilter={restoreFilter}
-        />
-      </div>
+      <FilterTitleContainer
+        ref={filtersContainerRef}
+        filters={filters}
+        currentFilterId={currentFilterId}
+        removedFilters={removedFilters}
+        getFilterTitle={getFilterTitle}
+        erroredFilters={erroredFilters}
+        onChange={onChange}
+        onRemove={onRemove}
+        onRearrange={onRearrange}
+        restoreFilter={restoreFilter}
+      />
       <div
         css={{
           display: 'flex',
diff --git 
a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx
 
b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx
index e498d2cd6a..3abee822b7 100644
--- 
a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx
+++ 
b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx
@@ -241,19 +241,14 @@ const StyledCollapse = styled(Collapse)`
 `;
 
 const StyledTabs = styled(Tabs)`
+  display: flex;
+  flex-direction: column;
+  height: 100%;
   .ant-tabs-nav {
-    position: sticky;
-    top: 0;
-    background: ${({ theme }) => theme.colors.grayscale.light5};
-    z-index: 1;
-  }
-
-  .ant-tabs-nav-list {
-    padding: 0;
+    margin-bottom: 0;
   }
-
-  .ant-form-item-label {
-    padding-bottom: 0;
+  .ant-tabs-content-holder {
+    overflow-y: scroll;
   }
 `;
 
diff --git 
a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx
 
b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx
index 649ac735de..0575020b28 100644
--- 
a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx
+++ 
b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx
@@ -66,6 +66,7 @@ const MIN_WIDTH = 880;
 const StyledModalWrapper = styled(StyledModal)<{ expanded: boolean }>`
   min-width: ${MIN_WIDTH}px;
   width: ${({ expanded }) => (expanded ? '100%' : MIN_WIDTH)} !important;
+  height: 100vh;
 
   @media (max-width: ${MIN_WIDTH + MODAL_MARGIN * 2}px) {
     width: 100% !important;
@@ -74,35 +75,24 @@ const StyledModalWrapper = styled(StyledModal)<{ expanded: 
boolean }>`
 
   .ant-modal-body {
     padding: 0px;
+    flex-basis: 100vh;
   }
 
   ${({ expanded }) =>
     expanded &&
     css`
-      height: 100%;
-
       .ant-modal-body {
         flex: 1 1 auto;
       }
-      .ant-modal-content {
-        height: 100%;
-      }
     `}
 `;
 
-export const StyledModalBody = styled.div<{ expanded: boolean }>`
-  display: flex;
-  height: ${({ expanded }) => (expanded ? '100%' : '700px')};
-  flex-direction: row;
-  flex: 1;
-  .filters-list {
-    width: ${({ theme }) => theme.gridUnit * 50}px;
-    overflow: auto;
-  }
-`;
-
 export const StyledForm = styled(AntdForm)`
   width: 100%;
+  height: 100%;
+  max-height: 100%;
+  overflow: hidden;
+  display: flex;
 `;
 
 export const StyledExpandButtonWrapper = styled.div`
@@ -645,40 +635,35 @@ function FiltersConfigModal({
         const isDivider = id.startsWith(NATIVE_FILTER_DIVIDER_PREFIX);
         const isActive = currentFilterId === id;
         return (
-          <div
-            key={id}
-            style={{
-              height: '100%',
-              overflowY: 'auto',
-              display: isActive ? '' : 'none',
-            }}
-          >
-            {isDivider ? (
-              <DividerConfigForm
-                componentId={id}
-                divider={filterConfigMap[id] as Divider}
-              />
-            ) : (
-              <FiltersConfigForm
-                expanded={expanded}
-                ref={configFormRef}
-                form={form}
-                filterId={id}
-                filterToEdit={filterConfigMap[id] as Filter}
-                removedFilters={removedFilters}
-                restoreFilter={restoreFilter}
-                getAvailableFilters={getAvailableFilters}
-                key={id}
-                activeFilterPanelKeys={activeFilterPanelKey}
-                handleActiveFilterPanelChange={handleActiveFilterPanelChange}
-                isActive={isActive}
-                setErroredFilters={setErroredFilters}
-                validateDependencies={validateDependencies}
-                getDependencySuggestion={getDependencySuggestion}
-                onModifyFilter={handleModifyFilter}
-              />
-            )}
-          </div>
+          isActive && (
+            <>
+              {isDivider ? (
+                <DividerConfigForm
+                  componentId={id}
+                  divider={filterConfigMap[id] as Divider}
+                />
+              ) : (
+                <FiltersConfigForm
+                  expanded={expanded}
+                  ref={configFormRef}
+                  form={form}
+                  filterId={id}
+                  filterToEdit={filterConfigMap[id] as Filter}
+                  removedFilters={removedFilters}
+                  restoreFilter={restoreFilter}
+                  getAvailableFilters={getAvailableFilters}
+                  key={id}
+                  activeFilterPanelKeys={activeFilterPanelKey}
+                  handleActiveFilterPanelChange={handleActiveFilterPanelChange}
+                  isActive={isActive}
+                  setErroredFilters={setErroredFilters}
+                  validateDependencies={validateDependencies}
+                  getDependencySuggestion={getDependencySuggestion}
+                  onModifyFilter={handleModifyFilter}
+                />
+              )}
+            </>
+          )
         );
       }),
     [
@@ -741,28 +726,27 @@ function FiltersConfigModal({
       }
     >
       <ErrorBoundary>
-        <StyledModalBody expanded={expanded}>
-          <StyledForm
-            form={form}
-            onValuesChange={handleValuesChange}
-            layout="vertical"
+        <StyledForm
+          form={form}
+          onValuesChange={handleValuesChange}
+          layout="vertical"
+          className="test2"
+        >
+          <FilterConfigurePane
+            erroredFilters={erroredFilters}
+            onRemove={handleRemoveItem}
+            onAdd={addFilter}
+            onChange={handleTabChange}
+            getFilterTitle={getFilterTitle}
+            currentFilterId={currentFilterId}
+            removedFilters={removedFilters}
+            restoreFilter={restoreFilter}
+            onRearrange={handleRearrange}
+            filters={orderedFilters}
           >
-            <FilterConfigurePane
-              erroredFilters={erroredFilters}
-              onRemove={handleRemoveItem}
-              onAdd={addFilter}
-              onChange={handleTabChange}
-              getFilterTitle={getFilterTitle}
-              currentFilterId={currentFilterId}
-              removedFilters={removedFilters}
-              restoreFilter={restoreFilter}
-              onRearrange={handleRearrange}
-              filters={orderedFilters}
-            >
-              {formList}
-            </FilterConfigurePane>
-          </StyledForm>
-        </StyledModalBody>
+            {formList}
+          </FilterConfigurePane>
+        </StyledForm>
       </ErrorBoundary>
     </StyledModalWrapper>
   );

Reply via email to