fix: Fixes email body when sharing a chart by email (#14664)

(cherry picked from commit 32f5f365d8)

superset-frontend/spec/javascripts/explore/components/BoundsControl_spec.jsx

@@ -17,47 +17,37 @@
  * under the License.
  */
 import React from 'react';
-import sinon from 'sinon';
+import { render, screen, waitFor } from 'spec/helpers/testing-library';
-import { styledMount as mount } from 'spec/helpers/theming';
+import userEvent from '@testing-library/user-event';
 import BoundsControl from 'src/explore/components/controls/BoundsControl';
-import { Input } from 'src/common/components';
 
 const defaultProps = {
   name: 'y_axis_bounds',
   label: 'Bounds of the y axis',
-  onChange: sinon.spy(),
+  onChange: jest.fn(),
 };
 
-describe('BoundsControl', () => {
+test('renders two inputs', () => {
-  let wrapper;
+  render(<BoundsControl {...defaultProps} />);
-
+  expect(screen.getAllByRole('spinbutton')).toHaveLength(2);
-  beforeEach(() => {
+});
-    wrapper = mount(<BoundsControl {...defaultProps} />);
+
-  });
+test('receives null on non-numeric', async () => {
-
+  render(<BoundsControl {...defaultProps} />);
-  it('renders two Input', () => {
+  const minInput = screen.getAllByRole('spinbutton')[0];
-    expect(wrapper.find(Input)).toHaveLength(2);
+  userEvent.type(minInput, 'text');
-  });
+  await waitFor(() =>
-
+    expect(defaultProps.onChange).toHaveBeenCalledWith([null, null]),
-  it('errors on non-numeric', () => {
+  );
-    wrapper
+});
-      .find(Input)
+
-      .first()
+test('calls onChange with correct values', async () => {
-      .simulate('change', { target: { value: 's' } });
+  render(<BoundsControl {...defaultProps} />);
-    expect(defaultProps.onChange.calledWith([null, null])).toBe(true);
+  const minInput = screen.getAllByRole('spinbutton')[0];
-    expect(defaultProps.onChange.getCall(0).args[1][0]).toContain(
+  const maxInput = screen.getAllByRole('spinbutton')[1];
-      'value should be numeric',
+  userEvent.type(minInput, '1');
-    );
+  userEvent.type(maxInput, '2');
-  });
+  await waitFor(() =>
-  it('casts to numeric', () => {
+    expect(defaultProps.onChange).toHaveBeenLastCalledWith([1, 2]),
-    wrapper
+  );
-      .find(Input)
-      .first()
-      .simulate('change', { target: { value: '1' } });
-    wrapper
-      .find(Input)
-      .last()
-      .simulate('change', { target: { value: '5' } });
-    expect(defaultProps.onChange.calledWith([1, 5])).toBe(true);
-  });
 });

superset-frontend/src/SqlLab/components/TableElement.jsx

@@ -204,7 +204,7 @@ class TableElement extends React.PureComponent {
       >
         <Tooltip
           id="copy-to-clipboard-tooltip"
-          placement="top"
+          placement="topLeft"
           style={{ cursor: 'pointer' }}
           title={table.name}
           trigger={['hover']}

superset-frontend/src/common/components/index.tsx

@@ -18,7 +18,13 @@
  */
 import React from 'react';
 import { styled } from '@superset-ui/core';
-import { Dropdown, Menu as AntdMenu, Input as AntdInput, Skeleton } from 'antd';
+import {
+  Dropdown,
+  Menu as AntdMenu,
+  Input as AntdInput,
+  InputNumber as AntdInputNumber,
+  Skeleton,
+} from 'antd';
 import { DropDownProps } from 'antd/lib/dropdown';
 /*
   Antd is re-exported from here so we can override components with Emotion as needed.
@@ -36,7 +42,6 @@ export {
   Dropdown,
   Form,
   Empty,
-  InputNumber,
   Modal,
   Typography,
   Tree,
@@ -118,18 +123,29 @@ export const StyledNav = styled(AntdMenu)`
       color: ${({ theme }) => theme.colors.grayscale.dark1};
     }
   }
+
   &:not(.ant-menu-dark) > .ant-menu-submenu,
   &:not(.ant-menu-dark) > .ant-menu-item {
-    margin: 0px;
     &:hover {
       border-bottom: none;
     }
   }
 
+  @media (min-width: 767px) {
+    &:not(.ant-menu-dark) > .ant-menu-submenu,
+    &:not(.ant-menu-dark) > .ant-menu-item {
+      margin: 0px;
+    }
+  }
+
   & > .ant-menu-item > a {
     padding: ${({ theme }) => theme.gridUnit * 4}px;
   }
+`;
 
+export const StyledSubMenu = styled(AntdMenu.SubMenu)`
+  color: ${({ theme }) => theme.colors.grayscale.dark1};
+  border-bottom: none;
   .ant-menu-submenu-open,
   .ant-menu-submenu-active {
     background-color: ${({ theme }) => theme.colors.primary.light5};
@@ -144,12 +160,9 @@ export const StyledNav = styled(AntdMenu)`
       }
     }
   }
-`;
-
-export const StyledSubMenu = styled(AntdMenu.SubMenu)`
-  color: ${({ theme }) => theme.colors.grayscale.dark1};
-  border-bottom: none;
   .ant-menu-submenu-title {
+    position: relative;
+    top: ${({ theme }) => -theme.gridUnit - 3}px;
     &:after {
       content: '';
       position: absolute;
@@ -163,17 +176,24 @@ export const StyledSubMenu = styled(AntdMenu.SubMenu)`
       background-color: ${({ theme }) => theme.colors.primary.base};
     }
   }
+  .ant-menu-submenu-arrow {
+    top: 67%;
+  }
   & > .ant-menu-submenu-title {
     padding: 0 ${({ theme }) => theme.gridUnit * 6}px 0
       ${({ theme }) => theme.gridUnit * 3}px !important;
     svg {
       position: absolute;
-      top: ${({ theme }) => theme.gridUnit * 4}px;
+      top: ${({ theme }) => theme.gridUnit * 4 + 7}px;
       right: ${({ theme }) => theme.gridUnit}px;
       width: ${({ theme }) => theme.gridUnit * 6}px;
     }
+    & > span {
+      position: relative;
+      top: 7px;
+    }
     &:hover {
-      color: ${({ theme }) => theme.colors.grayscale.dark1};
+      color: ${({ theme }) => theme.colors.primary.base};
     }
   }
 `;
@@ -200,6 +220,11 @@ export const Input = styled(AntdInput)`
   border-radius: ${({ theme }) => theme.borderRadius}px;
 `;
 
+export const InputNumber = styled(AntdInputNumber)`
+  border: 1px solid ${({ theme }) => theme.colors.secondary.light3};
+  border-radius: ${({ theme }) => theme.borderRadius}px;
+`;
+
 export const TextArea = styled(AntdInput.TextArea)`
   border: 1px solid ${({ theme }) => theme.colors.secondary.light3};
   border-radius: ${({ theme }) => theme.borderRadius}px;

superset-frontend/src/components/Alert/Alert.stories.tsx

@@ -74,10 +74,16 @@ AlertGallery.story = {
   },
 };
 
-export const InteractiveAlert = (args: AlertProps) => <Alert {...args} />;
+export const InteractiveAlert = (args: AlertProps) => (
+  <>
+    <Alert {...args} />
+    Some content to test the `roomBelow` prop
+  </>
+);
 
 InteractiveAlert.args = {
   closable: true,
+  roomBelow: false,
   type: 'info',
   message: smallText,
   description: bigText,

superset-frontend/src/components/Alert/index.tsx

@@ -25,7 +25,9 @@ import { useTheme } from '@superset-ui/core';
 import Icon from 'src/components/Icon';
 import Icons from 'src/components/Icons';
 
-export type AlertProps = PropsWithChildren<AntdAlertProps>;
+export type AlertProps = PropsWithChildren<
+  AntdAlertProps & { roomBelow?: boolean }
+>;
 
 export default function Alert(props: AlertProps) {
   const {
@@ -33,11 +35,12 @@ export default function Alert(props: AlertProps) {
     description,
     showIcon = true,
     closable = true,
+    roomBelow = false,
     children,
   } = props;
 
   const theme = useTheme();
-  const { colors, typography } = theme;
+  const { colors, typography, gridUnit } = theme;
   const { alert, error, info, success } = colors;
 
   let baseColor = info;
@@ -60,12 +63,13 @@ export default function Alert(props: AlertProps) {
       icon={<AlertIcon aria-label={`${type} icon`} />}
       closeText={closable && <Icon name="x-small" aria-label="close icon" />}
       css={{
-        padding: '6px 10px',
+        marginBottom: roomBelow ? gridUnit * 4 : 0,
+        padding: `${gridUnit * 2}px ${gridUnit * 3}px`,
         alignItems: 'flex-start',
         border: 0,
         backgroundColor: baseColor.light2,
         '& .ant-alert-icon': {
-          marginRight: 10,
+          marginRight: gridUnit * 2,
         },
         '& .ant-alert-message': {
           color: baseColor.dark2,

superset-frontend/src/components/Label/index.tsx

@@ -101,6 +101,7 @@ export default function Label(props: LabelProps) {
         padding: '0.35em 0.8em',
         lineHeight: 1,
         color,
+        maxWidth: '100%',
         '&:hover': {
           backgroundColor: backgroundColorHover,
           borderColor: borderColorHover,

superset-frontend/src/components/ListView/Filters/Search.tsx

@@ -42,6 +42,12 @@ export default function SearchFilter({
     setValue('');
     onSubmit('');
   };
+  const handleChange = (e: React.ChangeEvent<HTMLInputElement>) => {
+    setValue(e.currentTarget.value);
+    if (e.currentTarget.value === '') {
+      onClear();
+    }
+  };
 
   return (
     <FilterContainer>
@@ -50,9 +56,7 @@ export default function SearchFilter({
         placeholder={Header}
         name={name}
         value={value}
-        onChange={e => {
+        onChange={handleChange}
-          setValue(e.currentTarget.value);
-        }}
         onSubmit={handleSubmit}
         onClear={onClear}
       />

superset-frontend/src/components/Menu/Menu.tsx

@@ -17,8 +17,9 @@
  * under the License.
  */
 import React, { useState, useEffect } from 'react';
-import { styled } from '@superset-ui/core';
+import { styled, css } from '@superset-ui/core';
 import { debounce } from 'lodash';
+import { Global } from '@emotion/react';
 import { getUrlParam } from 'src/utils/urlUtils';
 import { MainNav as DropdownMenu, MenuMode } from 'src/common/components';
 import { Link } from 'react-router-dom';
@@ -98,10 +99,14 @@ const StyledHeader = styled.header`
     height: 100%;
     line-height: inherit;
   }
-  /*.ant-menu > .ant-menu-item > a {
+  .ant-menu > .ant-menu-item > a {
     padding: ${({ theme }) => theme.gridUnit * 4}px;
-  }*/
+  }
   @media (max-width: 767px) {
+    .ant-menu-item {
+      padding: 0 ${({ theme }) => theme.gridUnit * 6}px 0
+        ${({ theme }) => theme.gridUnit * 3}px !important;
+    }
     .ant-menu > .ant-menu-item > a {
       padding: 0px;
     }
@@ -200,6 +205,16 @@ export function Menu({
   };
   return (
     <StyledHeader className="top" id="main-menu" role="navigation">
+      <Global
+        styles={css`
+          .ant-menu-submenu.ant-menu-submenu-popup.ant-menu.ant-menu-light.ant-menu-submenu-placement-bottomLeft {
+            border-radius: 0px;
+          }
+          .ant-menu-submenu.ant-menu-submenu-popup.ant-menu.ant-menu-light {
+            border-radius: 0px;
+          }
+        `}
+      />
       <Row>
         <Col lg={19} md={19} sm={24} xs={24}>
           <a className="navbar-brand" href={brand.path}>

superset-frontend/src/components/Menu/SubMenu.tsx

@@ -21,7 +21,7 @@ import { Link, useHistory } from 'react-router-dom';
 import { styled } from '@superset-ui/core';
 import cx from 'classnames';
 import { debounce } from 'lodash';
-import { Col, Row } from 'antd';
+import { Row } from 'antd';
 import { Menu, MenuMode } from 'src/common/components';
 import Button, { OnClickHandler } from 'src/components/Button';
 
@@ -42,6 +42,8 @@ const StyledHeader = styled.div`
     padding: 14px 0;
     margin-right: ${({ theme }) => theme.gridUnit * 3}px;
     float: right;
+    position: absolute;
+    right: 0;
   }
   .nav-right-collapse {
     display: flex;
@@ -111,8 +113,8 @@ const StyledHeader = styled.div`
   @media (max-width: 767px) {
     .header,
     .nav-right {
-      float: left;
+      position: relative;
-      padding-left: ${({ theme }) => theme.gridUnit * 2}px;
+      margin-left: ${({ theme }) => theme.gridUnit * 2}px;
     }
   }
 `;
@@ -150,15 +152,12 @@ export interface SubMenuProps {
    *  otherwise, a 'You should not use <Link> outside a <Router>' error will be thrown */
   usesRouter?: boolean;
   color?: string;
-  headerSize?: number;
 }
 
 const SubMenuComponent: React.FunctionComponent<SubMenuProps> = props => {
   const [showMenu, setMenu] = useState<MenuMode>('horizontal');
   const [navRightStyle, setNavRightStyle] = useState('nav-right');
-  const [navRightCol, setNavRightCol] = useState(8);
 
-  const { headerSize = 2 } = props;
   let hasHistory = true;
   // If no parent <Router> component exists, useHistory throws an error
   try {
@@ -178,14 +177,12 @@ const SubMenuComponent: React.FunctionComponent<SubMenuProps> = props => {
         props.buttons.length >= 3 &&
         window.innerWidth >= 795
       ) {
-        setNavRightCol(8);
         setNavRightStyle('nav-right');
       } else if (
         props.buttons &&
         props.buttons.length >= 3 &&
         window.innerWidth <= 795
       ) {
-        setNavRightCol(24);
         setNavRightStyle('nav-right-collapse');
       }
     }
@@ -195,69 +192,56 @@ const SubMenuComponent: React.FunctionComponent<SubMenuProps> = props => {
     return () => window.removeEventListener('resize', resize);
   }, [props.buttons]);
 
-  const offset = props.name ? headerSize : 0;
-
   return (
     <StyledHeader>
       <Row className="menu" role="navigation">
-        {props.name && (
+        {props.name && <div className="header">{props.name}</div>}
-          <Col md={offset} xs={24}>
+        <Menu mode={showMenu} style={{ backgroundColor: 'transparent' }}>
-            <div className="header">{props.name}</div>
+          {props.tabs?.map(tab => {
-          </Col>
+            if ((props.usesRouter || hasHistory) && !!tab.usesRouter) {
-        )}
+              return (
-        <Col md={16 - offset} sm={24} xs={24}>
+                <Menu.Item key={tab.label}>
-          <Menu mode={showMenu} style={{ backgroundColor: 'transparent' }}>
+                  <li
-            {props.tabs &&
+                    role="tab"
-              props.tabs.map(tab => {
+                    data-test={tab['data-test']}
-                if ((props.usesRouter || hasHistory) && !!tab.usesRouter) {
+                    className={tab.name === props.activeChild ? 'active' : ''}
-                  return (
+                  >
-                    <Menu.Item key={tab.label}>
+                    <div>
-                      <li
+                      <Link to={tab.url || ''}>{tab.label}</Link>
-                        role="tab"
+                    </div>
-                        data-test={tab['data-test']}
+                  </li>
-                        className={
+                </Menu.Item>
-                          tab.name === props.activeChild ? 'active' : ''
+              );
-                        }
+            }
-                      >
-                        <div>
-                          <Link to={tab.url || ''}>{tab.label}</Link>
-                        </div>
-                      </li>
-                    </Menu.Item>
-                  );
-                }
 
-                return (
+            return (
-                  <Menu.Item key={tab.label}>
+              <Menu.Item key={tab.label}>
-                    <li
+                <li
-                      className={cx('no-router', {
+                  className={cx('no-router', {
-                        active: tab.name === props.activeChild,
+                    active: tab.name === props.activeChild,
-                      })}
+                  })}
-                      role="tab"
+                  role="tab"
-                    >
+                >
-                      <a href={tab.url} onClick={tab.onClick}>
+                  <a href={tab.url} onClick={tab.onClick}>
-                        {tab.label}
+                    {tab.label}
-                      </a>
+                  </a>
-                    </li>
+                </li>
-                  </Menu.Item>
+              </Menu.Item>
-                );
+            );
-              })}
+          })}
-          </Menu>
+        </Menu>
-        </Col>
+        <div className={navRightStyle}>
-        <Col lg={8} md={navRightCol} sm={24} xs={24}>
+          {props.buttons?.map((btn, i) => (
-          <div className={navRightStyle}>
+            <Button
-            {props.buttons?.map((btn, i) => (
+              key={i}
-              <Button
+              buttonStyle={btn.buttonStyle}
-                key={i}
+              onClick={btn.onClick}
-                buttonStyle={btn.buttonStyle}
+              data-test={btn['data-test']}
-                onClick={btn.onClick}
+            >
-                data-test={btn['data-test']}
+              {btn.name}
-              >
+            </Button>
-                {btn.name}
+          ))}
-              </Button>
+        </div>
-            ))}
-          </div>
-        </Col>
       </Row>
       {props.children}
     </StyledHeader>

superset-frontend/src/components/Select/styles.tsx

@@ -101,7 +101,7 @@ export const defaultTheme: (
     controlHeight: 34,
     lineHeight: 19,
     fontSize: 14,
-    minWidth: '7.5em', // just enough to display 'No options'
+    minWidth: '6.5em',
   },
 });
 

superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx

@@ -181,7 +181,7 @@ const DashboardBuilder: FC<DashboardBuilderProps> = () => {
             depth={DASHBOARD_ROOT_DEPTH}
             index={0}
             orientation="column"
-            onDrop={() => dispatch(handleComponentDrop)}
+            onDrop={dropResult => dispatch(handleComponentDrop(dropResult))}
             editMode={editMode}
             // you cannot drop on/displace tabs if they already exist
             disableDragdrop={!!topLevelTabs}

superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx

@@ -72,6 +72,7 @@ const DashboardContainer: FC<DashboardContainerProps> = ({ topLevelTabs }) => {
             activeKey={activeKey}
             renderTabBar={() => <></>}
             fullWidth={false}
+            allowOverflow
           >
             {childIds.map((id, index) => (
               // Matching the key of the first TabPane irrespective of topLevelTabs

superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/HeaderActionsDropdown.test.tsx

@@ -65,7 +65,7 @@ const editModeOnProps = {
 
 function setup(props: HeaderDropdownProps) {
   return (
-    <div className="dashboard">
+    <div className="dashboard-header">
       <HeaderActionsDropdown {...props} />
     </div>
   );

superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/index.jsx

@@ -317,7 +317,7 @@ class HeaderActionsDropdown extends React.PureComponent {
         overlay={menu}
         trigger={['click']}
         getPopupContainer={triggerNode =>
-          triggerNode.closest(SCREENSHOT_NODE_SELECTOR)
+          triggerNode.closest('.dashboard-header')
         }
       >
         <DropdownButton id="save-dash-split-button" role="button">

superset-frontend/src/dashboard/components/SliceHeaderControls/index.jsx

@@ -266,6 +266,7 @@ class SliceHeaderControls extends React.PureComponent {
             copyMenuItemTitle={t('Copy chart URL')}
             emailMenuItemTitle={t('Share chart by email')}
             emailSubject={t('Superset chart')}
+            emailBody={t('Check out this chart: ')}
             addSuccessToast={addSuccessToast}
             addDangerToast={addDangerToast}
           />

superset-frontend/src/dashboard/components/dnd/handleHover.js

@@ -17,7 +17,8 @@
  * under the License.
  */
 import { throttle } from 'lodash';
-import getDropPosition from '../../util/getDropPosition';
+import { DASHBOARD_ROOT_TYPE } from 'src/dashboard/util/componentTypes';
+import getDropPosition from 'src/dashboard/util/getDropPosition';
 import handleScroll from './handleScroll';
 
 const HOVER_THROTTLE_MS = 100;
@@ -28,9 +29,13 @@ function handleHover(props, monitor, Component) {
 
   const dropPosition = getDropPosition(monitor, Component);
 
-  handleScroll(dropPosition);
+  const isDashboardRoot =
+    Component?.props?.component?.type === DASHBOARD_ROOT_TYPE;
+  const scroll = isDashboardRoot ? 'SCROLL_TOP' : null;
 
-  if (!dropPosition || dropPosition === 'SCROLL_TOP') {
+  handleScroll(scroll);
+
+  if (!dropPosition) {
     Component.setState(() => ({ dropIndicator: null }));
     return;
   }

superset-frontend/src/dashboard/components/dnd/handleScroll/handleScroll.test.ts

@@ -28,6 +28,7 @@ afterAll(() => {
 
 test('calling: "NOT_SCROLL_TOP" ,"SCROLL_TOP", "NOT_SCROLL_TOP"', () => {
   window.scroll = jest.fn();
+  document.documentElement.scrollTop = 500;
 
   handleScroll('NOT_SCROLL_TOP');
 

superset-frontend/src/dashboard/components/dnd/handleScroll/index.ts

@@ -20,22 +20,34 @@ let scrollTopDashboardInterval: any;
 const SCROLL_STEP = 120;
 const INTERVAL_DELAY = 50;
 
-export default function handleScroll(dropPosition: string) {
+export default function handleScroll(scroll: string) {
-  if (dropPosition === 'SCROLL_TOP') {
+  const setupScroll =
-    if (!scrollTopDashboardInterval) {
+    scroll === 'SCROLL_TOP' &&
-      scrollTopDashboardInterval = setInterval(() => {
+    !scrollTopDashboardInterval &&
-        let scrollTop = document.documentElement.scrollTop - SCROLL_STEP;
+    document.documentElement.scrollTop !== 0;
-        if (scrollTop < 0) {
+
-          scrollTop = 0;
+  const clearScroll =
-        }
+    scrollTopDashboardInterval &&
-        window.scroll({
+    (scroll !== 'SCROLL_TOP' || document.documentElement.scrollTop === 0);
-          top: scrollTop,
+
-          behavior: 'smooth',
+  if (setupScroll) {
-        });
+    scrollTopDashboardInterval = setInterval(() => {
-      }, INTERVAL_DELAY);
+      if (document.documentElement.scrollTop === 0) {
-    }
+        clearInterval(scrollTopDashboardInterval);
-  }
+        scrollTopDashboardInterval = null;
-  if (dropPosition !== 'SCROLL_TOP' && scrollTopDashboardInterval) {
+        return;
+      }
+
+      let scrollTop = document.documentElement.scrollTop - SCROLL_STEP;
+      if (scrollTop < 0) {
+        scrollTop = 0;
+      }
+      window.scroll({
+        top: scrollTop,
+        behavior: 'smooth',
+      });
+    }, INTERVAL_DELAY);
+  } else if (clearScroll) {
     clearInterval(scrollTopDashboardInterval);
     scrollTopDashboardInterval = null;
   }

superset-frontend/src/dashboard/util/getDropPosition.js

@@ -17,13 +17,12 @@
  * under the License.
  */
 import isValidChild from './isValidChild';
-import { DASHBOARD_ROOT_TYPE, TAB_TYPE, TABS_TYPE } from './componentTypes';
+import { TAB_TYPE, TABS_TYPE } from './componentTypes';
 
 export const DROP_TOP = 'DROP_TOP';
 export const DROP_RIGHT = 'DROP_RIGHT';
 export const DROP_BOTTOM = 'DROP_BOTTOM';
 export const DROP_LEFT = 'DROP_LEFT';
-export const SCROLL_TOP = 'SCROLL_TOP';
 
 // this defines how close the mouse must be to the edge of a component to display
 // a sibling type drop indicator
@@ -55,10 +54,6 @@ export default function getDropPosition(monitor, Component) {
     return null;
   }
 
-  if (component.type === DASHBOARD_ROOT_TYPE) {
-    return SCROLL_TOP;
-  }
-
   // TODO need a better solution to prevent nested tabs
   if (
     draggingItem.type === TABS_TYPE &&

superset-frontend/src/datasource/ChangeDatasourceModal.tsx

@@ -248,6 +248,7 @@ const ChangeDatasourceModal: FunctionComponent<ChangeDatasourceModalProps> = ({
         {!confirmChange && (
           <>
             <Alert
+              roomBelow
               type="warning"
               message={
                 <>

superset-frontend/src/datasource/DatasourceEditor.jsx

@@ -239,7 +239,7 @@ function ColumnCollectionTable({
           ) : (
             v
           ),
-        type: d => <Label>{d}</Label>,
+        type: d => (d ? <Label>{d}</Label> : null),
         is_dttm: checkboxGenerator,
         filterable: checkboxGenerator,
         groupby: checkboxGenerator,

superset-frontend/src/explore/components/ControlRow.tsx

@@ -16,17 +16,19 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { ExpandedControlItem } from '@superset-ui/chart-controls';
 import React from 'react';
 
 const NUM_COLUMNS = 12;
 
-export default function ControlRow({
+type Control = React.ReactElement | null;
-  controls,
+
-}: {
+export default function ControlRow({ controls }: { controls: Control[] }) {
-  controls: ExpandedControlItem[];
+  // ColorMapControl renders null and should not be counted
-}) {
+  // in the columns number
-  const colSize = NUM_COLUMNS / controls.length;
+  const countableControls = controls.filter(
+    control => !['ColorMapControl'].includes(control?.props.type),
+  );
+  const colSize = NUM_COLUMNS / countableControls.length;
   return (
     <div className="row space-1">
       {controls.map((control, i) => (

superset-frontend/src/explore/components/QueryAndSaveBtns.jsx

@@ -93,6 +93,9 @@ export default function QueryAndSaveBtns({
         '& button': {
           width: 100,
         },
+        '.errMsg': {
+          marginLeft: theme.gridUnit * 4,
+        },
       }}
     >
       <ButtonGroup className="query-and-save">
@@ -110,7 +113,7 @@ export default function QueryAndSaveBtns({
         </Button>
       </ButtonGroup>
       {errorMessage && (
-        <span>
+        <span className="errMsg">
           {' '}
           <Tooltip
             id="query-error-tooltip"

superset-frontend/src/explore/components/controls/BoundsControl.jsx

@@ -18,9 +18,10 @@
  */
 import React from 'react';
 import PropTypes from 'prop-types';
-import { Row, Col, Input } from 'src/common/components';
+import { InputNumber } from 'src/common/components';
-import { t } from '@superset-ui/core';
+import { t, styled } from '@superset-ui/core';
-import ControlHeader from '../ControlHeader';
+import { isEqual, debounce } from 'lodash';
+import ControlHeader from 'src/explore/components/ControlHeader';
 
 const propTypes = {
   onChange: PropTypes.func,
@@ -32,35 +33,63 @@ const defaultProps = {
   value: [null, null],
 };
 
+const StyledDiv = styled.div`
+  display: flex;
+`;
+
+const MinInput = styled(InputNumber)`
+  flex: 1;
+  margin-right: ${({ theme }) => theme.gridUnit}px;
+`;
+
+const MaxInput = styled(InputNumber)`
+  flex: 1;
+  margin-left: ${({ theme }) => theme.gridUnit}px;
+`;
+
 export default class BoundsControl extends React.Component {
   constructor(props) {
     super(props);
     this.state = {
       minMax: [
-        props.value[0] === null ? '' : props.value[0],
+        Number.isNaN(this.props.value[0]) ? '' : props.value[0],
-        props.value[1] === null ? '' : props.value[1],
+        Number.isNaN(this.props.value[1]) ? '' : props.value[1],
       ],
     };
-    this.onChange = this.onChange.bind(this);
+    this.onChange = debounce(this.onChange.bind(this), 300);
     this.onMinChange = this.onMinChange.bind(this);
     this.onMaxChange = this.onMaxChange.bind(this);
+    this.update = this.update.bind(this);
   }
 
-  onMinChange(event) {
+  componentDidUpdate(prevProps) {
-    const min = event.target.value;
+    if (!isEqual(prevProps.value, this.props.value)) {
+      this.update();
+    }
+  }
+
+  update() {
+    this.setState({
+      minMax: [
+        Number.isNaN(this.props.value[0]) ? '' : this.props.value[0],
+        Number.isNaN(this.props.value[1]) ? '' : this.props.value[1],
+      ],
+    });
+  }
+
+  onMinChange(value) {
     this.setState(
       prevState => ({
-        minMax: [min, prevState.minMax[1]],
+        minMax: [value, prevState.minMax[1]],
       }),
       this.onChange,
     );
   }
 
-  onMaxChange(event) {
+  onMaxChange(value) {
-    const max = event.target.value;
     this.setState(
       prevState => ({
-        minMax: [prevState.minMax[0], max],
+        minMax: [prevState.minMax[0], value],
       }),
       this.onChange,
     );
@@ -68,44 +97,29 @@ export default class BoundsControl extends React.Component {
 
   onChange() {
     const mm = this.state.minMax;
-    const errors = [];
+    const min = parseFloat(mm[0]) || null;
-    if (mm[0] && Number.isNaN(Number(mm[0]))) {
+    const max = parseFloat(mm[1]) || null;
-      errors.push(t('`Min` value should be numeric or empty'));
+    this.props.onChange([min, max]);
-    }
-    if (mm[1] && Number.isNaN(Number(mm[1]))) {
-      errors.push(t('`Max` value should be numeric or empty'));
-    }
-    if (errors.length === 0) {
-      this.props.onChange([parseFloat(mm[0]), parseFloat(mm[1])], errors);
-    } else {
-      this.props.onChange([null, null], errors);
-    }
   }
 
   render() {
     return (
       <div>
         <ControlHeader {...this.props} />
-        <Row gutter={16}>
+        <StyledDiv>
-          <Col xs={12}>
+          <MinInput
-            <Input
+            data-test="min-bound"
-              data-test="min-bound"
+            placeholder={t('Min')}
-              type="text"
+            onChange={this.onMinChange}
-              placeholder={t('Min')}
+            value={this.state.minMax[0]}
-              onChange={this.onMinChange}
+          />
-              value={this.state.minMax[0]}
+          <MaxInput
-            />
+            data-test="max-bound"
-          </Col>
+            placeholder={t('Max')}
-          <Col xs={12}>
+            onChange={this.onMaxChange}
-            <Input
+            value={this.state.minMax[1]}
-              type="text"
+          />
-              data-test="max-bound"
+        </StyledDiv>
-              placeholder={t('Max')}
-              onChange={this.onMaxChange}
-              value={this.state.minMax[1]}
-            />
-          </Col>
-        </Row>
       </div>
     );
   }

superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.jsx

@@ -425,7 +425,7 @@ export default class AdhocMetricEditPopover extends React.PureComponent {
                 }
                 editorProps={{ $blockScrolling: true }}
                 enableLiveAutocompletion
-                className="adhoc-filter-sql-editor"
+                className="filter-sql-editor"
                 wrapEnabled
               />
             ) : (

superset-frontend/src/explore/components/controls/TimeSeriesColumnControl/TimeSeriesColumnControl.test.tsx

@@ -21,6 +21,8 @@ import { render, screen } from 'spec/helpers/testing-library';
 import userEvent from '@testing-library/user-event';
 import TimeSeriesColumnControl from '.';
 
+jest.mock('lodash/debounce', () => jest.fn(fn => fn));
+
 test('renders with default props', () => {
   render(<TimeSeriesColumnControl />);
   expect(screen.getByText('Time series columns')).toBeInTheDocument();
@@ -36,16 +38,6 @@ test('renders popover on edit', () => {
   expect(screen.getByText('Type')).toBeInTheDocument();
 });
 
-test('triggers onChange when type changes', () => {
-  const onChange = jest.fn();
-  render(<TimeSeriesColumnControl onChange={onChange} />);
-  userEvent.click(screen.getByRole('button'));
-  userEvent.click(screen.getByText('Select...'));
-  expect(onChange).not.toHaveBeenCalled();
-  userEvent.click(screen.getByText('Time comparison'));
-  expect(onChange).toHaveBeenCalled();
-});
-
 test('renders time comparison', () => {
   render(<TimeSeriesColumnControl colType="time" />);
   userEvent.click(screen.getByRole('button'));
@@ -82,23 +74,47 @@ test('renders period average', () => {
   expect(screen.getByText('Number format')).toBeInTheDocument();
 });
 
+test('triggers onChange when type changes', () => {
+  const onChange = jest.fn();
+  render(<TimeSeriesColumnControl onChange={onChange} />);
+  userEvent.click(screen.getByRole('button'));
+  userEvent.click(screen.getByText('Select...'));
+  userEvent.click(screen.getByText('Time comparison'));
+  expect(onChange).not.toHaveBeenCalled();
+  userEvent.click(screen.getByRole('button', { name: 'Save' }));
+  expect(onChange).toHaveBeenCalledWith(
+    expect.objectContaining({ colType: 'time' }),
+  );
+});
+
 test('triggers onChange when time lag changes', () => {
+  const timeLag = '1';
   const onChange = jest.fn();
   render(<TimeSeriesColumnControl colType="time" onChange={onChange} />);
   userEvent.click(screen.getByRole('button'));
+  const timeLagInput = screen.getByPlaceholderText('Time Lag');
+  userEvent.clear(timeLagInput);
+  userEvent.type(timeLagInput, timeLag);
   expect(onChange).not.toHaveBeenCalled();
-  userEvent.type(screen.getByPlaceholderText('Time Lag'), '1');
+  userEvent.click(screen.getByRole('button', { name: 'Save' }));
-  expect(onChange).toHaveBeenCalled();
+  expect(onChange).toHaveBeenCalledWith(expect.objectContaining({ timeLag }));
 });
 
 test('triggers onChange when color bounds changes', () => {
+  const min = 1;
+  const max = 5;
   const onChange = jest.fn();
   render(<TimeSeriesColumnControl colType="time" onChange={onChange} />);
   userEvent.click(screen.getByRole('button'));
+  const minInput = screen.getByPlaceholderText('Min');
+  const maxInput = screen.getByPlaceholderText('Max');
+  userEvent.type(minInput, min.toString());
+  userEvent.type(maxInput, max.toString());
   expect(onChange).not.toHaveBeenCalled();
-  userEvent.type(screen.getByPlaceholderText('Min'), '1');
+  userEvent.click(screen.getByRole('button', { name: 'Save' }));
-  userEvent.type(screen.getByPlaceholderText('Max'), '10');
+  expect(onChange).toHaveBeenLastCalledWith(
-  expect(onChange).toHaveBeenCalledTimes(3);
+    expect.objectContaining({ bounds: [min, max] }),
+  );
 });
 
 test('triggers onChange when time type changes', () => {
@@ -106,71 +122,102 @@ test('triggers onChange when time type changes', () => {
   render(<TimeSeriesColumnControl colType="time" onChange={onChange} />);
   userEvent.click(screen.getByRole('button'));
   userEvent.click(screen.getByText('Select...'));
-  expect(onChange).not.toHaveBeenCalled();
   userEvent.click(screen.getByText('Difference'));
-  expect(onChange).toHaveBeenCalled();
+  expect(onChange).not.toHaveBeenCalled();
+  userEvent.click(screen.getByRole('button', { name: 'Save' }));
+  expect(onChange).toHaveBeenCalledWith(
+    expect.objectContaining({ comparisonType: 'diff' }),
+  );
 });
 
 test('triggers onChange when number format changes', () => {
+  const numberFormatString = 'Test format';
   const onChange = jest.fn();
   render(<TimeSeriesColumnControl colType="time" onChange={onChange} />);
   userEvent.click(screen.getByRole('button'));
+  userEvent.type(
+    screen.getByPlaceholderText('Number format string'),
+    numberFormatString,
+  );
   expect(onChange).not.toHaveBeenCalled();
-  userEvent.type(screen.getByPlaceholderText('Number format string'), 'format');
+  userEvent.click(screen.getByRole('button', { name: 'Save' }));
-  expect(onChange).toHaveBeenCalled();
+  expect(onChange).toHaveBeenCalledWith(
+    expect.objectContaining({ d3format: numberFormatString }),
+  );
 });
 
 test('triggers onChange when width changes', () => {
+  const width = '10';
   const onChange = jest.fn();
   render(<TimeSeriesColumnControl colType="spark" onChange={onChange} />);
   userEvent.click(screen.getByRole('button'));
+  userEvent.type(screen.getByPlaceholderText('Width'), width);
   expect(onChange).not.toHaveBeenCalled();
-  userEvent.type(screen.getByPlaceholderText('Width'), '10');
+  userEvent.click(screen.getByRole('button', { name: 'Save' }));
-  expect(onChange).toHaveBeenCalled();
+  expect(onChange).toHaveBeenCalledWith(expect.objectContaining({ width }));
 });
 
 test('triggers onChange when height changes', () => {
+  const height = '10';
   const onChange = jest.fn();
   render(<TimeSeriesColumnControl colType="spark" onChange={onChange} />);
   userEvent.click(screen.getByRole('button'));
+  userEvent.type(screen.getByPlaceholderText('Height'), height);
   expect(onChange).not.toHaveBeenCalled();
-  userEvent.type(screen.getByPlaceholderText('Height'), '10');
+  userEvent.click(screen.getByRole('button', { name: 'Save' }));
-  expect(onChange).toHaveBeenCalled();
+  expect(onChange).toHaveBeenCalledWith(expect.objectContaining({ height }));
 });
 
 test('triggers onChange when time ratio changes', () => {
+  const timeRatio = '10';
   const onChange = jest.fn();
   render(<TimeSeriesColumnControl colType="spark" onChange={onChange} />);
   userEvent.click(screen.getByRole('button'));
+  userEvent.type(screen.getByPlaceholderText('Time Ratio'), timeRatio);
   expect(onChange).not.toHaveBeenCalled();
-  userEvent.type(screen.getByPlaceholderText('Time Ratio'), '10');
+  userEvent.click(screen.getByRole('button', { name: 'Save' }));
-  expect(onChange).toHaveBeenCalled();
+  expect(onChange).toHaveBeenCalledWith(expect.objectContaining({ timeRatio }));
 });
 
 test('triggers onChange when show Y-axis changes', () => {
   const onChange = jest.fn();
   render(<TimeSeriesColumnControl colType="spark" onChange={onChange} />);
   userEvent.click(screen.getByRole('button'));
-  expect(onChange).not.toHaveBeenCalled();
   userEvent.click(screen.getByRole('checkbox'));
-  expect(onChange).toHaveBeenCalled();
+  expect(onChange).not.toHaveBeenCalled();
+  userEvent.click(screen.getByRole('button', { name: 'Save' }));
+  expect(onChange).toHaveBeenCalledWith(
+    expect.objectContaining({ showYAxis: true }),
+  );
 });
 
 test('triggers onChange when Y-axis bounds changes', () => {
+  const min = 1;
+  const max = 5;
   const onChange = jest.fn();
   render(<TimeSeriesColumnControl colType="spark" onChange={onChange} />);
   userEvent.click(screen.getByRole('button'));
+  const minInput = screen.getByPlaceholderText('Min');
+  const maxInput = screen.getByPlaceholderText('Max');
+  userEvent.type(minInput, min.toString());
+  userEvent.clear(maxInput);
+  userEvent.type(maxInput, max.toString());
   expect(onChange).not.toHaveBeenCalled();
-  userEvent.type(screen.getByPlaceholderText('Min'), '1');
+  userEvent.click(screen.getByRole('button', { name: 'Save' }));
-  userEvent.type(screen.getByPlaceholderText('Max'), '10');
+  expect(onChange).toHaveBeenCalledWith(
-  expect(onChange).toHaveBeenCalledTimes(3);
+    expect.objectContaining({ yAxisBounds: [min, max] }),
+  );
 });
 
 test('triggers onChange when date format changes', () => {
+  const dateFormat = 'yy/MM/dd';
   const onChange = jest.fn();
   render(<TimeSeriesColumnControl colType="spark" onChange={onChange} />);
   userEvent.click(screen.getByRole('button'));
+  userEvent.type(screen.getByPlaceholderText('Date format string'), dateFormat);
   expect(onChange).not.toHaveBeenCalled();
-  userEvent.type(screen.getByPlaceholderText('Date format string'), 'yy/MM/dd');
+  userEvent.click(screen.getByRole('button', { name: 'Save' }));
-  expect(onChange).toHaveBeenCalled();
+  expect(onChange).toHaveBeenCalledWith(
+    expect.objectContaining({ dateFormat }),
+  );
 });

superset-frontend/src/explore/components/controls/TimeSeriesColumnControl/index.jsx

@@ -19,11 +19,11 @@
 import React from 'react';
 import PropTypes from 'prop-types';
 import { Row, Col, Input } from 'src/common/components';
+import Button from 'src/components/Button';
 import Popover from 'src/components/Popover';
 import Select from 'src/components/Select';
 import { t, styled } from '@superset-ui/core';
 import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls';
-
 import BoundsControl from '../BoundsControl';
 import CheckboxControl from '../CheckboxControl';
 
@@ -76,6 +76,8 @@ const colTypeOptions = [
 
 const StyledRow = styled(Row)`
   margin-top: ${({ theme }) => theme.gridUnit * 2}px;
+  display: flex;
+  align-items: center;
 `;
 
 const StyledCol = styled(Col)`
@@ -88,10 +90,27 @@ const StyledTooltip = styled(InfoTooltipWithTrigger)`
   color: ${({ theme }) => theme.colors.grayscale.light1};
 `;
 
+const ButtonBar = styled.div`
+  margin-top: ${({ theme }) => theme.gridUnit * 5}px;
+  display: flex;
+  justify-content: center;
+`;
+
 export default class TimeSeriesColumnControl extends React.Component {
   constructor(props) {
     super(props);
-    const state = {
+
+    this.onSave = this.onSave.bind(this);
+    this.onClose = this.onClose.bind(this);
+    this.resetState = this.resetState.bind(this);
+    this.initialState = this.initialState.bind(this);
+    this.onPopoverVisibleChange = this.onPopoverVisibleChange.bind(this);
+
+    this.state = this.initialState();
+  }
+
+  initialState() {
+    return {
       label: this.props.label,
       tooltip: this.props.tooltip,
       colType: this.props.colType,
@@ -105,57 +124,73 @@ export default class TimeSeriesColumnControl extends React.Component {
       bounds: this.props.bounds,
       d3format: this.props.d3format,
       dateFormat: this.props.dateFormat,
+      popoverVisible: false,
     };
-    delete state.onChange;
-    this.state = state;
-    this.onChange = this.onChange.bind(this);
   }
 
-  onChange() {
+  resetState() {
+    const initialState = this.initialState();
+    this.setState({ ...initialState });
+  }
+
+  onSave() {
     this.props.onChange(this.state);
+    this.setState({ popoverVisible: false });
+  }
+
+  onClose() {
+    this.resetState();
   }
 
   onSelectChange(attr, opt) {
-    this.setState({ [attr]: opt.value }, this.onChange);
+    this.setState({ [attr]: opt.value });
   }
 
   onTextInputChange(attr, event) {
-    this.setState({ [attr]: event.target.value }, this.onChange);
+    this.setState({ [attr]: event.target.value });
   }
 
   onCheckboxChange(attr, value) {
-    this.setState({ [attr]: value }, this.onChange);
+    this.setState({ [attr]: value });
   }
 
   onBoundsChange(bounds) {
-    this.setState({ bounds }, this.onChange);
+    this.setState({ bounds });
+  }
+
+  onPopoverVisibleChange(popoverVisible) {
+    if (popoverVisible) {
+      this.setState({ popoverVisible });
+    } else {
+      this.resetState();
+    }
   }
 
   onYAxisBoundsChange(yAxisBounds) {
-    this.setState({ yAxisBounds }, this.onChange);
+    this.setState({ yAxisBounds });
   }
 
   textSummary() {
-    return `${this.state.label}`;
+    return `${this.props.label}`;
   }
 
   formRow(label, tooltip, ttLabel, control) {
     return (
       <StyledRow>
-        <StyledCol xs={24} md={10}>
+        <StyledCol xs={24} md={11}>
           {label}
           <StyledTooltip placement="top" tooltip={tooltip} label={ttLabel} />
         </StyledCol>
-        <StyledCol xs={24} md={14}>
+        <Col xs={24} md={13}>
           {control}
-        </StyledCol>
+        </Col>
       </StyledRow>
     );
   }
 
   renderPopover() {
     return (
-      <div id="ts-col-popo" style={{ width: 300 }}>
+      <div id="ts-col-popo" style={{ width: 320 }}>
         {this.formRow(
           'Label',
           'The column header label',
@@ -297,6 +332,19 @@ export default class TimeSeriesColumnControl extends React.Component {
               placeholder="Date format string"
             />,
           )}
+        <ButtonBar>
+          <Button buttonSize="small" onClick={this.onClose} cta>
+            {t('Close')}
+          </Button>
+          <Button
+            buttonStyle="primary"
+            buttonSize="small"
+            onClick={this.onSave}
+            cta
+          >
+            {t('Save')}
+          </Button>
+        </ButtonBar>
       </div>
     );
   }
@@ -310,6 +358,8 @@ export default class TimeSeriesColumnControl extends React.Component {
           placement="right"
           content={this.renderPopover()}
           title="Column Configuration"
+          visible={this.state.popoverVisible}
+          onVisibleChange={this.onPopoverVisibleChange}
         >
           <InfoTooltipWithTrigger
             icon="edit"

superset-frontend/src/explore/components/optionRenderers.tsx

@@ -27,8 +27,16 @@ import {
 } from '@superset-ui/chart-controls';
 
 const OptionContainer = styled.div`
+  > span {
+    display: flex;
+    align-items: center;
+  }
+
   .option-label {
     display: inline-block;
+    overflow: hidden;
+    text-overflow: ellipsis;
+    white-space: nowrap;
     & ~ i {
       margin-left: ${({ theme }) => theme.gridUnit}px;
     }

superset-frontend/src/views/CRUD/annotationlayers/AnnotationLayersList.tsx

@@ -344,11 +344,7 @@ function AnnotationLayersList({
 
   return (
     <>
-      <SubMenu
+      <SubMenu name={t('Annotation layers')} buttons={subMenuButtons} />
-        headerSize={8}
-        name={t('Annotation layers')}
-        buttons={subMenuButtons}
-      />
       <AnnotationLayerModal
         addDangerToast={addDangerToast}
         layer={currentAnnotationLayer}

superset-frontend/src/views/CRUD/csstemplates/CssTemplatesList.tsx

@@ -303,7 +303,7 @@ function CssTemplatesList({
 
   return (
     <>
-      <SubMenu headerSize={8} {...menuData} />
+      <SubMenu {...menuData} />
       <CssTemplateModal
         addDangerToast={addDangerToast}
         cssTemplate={currentCssTemplate}

superset-frontend/src/views/CRUD/data/components/SyntaxHighlighterCopy/index.tsx

@@ -36,11 +36,13 @@ SyntaxHighlighter.registerLanguage('json', jsonSyntax);
 
 const SyntaxHighlighterWrapper = styled.div`
   margin-top: -24px;
+
   &:hover {
     svg {
       visibility: visible;
     }
   }
+
   svg {
     position: relative;
     top: 40px;
@@ -64,13 +66,13 @@ export default function SyntaxHighlighterCopy({
   function copyToClipboard(textToCopy: string) {
     copyTextToClipboard(textToCopy)
       .then(() => {
-        if (addDangerToast) {
+        if (addSuccessToast) {
-          addDangerToast(t('Sorry, your browser does not support copying.'));
+          addSuccessToast(t('SQL Copied!'));
         }
       })
       .catch(() => {
-        if (addSuccessToast) {
+        if (addDangerToast) {
-          addSuccessToast(t('SQL Copied!'));
+          addDangerToast(t('Sorry, your browser does not support copying.'));
         }
       });
   }

superset-frontend/stylesheets/superset.less

@@ -582,28 +582,3 @@ hr {
   background-image: url('../images/icons/error_solid_small_red.svg') !important;
   background-position: -2px center !important;
 }
-
-// AntD overrides since these are injected as inline styles and can't
-// be overriden in emotion
-.ant-menu-submenu.ant-menu-submenu-popup.ant-menu.ant-menu-light.ant-menu-submenu-placement-bottomLeft {
-  top: 51px !important;
-  margin-left: -2px !important;
-  @media (max-width: 767px) {
-    top: 269px !important;
-  }
-  & > .ant-menu {
-    border-radius: 0px !important;
-  }
-}
-
-.ant-menu-submenu.ant-menu-submenu-popup.ant-menu.ant-menu-light {
-  top: 51px !important;
-  border-radius: 0px !important;
-  @media (max-width: 767px) {
-    top: 269px !important;
-  }
-}
-
-.ant-dropdown.ant-dropdown-placement-bottomRight {
-  top: 133px !important;
-}