From 75f927b9ebe4174d9af37ac4d4ce1a043cee77f1 Mon Sep 17 00:00:00 2001
From: "Michael S. Molina" <70410625+michael-s-molina@users.noreply.github.com>
Date: Mon, 11 Jan 2021 21:47:10 -0300
Subject: [PATCH] Fixes control panel fields styling (#12236) (#12326)

---
 .../integration/explore/advanced.test.ts      | 17 ++++----
 .../explore/components/SelectControl_spec.jsx | 12 +++---
 .../src/components/Label/index.tsx            |  2 +-
 .../Select/SupersetStyledSelect.tsx           |  1 +
 .../src/components/Select/styles.tsx          | 42 ++++++++++++-------
 .../explore/components/DatasourcePanel.tsx    |  9 ++--
 .../components/ExploreViewContainer.jsx       |  4 +-
 .../src/explore/components/OptionControls.tsx |  4 +-
 .../components/controls/SelectControl.jsx     | 20 +++++----
 .../components/controls/TextControl.tsx       |  2 +-
 .../stylesheets/less/cosmo/bootswatch.less    |  9 ++++
 .../stylesheets/less/cosmo/variables.less     |  4 +-
 .../stylesheets/less/variables.less           |  2 +-
 13 files changed, 76 insertions(+), 52 deletions(-)

diff --git a/superset-frontend/cypress-base/cypress/integration/explore/advanced.test.ts b/superset-frontend/cypress-base/cypress/integration/explore/advanced.test.ts
index c0ffbada25acb..c27f054c2a8e1 100644
--- a/superset-frontend/cypress-base/cypress/integration/explore/advanced.test.ts
+++ b/superset-frontend/cypress-base/cypress/integration/explore/advanced.test.ts
@@ -35,13 +35,9 @@ describe('Advanced analytics', () => {
       .find('input[type=text]')
       .type('28 days{enter}');
 
-    cy.get('[data-test=time_compare]').find('.Select__control').click();
     cy.get('[data-test=time_compare]')
       .find('input[type=text]')
-      .type('364 days{enter}');
-    cy.get('[data-test=time_compare]')
-      .find('.Select__multi-value__label')
-      .contains('364 days');
+      .type('1 year{enter}');
 
     cy.get('button[data-test="run-query-button"]').click();
     cy.wait('@postJson');
@@ -51,10 +47,13 @@ describe('Advanced analytics', () => {
       chartSelector: 'svg',
     });
 
-    cy.get('[data-test=time_compare]').within(() => {
-      cy.get('.Select__multi-value__label').contains('364 days');
-      cy.get('.Select__multi-value__label').contains('28 days');
-    });
+    cy.get('.panel-title').contains('Advanced Analytics').click();
+    cy.get('[data-test=time_compare]')
+      .find('.Select__multi-value__label')
+      .contains('28 days');
+    cy.get('[data-test=time_compare]')
+      .find('.Select__multi-value__label')
+      .contains('1 year');
   });
 });
 
diff --git a/superset-frontend/spec/javascripts/explore/components/SelectControl_spec.jsx b/superset-frontend/spec/javascripts/explore/components/SelectControl_spec.jsx
index bfb16aab262b7..35a0fccab84b2 100644
--- a/superset-frontend/spec/javascripts/explore/components/SelectControl_spec.jsx
+++ b/superset-frontend/spec/javascripts/explore/components/SelectControl_spec.jsx
@@ -112,7 +112,7 @@ describe('SelectControl', () => {
               placeholder="add something"
             />,
           );
-          expect(withMulti.html()).not.toContain('placeholder=');
+          expect(withMulti.html()).not.toContain('option(s');
         });
       });
       describe('withSingleChoice', () => {
@@ -125,7 +125,7 @@ describe('SelectControl', () => {
               placeholder="add something"
             />,
           );
-          expect(singleChoice.html()).not.toContain('placeholder=');
+          expect(singleChoice.html()).not.toContain('option(s');
         });
       });
       describe('default placeholder', () => {
@@ -133,7 +133,7 @@ describe('SelectControl', () => {
           const defaultPlaceholder = mount(
             <SelectControl {...defaultProps} choices={[]} multi />,
           );
-          expect(defaultPlaceholder.html()).not.toContain('placeholder=');
+          expect(defaultPlaceholder.html()).not.toContain('option(s');
         });
       });
       describe('all choices selected', () => {
@@ -145,12 +145,12 @@ describe('SelectControl', () => {
               value={['today', '1 year ago']}
             />,
           );
-          expect(allChoicesSelected.html()).toContain('placeholder=""');
+          expect(allChoicesSelected.html()).not.toContain('option(s');
         });
       });
     });
     describe('when select is multi', () => {
-      it('renders the placeholder when a selection has been made', () => {
+      it('does not render the placeholder when a selection has been made', () => {
         wrapper = mount(
           <SelectControl
             {...defaultProps}
@@ -159,7 +159,7 @@ describe('SelectControl', () => {
             placeholder="add something"
           />,
         );
-        expect(wrapper.html()).toContain('add something');
+        expect(wrapper.html()).not.toContain('add something');
       });
       it('shows numbers of options as a placeholder by default', () => {
         wrapper = mount(<SelectControl {...defaultProps} multi />);
diff --git a/superset-frontend/src/components/Label/index.tsx b/superset-frontend/src/components/Label/index.tsx
index bed21fdd71007..18bd1d267cc6b 100644
--- a/superset-frontend/src/components/Label/index.tsx
+++ b/superset-frontend/src/components/Label/index.tsx
@@ -93,7 +93,7 @@ const SupersetLabel = styled(BootstrapLabel)`
     background-color: ${({ theme }) => theme.colors.grayscale.light3};
     color: ${({ theme }) => theme.colors.grayscale.dark1};
     border-color: ${({ theme, onClick }) =>
-      onClick ? theme.colors.grayscale.light1 : 'transparent'};
+      onClick ? theme.colors.grayscale.light2 : 'transparent'};
     &:hover {
       background-color: ${({ theme, onClick }) =>
         onClick ? theme.colors.primary.light2 : theme.colors.grayscale.light3};
diff --git a/superset-frontend/src/components/Select/SupersetStyledSelect.tsx b/superset-frontend/src/components/Select/SupersetStyledSelect.tsx
index ec62c6d8ae0f7..fdbf5e2426fe6 100644
--- a/superset-frontend/src/components/Select/SupersetStyledSelect.tsx
+++ b/superset-frontend/src/components/Select/SupersetStyledSelect.tsx
@@ -76,6 +76,7 @@ export type SupersetStyledSelectProps<
   // additional props for easier usage or backward compatibility
   labelKey?: string;
   valueKey?: string;
+  assistiveText?: string;
   multi?: boolean;
   clearable?: boolean;
   sortable?: boolean;
diff --git a/superset-frontend/src/components/Select/styles.tsx b/superset-frontend/src/components/Select/styles.tsx
index 623d6ce018232..9358021810010 100644
--- a/superset-frontend/src/components/Select/styles.tsx
+++ b/superset-frontend/src/components/Select/styles.tsx
@@ -98,7 +98,7 @@ export const defaultTheme: (
   spacing: {
     baseUnit: 3,
     menuGutter: 0,
-    controlHeight: 28,
+    controlHeight: 34,
     lineHeight: 19,
     fontSize: 14,
     minWidth: '7.5em', // just enough to display 'No options'
@@ -160,9 +160,9 @@ export const DEFAULT_STYLES: PartialStylesConfig = {
     { isFocused, menuIsOpen, theme: { borderRadius, colors } },
   ) => {
     const isPseudoFocused = isFocused && !menuIsOpen;
-    let borderColor = '#ccc';
+    let borderColor = colors.grayBorder;
     if (isPseudoFocused) {
-      borderColor = '#000';
+      borderColor = colors.grayBorderDark;
     } else if (menuIsOpen) {
       borderColor = `${colors.grayBorderDark} ${colors.grayBorder} ${colors.grayBorderLight}`;
     }
@@ -181,6 +181,7 @@ export const DEFAULT_STYLES: PartialStylesConfig = {
           box-shadow: 0 1px 0 rgba(0, 0, 0, 0.06);
         }
         flex-wrap: nowrap;
+        padding-left: 1px;
       `,
     ];
   },
@@ -312,9 +313,31 @@ const {
   DropdownIndicator,
   Option,
   Input,
+  SelectContainer,
 } = defaultComponents as Required<DeepNonNullable<SelectComponentsType>>;
 
 export const DEFAULT_COMPONENTS: SelectComponentsType = {
+  SelectContainer: ({ children, ...props }) => {
+    const {
+      selectProps: { assistiveText },
+    } = props;
+    return (
+      <div>
+        <SelectContainer {...props}>{children}</SelectContainer>
+        {assistiveText && (
+          <span
+            css={(theme: SupersetTheme) => ({
+              marginLeft: 3,
+              fontSize: theme.typography.sizes.s,
+              color: theme.colors.grayscale.light1,
+            })}
+          >
+            {assistiveText}
+          </span>
+        )}
+      </div>
+    );
+  },
   Option: ({ children, innerProps, data, ...props }) => (
     <ClassNames>
       {({ css }) => (
@@ -344,22 +367,13 @@ export const DEFAULT_COMPONENTS: SelectComponentsType = {
     </DropdownIndicator>
   ),
   Input: (props: InputProps) => {
-    const {
-      selectProps: { isMulti, value, placeholder },
-      getStyles,
-    } = props;
-    const isMultiWithValue = isMulti && Array.isArray(value) && !!value.length;
+    const { getStyles } = props;
     return (
       <Input
         {...props}
-        placeholder={isMultiWithValue ? placeholder : undefined}
         css={getStyles('input', props)}
         autoComplete="chrome-off"
-        inputStyle={
-          isMultiWithValue
-            ? { ...INPUT_TAG_BASE_STYLES, width: '100%' }
-            : INPUT_TAG_BASE_STYLES
-        }
+        inputStyle={INPUT_TAG_BASE_STYLES}
       />
     );
   },
diff --git a/superset-frontend/src/explore/components/DatasourcePanel.tsx b/superset-frontend/src/explore/components/DatasourcePanel.tsx
index 36eb07836952d..b185ea1a335f3 100644
--- a/superset-frontend/src/explore/components/DatasourcePanel.tsx
+++ b/superset-frontend/src/explore/components/DatasourcePanel.tsx
@@ -90,9 +90,6 @@ const DatasourceContainer = styled.div`
     padding-left: ${({ theme }) => theme.gridUnit * 2}px;
     padding-bottom: 0px;
   }
-  .form-control.input-sm {
-    margin-bottom: ${({ theme }) => theme.gridUnit * 3}px;
-  }
   .ant-collapse-item {
     background-color: ${({ theme }) => theme.colors.grayscale.light4};
     .anticon.anticon-right.ant-collapse-arrow > svg {
@@ -130,8 +127,8 @@ const DatasourceContainer = styled.div`
     font-size: ${({ theme }) => theme.typography.sizes.s}px;
     color: ${({ theme }) => theme.colors.grayscale.light1};
   }
-  .form-control.input-sm {
-    width: calc(100% - ${({ theme }) => theme.gridUnit * 2}px);
+  .form-control.input-md {
+    width: calc(100% - ${({ theme }) => theme.gridUnit * 4}px);
     margin: ${({ theme }) => theme.gridUnit * 2}px auto;
   }
   .type-label {
@@ -186,7 +183,7 @@ const DataSourcePanel = ({
       <input
         type="text"
         onChange={search}
-        className="form-control input-sm"
+        className="form-control input-md"
         placeholder={t('Search Metrics & Columns')}
       />
       <div className="field-selections">
diff --git a/superset-frontend/src/explore/components/ExploreViewContainer.jsx b/superset-frontend/src/explore/components/ExploreViewContainer.jsx
index d3df42ad4f158..6599890f0f501 100644
--- a/superset-frontend/src/explore/components/ExploreViewContainer.jsx
+++ b/superset-frontend/src/explore/components/ExploreViewContainer.jsx
@@ -81,10 +81,10 @@ const Styles = styled.div`
   border-top: 1px solid ${({ theme }) => theme.colors.grayscale.light2};
   .explore-column {
     display: flex;
-    flex: 0 0 ${({ theme }) => theme.gridUnit * 80}px;
+    flex: 0 0 ${({ theme }) => theme.gridUnit * 95}px;
     flex-direction: column;
     padding: ${({ theme }) => 2 * theme.gridUnit}px 0;
-    max-width: ${({ theme }) => theme.gridUnit * 80}px;
+    max-width: ${({ theme }) => theme.gridUnit * 95}px;
     max-height: 100%;
   }
   .data-source-selection {
diff --git a/superset-frontend/src/explore/components/OptionControls.tsx b/superset-frontend/src/explore/components/OptionControls.tsx
index 8e3121d824c54..8d157c876d913 100644
--- a/superset-frontend/src/explore/components/OptionControls.tsx
+++ b/superset-frontend/src/explore/components/OptionControls.tsx
@@ -87,7 +87,7 @@ export const HeaderContainer = styled.div`
 export const LabelsContainer = styled.div`
   padding: ${({ theme }) => theme.gridUnit}px;
   border: solid 1px ${({ theme }) => theme.colors.grayscale.light2};
-  border-radius: 3px;
+  border-radius: ${({ theme }) => theme.gridUnit}px;
 `;
 
 export const AddControlLabel = styled.div`
@@ -99,7 +99,7 @@ export const AddControlLabel = styled.div`
   font-size: ${({ theme }) => theme.typography.sizes.s}px;
   color: ${({ theme }) => theme.colors.grayscale.light1};
   border: dashed 1px ${({ theme }) => theme.colors.grayscale.light2};
-  border-radius: 3px;
+  border-radius: ${({ theme }) => theme.gridUnit}px;
   cursor: pointer;
 
   :hover {
diff --git a/superset-frontend/src/explore/components/controls/SelectControl.jsx b/superset-frontend/src/explore/components/controls/SelectControl.jsx
index ccd0574f579d5..62e0826232433 100644
--- a/superset-frontend/src/explore/components/controls/SelectControl.jsx
+++ b/superset-frontend/src/explore/components/controls/SelectControl.jsx
@@ -203,13 +203,6 @@ export default class SelectControl extends React.PureComponent {
     return remainingOptions;
   }
 
-  createPlaceholder() {
-    const optionsRemaining = this.optionsRemaining();
-    const placeholder =
-      this.props.placeholder || t('%s option(s)', optionsRemaining);
-    return optionsRemaining ? placeholder : '';
-  }
-
   createMetaSelectAllOption() {
     const option = { label: 'Select All', meta: true };
     option[this.props.valueKey] = 'Select All';
@@ -235,9 +228,19 @@ export default class SelectControl extends React.PureComponent {
       valueKey,
       valueRenderer,
     } = this.props;
-    const placeholder = this.createPlaceholder();
+
+    const optionsRemaining = this.optionsRemaining();
+    const optionRemaingText = optionsRemaining
+      ? t('%s option(s)', optionsRemaining)
+      : '';
+    const placeholder = this.props.placeholder || optionRemaingText;
     const isMulti = this.props.isMulti || this.props.multi;
 
+    let assistiveText;
+    if (isMulti && optionsRemaining && Array.isArray(value) && !!value.length) {
+      assistiveText = optionRemaingText;
+    }
+
     const selectProps = {
       autoFocus,
       clearable,
@@ -257,6 +260,7 @@ export default class SelectControl extends React.PureComponent {
       optionRenderer,
       options: this.state.options,
       placeholder,
+      assistiveText,
       promptTextCreator,
       selectRef: this.getSelectRef,
       value,
diff --git a/superset-frontend/src/explore/components/controls/TextControl.tsx b/superset-frontend/src/explore/components/controls/TextControl.tsx
index 038d1698c8d86..ffc1f56d7c22a 100644
--- a/superset-frontend/src/explore/components/controls/TextControl.tsx
+++ b/superset-frontend/src/explore/components/controls/TextControl.tsx
@@ -107,7 +107,7 @@ export default class TextControl extends React.Component<
     return (
       <div>
         <ControlHeader {...this.props} />
-        <FormGroup controlId={this.state.controlId} bsSize="small">
+        <FormGroup controlId={this.state.controlId} bsSize="medium">
           <FormControl
             type="text"
             data-test="inline-name"
diff --git a/superset-frontend/stylesheets/less/cosmo/bootswatch.less b/superset-frontend/stylesheets/less/cosmo/bootswatch.less
index 576336cf77b21..b9902f689cb19 100644
--- a/superset-frontend/stylesheets/less/cosmo/bootswatch.less
+++ b/superset-frontend/stylesheets/less/cosmo/bootswatch.less
@@ -148,6 +148,10 @@ table,
 
 // Forms ======================================================================
 
+.form-control {
+  box-shadow: none;
+}
+
 .has-warning {
   .help-block,
   .control-label,
@@ -395,6 +399,11 @@ label {
   font-size: 24px;
 }
 
+.list-group-item {
+  padding-top: 5px;
+  padding-bottom: 5px;
+}
+
 a.list-group-item {
   &-success {
     &.active {
diff --git a/superset-frontend/stylesheets/less/cosmo/variables.less b/superset-frontend/stylesheets/less/cosmo/variables.less
index 8f13ac30eb2fc..6b9e3b0ca1895 100644
--- a/superset-frontend/stylesheets/less/cosmo/variables.less
+++ b/superset-frontend/stylesheets/less/cosmo/variables.less
@@ -91,7 +91,7 @@
 //
 // ## Define common padding and border radius sizes and more. Values based on 14px text and 1.428 line-height (~20px to start).
 
-@padding-base-vertical: 10px;
+@padding-base-vertical: 6.5px;
 @padding-base-horizontal: 18px;
 
 @padding-large-vertical: 18px;
@@ -152,7 +152,7 @@
 
 @btn-default-color: @bs-gray;
 @btn-default-bg: @lightest;
-@btn-default-border: @bs-gray-light;
+@btn-default-border: @gray-light;
 
 @btn-success-color: @btn-primary-color;
 @btn-success-bg: @brand-success;
diff --git a/superset-frontend/stylesheets/less/variables.less b/superset-frontend/stylesheets/less/variables.less
index 52974c3e02e62..34bfdac628158 100644
--- a/superset-frontend/stylesheets/less/variables.less
+++ b/superset-frontend/stylesheets/less/variables.less
@@ -47,7 +47,7 @@
 
 @almost-black: #263238;
 @gray-dark: #484848;
-@gray-light: #cfd8dc;
+@gray-light: #e0e0e0;
 @gray: #879399;
 @gray-bg: #f7f7f7;
 @gray-heading: #a3a3a3;