From be833dce4f45919cb96a36a48aa019fc09bb8abf Mon Sep 17 00:00:00 2001
From: "Michael S. Molina" <70410625+michael-s-molina@users.noreply.github.com>
Date: Mon, 22 Jul 2024 09:02:16 -0300
Subject: [PATCH] fix: Layout of native filters modal with lengthy columns
 (#29648)

---
 .../FilterScope/FilterScope.test.tsx          |  1 +
 .../FiltersConfigForm/FiltersConfigForm.tsx   | 45 ++++++++++++++-----
 .../getControlItemsMap.test.tsx               |  1 +
 .../FiltersConfigForm/getControlItemsMap.tsx  |  4 ++
 .../FiltersConfigModal/FiltersConfigModal.tsx |  2 +
 5 files changed, 41 insertions(+), 12 deletions(-)

diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FilterScope/FilterScope.test.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FilterScope/FilterScope.test.tsx
index ce8ff0245ee1d..93f3dd6ec1c83 100644
--- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FilterScope/FilterScope.test.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FilterScope/FilterScope.test.tsx
@@ -34,6 +34,7 @@ describe('FilterScope', () => {
   const save = jest.fn();
   let form: FormInstance<NativeFiltersForm>;
   const mockedProps = {
+    expanded: false,
     filterId: 'DefaultFilterId',
     dependencies: [],
     setErroredFilters: jest.fn(),
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 154cdbf368413..63cdd0d052160 100644
--- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx
@@ -105,6 +105,8 @@ import {
 import { FILTER_SUPPORTED_TYPES, INPUT_WIDTH } from './constants';
 import DependencyList from './DependencyList';
 
+const FORM_ITEM_WIDTH = 260;
+
 const TabPane = styled(Tabs.TabPane)`
   padding: ${({ theme }) => theme.gridUnit * 4}px 0px;
 `;
@@ -136,8 +138,8 @@ const controlsOrder: ControlKey[] = [
   'inverseSelection',
 ];
 
-export const StyledFormItem = styled(FormItem)`
-  width: 49%;
+export const StyledFormItem = styled(FormItem)<{ expanded: boolean }>`
+  width: ${({ expanded }) => (expanded ? '49%' : `${FORM_ITEM_WIDTH}px`)};
   margin-bottom: ${({ theme }) => theme.gridUnit * 4}px;
 
   & .ant-form-item-label {
@@ -149,10 +151,10 @@ export const StyledFormItem = styled(FormItem)`
   }
 `;
 
-export const StyledRowFormItem = styled(FormItem)`
+export const StyledRowFormItem = styled(FormItem)<{ expanded: boolean }>`
   margin-bottom: 0;
   padding-bottom: 0;
-  min-width: 50%;
+  min-width: ${({ expanded }) => (expanded ? '50%' : `${FORM_ITEM_WIDTH}px`)};
 
   & .ant-form-item-label {
     padding-bottom: 0;
@@ -167,8 +169,8 @@ export const StyledRowFormItem = styled(FormItem)`
   }
 `;
 
-export const StyledRowSubFormItem = styled(FormItem)`
-  min-width: 50%;
+export const StyledRowSubFormItem = styled(FormItem)<{ expanded: boolean }>`
+  min-width: ${({ expanded }) => (expanded ? '50%' : `${FORM_ITEM_WIDTH}px`)};
 
   & .ant-form-item-label {
     padding-bottom: 0;
@@ -264,9 +266,9 @@ const StyledAsterisk = styled.span`
   }
 `;
 
-const FilterTypeInfo = styled.div`
-  ${({ theme }) => `
-    width: 49%;
+const FilterTypeInfo = styled.div<{ expanded: boolean }>`
+  ${({ theme, expanded }) => `
+    width: ${expanded ? '49%' : `${FORM_ITEM_WIDTH}px`};
     font-size: ${theme.typography.sizes.s}px;
     color: ${theme.colors.grayscale.light1};
     margin:
@@ -300,6 +302,7 @@ export const FilterPanels = {
 };
 
 export interface FiltersConfigFormProps {
+  expanded: boolean;
   filterId: string;
   filterToEdit?: Filter;
   removedFilters: Record<string, FilterRemoval>;
@@ -334,6 +337,7 @@ const FILTER_TYPE_NAME_MAPPING = {
  */
 const FiltersConfigForm = (
   {
+    expanded,
     filterId,
     filterToEdit,
     removedFilters,
@@ -376,7 +380,7 @@ const FiltersConfigForm = (
   const nativeFilterVizTypes = Object.entries(nativeFilterItems)
     // @ts-ignore
     .filter(([, { value }]) => value.behaviors?.includes(Behavior.NativeFilter))
-    .map(([key]) => key);
+    .map(([key]) => key as keyof typeof FILTER_SUPPORTED_TYPES);
 
   const loadedDatasets = useSelector<RootState, DatasourcesState>(
     ({ datasources }) => datasources,
@@ -411,6 +415,7 @@ const FiltersConfigForm = (
 
   const { controlItems = {}, mainControlItems = {} } = formFilter
     ? getControlItemsMap({
+        expanded,
         datasetId,
         disabled: false,
         forceUpdate,
@@ -760,6 +765,7 @@ const FiltersConfigForm = (
 
   const timeColumn = (
     <StyledRowFormItem
+      expanded={expanded}
       name={['filters', filterId, 'granularity_sqla']}
       label={
         <>
@@ -807,6 +813,7 @@ const FiltersConfigForm = (
       >
         <StyledContainer>
           <StyledFormItem
+            expanded={expanded}
             name={['filters', filterId, 'type']}
             hidden
             initialValue={NativeFilterType.NativeFilter}
@@ -814,6 +821,7 @@ const FiltersConfigForm = (
             <Input />
           </StyledFormItem>
           <StyledFormItem
+            expanded={expanded}
             name={['filters', filterId, 'name']}
             label={<StyledLabel>{t('Filter name')}</StyledLabel>}
             initialValue={filterToEdit?.name}
@@ -822,6 +830,7 @@ const FiltersConfigForm = (
             <Input {...getFiltersConfigModalTestId('name-input')} />
           </StyledFormItem>
           <StyledFormItem
+            expanded={expanded}
             name={['filters', filterId, 'filterType']}
             rules={[{ required: !isRemoved, message: t('Name is required') }]}
             initialValue={filterToEdit?.filterType || 'filter_select'}
@@ -867,7 +876,7 @@ const FiltersConfigForm = (
           </StyledFormItem>
         </StyledContainer>
         {formFilter?.filterType === 'filter_time' && (
-          <FilterTypeInfo>
+          <FilterTypeInfo expanded={expanded}>
             {t(`Dashboard time range filters apply to temporal columns defined in
           the filter section of each chart. Add temporal columns to the chart
           filters to have this dashboard filter impact those charts.`)}
@@ -877,6 +886,7 @@ const FiltersConfigForm = (
           <StyledRowContainer>
             {showDataset ? (
               <StyledFormItem
+                expanded={expanded}
                 name={['filters', filterId, 'dataset']}
                 label={<StyledLabel>{t('Dataset')}</StyledLabel>}
                 initialValue={
@@ -915,7 +925,10 @@ const FiltersConfigForm = (
                 />
               </StyledFormItem>
             ) : (
-              <StyledFormItem label={<StyledLabel>{t('Dataset')}</StyledLabel>}>
+              <StyledFormItem
+                expanded={expanded}
+                label={<StyledLabel>{t('Dataset')}</StyledLabel>}
+              >
                 <Loading position="inline-centered" />
               </StyledFormItem>
             )}
@@ -941,6 +954,7 @@ const FiltersConfigForm = (
             >
               {canDependOnOtherFilters && hasAvailableFilters && (
                 <StyledRowFormItem
+                  expanded={expanded}
                   name={['filters', filterId, 'dependencies']}
                   initialValue={dependencies}
                 >
@@ -981,6 +995,7 @@ const FiltersConfigForm = (
                     }}
                   >
                     <StyledRowSubFormItem
+                      expanded={expanded}
                       name={['filters', filterId, 'adhoc_filters']}
                       css={{ width: INPUT_WIDTH }}
                       initialValue={filterToEdit?.adhoc_filters}
@@ -1016,6 +1031,7 @@ const FiltersConfigForm = (
                     </StyledRowSubFormItem>
                     {showTimeRangePicker && (
                       <StyledRowFormItem
+                        expanded={expanded}
                         name={['filters', filterId, 'time_range']}
                         label={<StyledLabel>{t('Time range')}</StyledLabel>}
                         initialValue={
@@ -1057,6 +1073,7 @@ const FiltersConfigForm = (
                     }}
                   >
                     <StyledRowFormItem
+                      expanded={expanded}
                       name={[
                         'filters',
                         filterId,
@@ -1077,6 +1094,7 @@ const FiltersConfigForm = (
                     </StyledRowFormItem>
                     {hasMetrics && (
                       <StyledRowSubFormItem
+                        expanded={expanded}
                         name={['filters', filterId, 'sortMetric']}
                         initialValue={filterToEdit?.sortMetric}
                         label={
@@ -1126,6 +1144,7 @@ const FiltersConfigForm = (
                     }}
                   >
                     <StyledRowFormItem
+                      expanded={expanded}
                       name={[
                         'filters',
                         filterId,
@@ -1164,6 +1183,7 @@ const FiltersConfigForm = (
             key={`${filterId}-${FilterPanels.settings.key}`}
           >
             <StyledFormItem
+              expanded={expanded}
               name={['filters', filterId, 'description']}
               initialValue={filterToEdit?.description}
               label={<StyledLabel>{t('Description')}</StyledLabel>}
@@ -1194,6 +1214,7 @@ const FiltersConfigForm = (
               >
                 {!isRemoved && (
                   <StyledRowSubFormItem
+                    expanded={expanded}
                     name={['filters', filterId, 'defaultDataMask']}
                     initialValue={initialDefaultValue}
                     data-test="default-input"
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.test.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.test.tsx
index d34abf2687956..368df7088b5da 100644
--- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.test.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.test.tsx
@@ -64,6 +64,7 @@ const filterMock: Filter = {
 };
 
 const createProps: () => ControlItemsProps = () => ({
+  expanded: false,
   datasetId: 1,
   disabled: false,
   forceUpdate: jest.fn(),
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.tsx
index 937862094b049..62fd4917ec8cb 100644
--- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.tsx
@@ -44,6 +44,7 @@ import {
 import { ColumnSelect } from './ColumnSelect';
 
 export interface ControlItemsProps {
+  expanded: boolean;
   datasetId: number;
   disabled: boolean;
   forceUpdate: Function;
@@ -60,6 +61,7 @@ const CleanFormItem = styled(FormItem)`
 `;
 
 export default function getControlItemsMap({
+  expanded,
   datasetId,
   disabled,
   forceUpdate,
@@ -104,6 +106,7 @@ export default function getControlItemsMap({
             }
           />
           <StyledFormItem
+            expanded={expanded}
             // don't show the column select unless we have a dataset
             name={['filters', filterId, 'column']}
             initialValue={initColumn}
@@ -174,6 +177,7 @@ export default function getControlItemsMap({
             }
           >
             <StyledRowFormItem
+              expanded={expanded}
               key={controlItem.name}
               name={['filters', filterId, 'controlValues', controlItem.name]}
               initialValue={initialValue}
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx
index bfb93a8da1fb7..7ad2838d2681c 100644
--- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx
@@ -581,6 +581,7 @@ function FiltersConfigModal({
               />
             ) : (
               <FiltersConfigForm
+                expanded={expanded}
                 ref={configFormRef}
                 form={form}
                 filterId={id}
@@ -613,6 +614,7 @@ function FiltersConfigModal({
       validateDependencies,
       getDependencySuggestion,
       handleActiveFilterPanelChange,
+      expanded,
     ],
   );