fix(filters modal): layout cleanup, simplification, fixes

superset-frontend/cypress-base/cypress/support/directories.ts

@@ -362,7 +362,7 @@ export const nativeFilters = {
   silentLoading: '.loading inline-centered css-101mkpk',
   filterConfigurationSections: {
     sectionHeader: '.ant-collapse-header',
-    displayedSection: 'div[style="height: 100%; overflow-y: auto;"]',
+    displayedSection: dataTestLocator('filter-configuration-tab'),
     collapseExpandButton: '.ant-collapse-arrow',
     checkedCheckbox: '.ant-checkbox-wrapper-checked',
     infoTooltip: '[aria-label="Show info tooltip"]',

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;
   }

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;

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',

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;
   }
 `;
 
@@ -826,6 +821,7 @@ const FiltersConfigForm = (
         tab={FilterTabs.configuration.name}
         key={FilterTabs.configuration.key}
         forceRender
+        data-test="filter-configuration-tab"
       >
         <StyledContainer>
           <StyledFormItem

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`
@@ -646,11 +636,9 @@ function FiltersConfigModal({
         const isActive = currentFilterId === id;
         return (
           <div
-            key={id}
             style={{
-              height: '100%',
-              overflowY: 'auto',
               display: isActive ? '' : 'none',
+              height: '100%',
             }}
           >
             {isDivider ? (
@@ -741,28 +729,26 @@ function FiltersConfigModal({
       }
     >
       <ErrorBoundary>
-        <StyledModalBody expanded={expanded}>
-          <StyledForm
-            form={form}
-            onValuesChange={handleValuesChange}
-            layout="vertical"
+        <StyledForm
+          form={form}
+          onValuesChange={handleValuesChange}
+          layout="vertical"
+        >
+          <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>
   );