fix: Enlarged select filter value (#15373)

* fix: Enlarged select filter value * fix: Makes the label take the whole width * fix: Moves the required icon before the cascade icon * fix: Fixes the cascading icon overlap with big texts
2021-06-25 13:33:12 -03:00 · 2021-06-25 13:33:12 -03:00 · 819118be13
parent accee507c0
commit 819118be13
7 changed files with 116 additions and 103 deletions
--- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControl.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControl.tsx
@ -18,11 +18,24 @@
 */
 import React from 'react';
 import { styled } from '@superset-ui/core';
+import { Form, FormItem } from 'src/components/Form';
 import FilterValue from './FilterValue';
 import { FilterProps } from './types';
+import { checkIsMissingRequiredValue } from '../utils';
+
+const StyledFormItem = styled(FormItem)`
+  & label {
+    width: 100%;
+    padding-right: ${({ theme }) => theme.gridUnit * 11}px;
+  }
+`;
+
+const StyledIcon = styled.div`
+  position: absolute;
+  right: 0;
+`;

 const StyledFilterControlTitle = styled.h4`
-  width: 100%;
  font-size: ${({ theme }) => theme.typography.sizes.s}px;
  color: ${({ theme }) => theme.colors.grayscale.dark1};
  margin: 0;
@ -37,7 +50,7 @@ const StyledFilterControlTitleBox = styled.div`
  margin-bottom: ${({ theme }) => theme.gridUnit}px;
 `;

-const StyledFilterControlContainer = styled.div`
+const StyledFilterControlContainer = styled(Form)`
  width: 100%;
 `;

@ -50,21 +63,34 @@ const FilterControl: React.FC<FilterProps> = ({
  inView,
 }) => {
  const { name = '<undefined>' } = filter;
+
+  const isMissingRequiredValue = checkIsMissingRequiredValue(
+    filter,
+    filter.dataMask?.filterState,
+  );
+
  return (
-    <StyledFilterControlContainer>
-      <StyledFilterControlTitleBox>
-        <StyledFilterControlTitle data-test="filter-control-name">
-          {name}
-        </StyledFilterControlTitle>
-        <div data-test="filter-icon">{icon}</div>
-      </StyledFilterControlTitleBox>
-      <FilterValue
-        dataMaskSelected={dataMaskSelected}
-        filter={filter}
-        directPathToChild={directPathToChild}
-        onFilterSelectionChange={onFilterSelectionChange}
-        inView={inView}
-      />
+    <StyledFilterControlContainer layout="vertical">
+      <StyledFormItem
+        label={
+          <StyledFilterControlTitleBox>
+            <StyledFilterControlTitle data-test="filter-control-name">
+              {name}
+            </StyledFilterControlTitle>
+            <StyledIcon data-test="filter-icon">{icon}</StyledIcon>
+          </StyledFilterControlTitleBox>
+        }
+        required={filter?.controlValues?.enableEmptyFilter}
+        validateStatus={isMissingRequiredValue ? 'error' : undefined}
+      >
+        <FilterValue
+          dataMaskSelected={dataMaskSelected}
+          filter={filter}
+          directPathToChild={directPathToChild}
+          onFilterSelectionChange={onFilterSelectionChange}
+          inView={inView}
+        />
+      </StyledFormItem>
    </StyledFilterControlContainer>
  );
 };
--- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx
@ -19,10 +19,10 @@
 import React, { useEffect, useRef, useState } from 'react';
 import {
  QueryFormData,
-  styled,
  SuperChart,
  DataMask,
  t,
+  styled,
  Behavior,
  ChartDataResponseResult,
  JsonObject,
@ -43,13 +43,14 @@ import { ClientErrorObject } from 'src/utils/getClientErrorObject';
 import { FilterProps } from './types';
 import { getFormData } from '../../utils';
 import { useCascadingFilters } from './state';
-import { checkIsMissingRequiredValue } from '../utils';

-const FilterItem = styled.div`
-  min-height: ${({ theme }) => theme.gridUnit * 11}px;
-  padding-bottom: ${({ theme }) => theme.gridUnit * 3}px;
-  & > div > div {
-    height: auto;
+const HEIGHT = 32;
+
+// Overrides superset-ui height with min-height
+const StyledDiv = styled.div`
+  & > div {
+    height: auto !important;
+    min-height: ${HEIGHT}px;
  }
 `;

@ -195,35 +196,27 @@ const FilterValue: React.FC<FilterProps> = ({
    );
  }

-  const isMissingRequiredValue = checkIsMissingRequiredValue(
-    filter,
-    filter.dataMask?.filterState,
-  );
-
  return (
-    <FilterItem data-test="form-item-value">
+    <StyledDiv data-test="form-item-value">
      {isLoading ? (
        <Loading position="inline-centered" />
      ) : (
        <SuperChart
-          height={50}
+          height={HEIGHT}
          width="100%"
          formData={formData}
          // For charts that don't have datasource we need workaround for empty placeholder
          queriesData={hasDataSource ? state : [{ data: [{}] }]}
          chartType={filterType}
          behaviors={[Behavior.NATIVE_FILTER]}
-          filterState={{
-            ...filter.dataMask?.filterState,
-            validateMessage: isMissingRequiredValue && t('Value is required'),
-          }}
+          filterState={{ ...filter.dataMask?.filterState }}
          ownState={filter.dataMask?.ownState}
          enableNoResults={metadata?.enableNoResults}
          isRefreshing={isRefreshing}
          hooks={{ setDataMask, setFocusedFilter, unsetFocusedFilter }}
        />
      )}
-    </FilterItem>
+    </StyledDiv>
  );
 };

--- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/DefaultValue.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/DefaultValue.tsx
@ -22,7 +22,6 @@ import {
  SetDataMaskHook,
  SuperChart,
  AppSection,
-  t,
 } from '@superset-ui/core';
 import { FormInstance } from 'antd/lib/form';
 import Loading from 'src/components/Loading';
@ -57,10 +56,6 @@ const DefaultValue: FC<DefaultValueProps> = ({
      setLoading(true);
    }
  }, [hasDataset, queriesData]);
-  const value = formFilter.defaultDataMask?.filterState.value;
-  const isMissingRequiredValue =
-    (value === null || value === undefined) &&
-    formFilter?.controlValues?.enableEmptyFilter;
  return loading ? (
    <Loading position="inline-centered" />
  ) : (
@ -79,7 +74,6 @@ const DefaultValue: FC<DefaultValueProps> = ({
      enableNoResults={enableNoResults}
      filterState={{
        ...formFilter.defaultDataMask?.filterState,
-        validateMessage: isMissingRequiredValue && t('Value is required'),
      }}
    />
  );
--- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx
@ -137,14 +137,24 @@ export const StyledRowFormItem = styled(FormItem)`
 `;

 export const StyledRowSubFormItem = styled(FormItem)`
+  min-width: 50%;
+
  & .ant-form-item-label {
    padding-bottom: 0;
  }

+  .ant-form-item {
+    margin-bottom: 0;
+  }
+
  .ant-form-item-control-input-content > div > div {
    height: auto;
  }

+  .ant-form-item-extra {
+    display: none;
+  }
+
  & .ant-form-item-control-input {
    height: auto;
  }
@ -801,7 +811,9 @@ const FiltersConfigForm = (
                      if (hasValue) {
                        return Promise.resolve();
                      }
-                      return Promise.reject();
+                      return Promise.reject(
+                        new Error(t('Default value is required')),
+                      );
                    },
                  },
                ]}
@ -1010,7 +1022,7 @@ const FiltersConfigForm = (
                  onChange={checked => onSortChanged(checked || undefined)}
                  initialValue={hasSorting}
                >
-                  <StyledFormItem
+                  <StyledRowFormItem
                    name={[
                      'filters',
                      filterId,
@ -1038,7 +1050,7 @@ const FiltersConfigForm = (
                        onSortChanged(value)
                      }
                    />
-                  </StyledFormItem>
+                  </StyledRowFormItem>
                  {hasMetrics && (
                    <StyledRowSubFormItem
                      name={['filters', filterId, 'sortMetric']}
--- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/state.ts
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/state.ts
@ -54,8 +54,7 @@ export const useDefaultValue = (
  filterToEdit?: Filter,
 ) => {
  const [hasDefaultValue, setHasPartialDefaultValue] = useState(
-    !!filterToEdit?.defaultDataMask?.filterState?.value ||
-      formFilter?.controlValues?.enableEmptyFilter,
+    !!filterToEdit?.defaultDataMask?.filterState?.value,
  );
  const [isRequired, setisRequired] = useState(
    formFilter?.controlValues?.enableEmptyFilter,
--- a/superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx
+++ b/superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx
@ -26,11 +26,9 @@ import {
  GenericDataType,
  JsonObject,
  smartDateDetailedFormatter,
-  styled,
  t,
  tn,
 } from '@superset-ui/core';
-import { FormItem } from 'src/components/Form';
 import React, {
  RefObject,
  ReactElement,
@ -82,10 +80,6 @@ function reducer(
  }
 }

-const Error = styled.div`
-  color: ${({ theme }) => theme.colors.error.base};
-`;
-
 export default function PluginFilterSelect(props: PluginFilterSelectProps) {
  const {
    coltypeMap,
@ -279,57 +273,52 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) {

  return (
    <Styles height={height} width={width}>
-      <FormItem
-        validateStatus={filterState.validateMessage && 'error'}
-        extra={<Error>{filterState.validateMessage}</Error>}
+      <StyledSelect
+        allowClear
+        // @ts-ignore
+        value={filterState.value || []}
+        disabled={isDisabled}
+        showSearch={showSearch}
+        mode={multiSelect ? 'multiple' : undefined}
+        placeholder={placeholderText}
+        onSearch={searchWrapper}
+        onSelect={clearSuggestionSearch}
+        onBlur={handleBlur}
+        onDropdownVisibleChange={setIsDropdownVisible}
+        dropdownRender={(
+          originNode: ReactElement & { ref?: RefObject<HTMLElement> },
+        ) => {
+          if (isDropdownVisible && !wasDropdownVisible) {
+            originNode.ref?.current?.scrollTo({ top: 0 });
+          }
+          return originNode;
+        }}
+        onFocus={setFocusedFilter}
+        // @ts-ignore
+        onChange={handleChange}
+        ref={inputRef}
+        loading={isRefreshing}
+        maxTagCount={5}
+        menuItemSelectedIcon={<Icon iconSize="m" />}
      >
-        <StyledSelect
-          allowClear
-          // @ts-ignore
-          value={filterState.value || []}
-          disabled={isDisabled}
-          showSearch={showSearch}
-          mode={multiSelect ? 'multiple' : undefined}
-          placeholder={placeholderText}
-          onSearch={searchWrapper}
-          onSelect={clearSuggestionSearch}
-          onBlur={handleBlur}
-          onDropdownVisibleChange={setIsDropdownVisible}
-          dropdownRender={(
-            originNode: ReactElement & { ref?: RefObject<HTMLElement> },
-          ) => {
-            if (isDropdownVisible && !wasDropdownVisible) {
-              originNode.ref?.current?.scrollTo({ top: 0 });
-            }
-            return originNode;
-          }}
-          onFocus={setFocusedFilter}
-          // @ts-ignore
-          onChange={handleChange}
-          ref={inputRef}
-          loading={isRefreshing}
-          maxTagCount={5}
-          menuItemSelectedIcon={<Icon iconSize="m" />}
-        >
-          {sortedData.map(row => {
-            const [value] = groupby.map(col => row[col]);
-            return (
-              // @ts-ignore
-              <Option key={`${value}`} value={value}>
-                {labelFormatter(value, datatype)}
-              </Option>
-            );
-          })}
-          {currentSuggestionSearch &&
-            !ensureIsArray(filterState.value).some(
-              suggestion => suggestion === currentSuggestionSearch,
-            ) && (
-              <Option value={currentSuggestionSearch}>
-                {`${t('Create "%s"', currentSuggestionSearch)}`}
-              </Option>
-            )}
-        </StyledSelect>
-      </FormItem>
+        {sortedData.map(row => {
+          const [value] = groupby.map(col => row[col]);
+          return (
+            // @ts-ignore
+            <Option key={`${value}`} value={value}>
+              {labelFormatter(value, datatype)}
+            </Option>
+          );
+        })}
+        {currentSuggestionSearch &&
+          !ensureIsArray(filterState.value).some(
+            suggestion => suggestion === currentSuggestionSearch,
+          ) && (
+            <Option value={currentSuggestionSearch}>
+              {`${t('Create "%s"', currentSuggestionSearch)}`}
+            </Option>
+          )}
+      </StyledSelect>
    </Styles>
  );
 }
--- a/superset-frontend/src/filters/components/common.ts
+++ b/superset-frontend/src/filters/components/common.ts
@ -21,7 +21,7 @@ import { Select } from 'src/common/components';
 import { PluginFilterStylesProps } from './types';

 export const Styles = styled.div<PluginFilterStylesProps>`
-  height: ${({ height }) => height}px;
+  min-height: ${({ height }) => height}px;
  width: ${({ width }) => width}px;
 `;