You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ru...@apache.org on 2020/11/05 21:30:48 UTC

[incubator-superset] branch master updated: refactor: Replace react-bootstrap MenuItems with Antd Menu (#11554)

This is an automated email from the ASF dual-hosted git repository.

rusackas pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git


The following commit(s) were added to refs/heads/master by this push:
     new 1490f307 refactor: Replace react-bootstrap MenuItems with Antd Menu (#11554)
1490f307 is described below

commit 1490f3074d3416eb7b68b1fd0b012994b440fac1
Author: Kamil Gabryjelski <ka...@gmail.com>
AuthorDate: Thu Nov 5 22:30:03 2020 +0100

    refactor: Replace react-bootstrap MenuItems with Antd Menu (#11554)
    
    * Refactor SliceHeaderControls
    
    * Refactor DisplayQueryButton
    
    * Fix duplicate keys
    
    * Refactor SliceAdder
    
    * Move css from styles to Emotion
    
    * Fix e2e test
---
 .../cypress/integration/dashboard/controls.test.js |   3 +-
 .../explore/components/DisplayQueryButton_spec.jsx |   6 +-
 superset-frontend/src/components/Menu/Menu.tsx     |   2 +-
 superset-frontend/src/components/ModalTrigger.jsx  |  11 --
 .../src/components/URLShortLinkModal.jsx           |   2 -
 .../src/dashboard/components/SliceAdder.jsx        |  38 +++--
 .../dashboard/components/SliceHeaderControls.jsx   | 149 ++++++++++-------
 .../dashboard/stylesheets/components/chart.less    |  12 --
 .../src/dashboard/stylesheets/dashboard.less       |  12 +-
 .../src/explore/components/DisplayQueryButton.jsx  | 142 +++++++++-------
 .../components/controls/DateFilterControl.jsx      | 186 +++++++++++----------
 11 files changed, 291 insertions(+), 272 deletions(-)

diff --git a/superset-frontend/cypress-base/cypress/integration/dashboard/controls.test.js b/superset-frontend/cypress-base/cypress/integration/dashboard/controls.test.js
index 6f9215c..8330eab 100644
--- a/superset-frontend/cypress-base/cypress/integration/dashboard/controls.test.js
+++ b/superset-frontend/cypress-base/cypress/integration/dashboard/controls.test.js
@@ -81,8 +81,7 @@ describe('Dashboard top-level controls', () => {
       .next()
       .find('[data-test="dashboard-slice-refresh-tooltip"]')
       .parent()
-      .parent()
-      .should('have.class', 'disabled');
+      .should('have.class', 'ant-menu-item-disabled');
 
     cy.wait(`@postJson_${mapId}_force`);
     cy.get('[data-test="refresh-dashboard-menu-item"]').should(
diff --git a/superset-frontend/spec/javascripts/explore/components/DisplayQueryButton_spec.jsx b/superset-frontend/spec/javascripts/explore/components/DisplayQueryButton_spec.jsx
index e49487e..4d1bb45 100644
--- a/superset-frontend/spec/javascripts/explore/components/DisplayQueryButton_spec.jsx
+++ b/superset-frontend/spec/javascripts/explore/components/DisplayQueryButton_spec.jsx
@@ -18,10 +18,10 @@
  */
 import React from 'react';
 import { mount } from 'enzyme';
+import { supersetTheme, ThemeProvider } from '@superset-ui/core';
+import { Menu } from 'src/common/components';
 import ModalTrigger from 'src/components/ModalTrigger';
 import { DisplayQueryButton } from 'src/explore/components/DisplayQueryButton';
-import { MenuItem } from 'react-bootstrap';
-import { supersetTheme, ThemeProvider } from '@superset-ui/core';
 
 describe('DisplayQueryButton', () => {
   const defaultProps = {
@@ -51,6 +51,6 @@ describe('DisplayQueryButton', () => {
       },
     });
     expect(wrapper.find(ModalTrigger)).toHaveLength(3);
-    expect(wrapper.find(MenuItem)).toHaveLength(5);
+    expect(wrapper.find(Menu.Item)).toHaveLength(5);
   });
 });
diff --git a/superset-frontend/src/components/Menu/Menu.tsx b/superset-frontend/src/components/Menu/Menu.tsx
index 90b2e57..a193b08 100644
--- a/superset-frontend/src/components/Menu/Menu.tsx
+++ b/superset-frontend/src/components/Menu/Menu.tsx
@@ -191,7 +191,7 @@ export function Menu({
                   </DropdownMenu.ItemGroup>,
                 ]}
                 {(navbarRight.version_string || navbarRight.version_sha) && [
-                  <DropdownMenu.Divider key="user-divider" />,
+                  <DropdownMenu.Divider key="navbar-divider" />,
                   <DropdownMenu.ItemGroup
                     key="about-section"
                     title={t('About')}
diff --git a/superset-frontend/src/components/ModalTrigger.jsx b/superset-frontend/src/components/ModalTrigger.jsx
index 90c8210..ddcc574 100644
--- a/superset-frontend/src/components/ModalTrigger.jsx
+++ b/superset-frontend/src/components/ModalTrigger.jsx
@@ -18,7 +18,6 @@
  */
 import React from 'react';
 import PropTypes from 'prop-types';
-import { MenuItem } from 'react-bootstrap';
 import Modal from 'src/common/components/Modal';
 import Button from 'src/components/Button';
 
@@ -31,7 +30,6 @@ const propTypes = {
   beforeOpen: PropTypes.func,
   onExit: PropTypes.func,
   isButton: PropTypes.bool,
-  isMenuItem: PropTypes.bool,
   className: PropTypes.string,
   tooltip: PropTypes.string,
   width: PropTypes.string,
@@ -43,7 +41,6 @@ const defaultProps = {
   beforeOpen: () => {},
   onExit: () => {},
   isButton: false,
-  isMenuItem: false,
   className: '',
   modalTitle: '',
 };
@@ -102,14 +99,6 @@ export default class ModalTrigger extends React.Component {
         </>
       );
     }
-    if (this.props.isMenuItem) {
-      return (
-        <>
-          <MenuItem onClick={this.open}>{this.props.triggerNode}</MenuItem>
-          {this.renderModal()}
-        </>
-      );
-    }
     /* eslint-disable jsx-a11y/interactive-supports-focus */
     return (
       <>
diff --git a/superset-frontend/src/components/URLShortLinkModal.jsx b/superset-frontend/src/components/URLShortLinkModal.jsx
index b999712..2b5bbfc 100644
--- a/superset-frontend/src/components/URLShortLinkModal.jsx
+++ b/superset-frontend/src/components/URLShortLinkModal.jsx
@@ -29,7 +29,6 @@ const propTypes = {
   emailSubject: PropTypes.string,
   emailContent: PropTypes.string,
   addDangerToast: PropTypes.func.isRequired,
-  isMenuItem: PropTypes.bool,
   title: PropTypes.string,
   triggerNode: PropTypes.node.isRequired,
 };
@@ -65,7 +64,6 @@ class URLShortLinkModal extends React.Component {
     return (
       <ModalTrigger
         ref={this.setModalRef}
-        isMenuItem={this.props.isMenuItem}
         triggerNode={this.props.triggerNode}
         beforeOpen={this.getCopyUrl}
         modalTitle={this.props.title || t('Share Dashboard')}
diff --git a/superset-frontend/src/dashboard/components/SliceAdder.jsx b/superset-frontend/src/dashboard/components/SliceAdder.jsx
index f9fc471..141ccac 100644
--- a/superset-frontend/src/dashboard/components/SliceAdder.jsx
+++ b/superset-frontend/src/dashboard/components/SliceAdder.jsx
@@ -19,11 +19,11 @@
 /* eslint-env browser */
 import React from 'react';
 import PropTypes from 'prop-types';
-import { DropdownButton, MenuItem } from 'react-bootstrap';
 import { List } from 'react-virtualized';
 import SearchInput, { createFilter } from 'react-search-input';
 import { t } from '@superset-ui/core';
 
+import { Select } from 'src/common/components';
 import AddSliceCard from './AddSliceCard';
 import AddSliceDragPreview from './dnd/AddSliceDragPreview';
 import DragDroppable from './dnd/DragDroppable';
@@ -52,12 +52,14 @@ const defaultProps = {
 };
 
 const KEYS_TO_FILTERS = ['slice_name', 'viz_type', 'datasource_name'];
-const KEYS_TO_SORT = [
-  { key: 'slice_name', label: 'Name' },
-  { key: 'viz_type', label: 'Vis type' },
-  { key: 'datasource_name', label: 'Dataset' },
-  { key: 'changed_on', label: 'Recent' },
-];
+const KEYS_TO_SORT = {
+  slice_name: 'Name',
+  viz_type: 'Vis type',
+  datasource_name: 'Dataset',
+  changed_on: 'Recent',
+};
+
+const DEFAULT_SORT_KEY = 'changed_on';
 
 const MARGIN_BOTTOM = 16;
 const SIDEPANE_HEADER_HEIGHT = 30;
@@ -84,7 +86,7 @@ class SliceAdder extends React.Component {
     this.state = {
       filteredSlices: [],
       searchTerm: '',
-      sortBy: KEYS_TO_SORT.findIndex(item => item.key === 'changed_on'),
+      sortBy: DEFAULT_SORT_KEY,
       selectedSliceIdsSet: new Set(props.selectedSliceIds),
     };
     this.rowRenderer = this.rowRenderer.bind(this);
@@ -102,7 +104,7 @@ class SliceAdder extends React.Component {
     if (nextProps.lastUpdated !== this.props.lastUpdated) {
       nextState.filteredSlices = Object.values(nextProps.slices)
         .filter(createFilter(this.state.searchTerm, KEYS_TO_FILTERS))
-        .sort(SliceAdder.sortByComparator(KEYS_TO_SORT[this.state.sortBy].key));
+        .sort(SliceAdder.sortByComparator(this.state.sortBy));
     }
 
     if (nextProps.selectedSliceIds !== this.props.selectedSliceIds) {
@@ -123,7 +125,7 @@ class SliceAdder extends React.Component {
   getFilteredSortedSlices(searchTerm, sortBy) {
     return Object.values(this.props.slices)
       .filter(createFilter(searchTerm, KEYS_TO_FILTERS))
-      .sort(SliceAdder.sortByComparator(KEYS_TO_SORT[sortBy].key));
+      .sort(SliceAdder.sortByComparator(sortBy));
   }
 
   handleKeyPress(ev) {
@@ -216,17 +218,17 @@ class SliceAdder extends React.Component {
             onKeyPress={this.handleKeyPress}
             data-test="dashboard-charts-filter-search-input"
           />
-          <DropdownButton
-            title={`Sort by ${KEYS_TO_SORT[this.state.sortBy].label}`}
-            onSelect={this.handleSelect}
+          <Select
             id="slice-adder-sortby"
+            defaultValue={DEFAULT_SORT_KEY}
+            onChange={this.handleSelect}
           >
-            {KEYS_TO_SORT.map((item, index) => (
-              <MenuItem key={item.key} eventKey={index}>
-                Sort by {item.label}
-              </MenuItem>
+            {Object.entries(KEYS_TO_SORT).map(([key, label]) => (
+              <Select.Option key={key} value={key}>
+                Sort by {label}
+              </Select.Option>
             ))}
-          </DropdownButton>
+          </Select>
         </div>
         {this.props.isLoading && <Loading />}
         {!this.props.isLoading && this.state.filteredSlices.length > 0 && (
diff --git a/superset-frontend/src/dashboard/components/SliceHeaderControls.jsx b/superset-frontend/src/dashboard/components/SliceHeaderControls.jsx
index 1778ef5..a453daa 100644
--- a/superset-frontend/src/dashboard/components/SliceHeaderControls.jsx
+++ b/superset-frontend/src/dashboard/components/SliceHeaderControls.jsx
@@ -19,8 +19,9 @@
 import React from 'react';
 import PropTypes from 'prop-types';
 import moment from 'moment';
-import { Dropdown, MenuItem } from 'react-bootstrap';
-import { t } from '@superset-ui/core';
+import { DropdownButton } from 'react-bootstrap';
+import { styled, t } from '@superset-ui/core';
+import { Menu } from 'src/common/components';
 import URLShortLinkModal from '../../components/URLShortLinkModal';
 import downloadAsImage from '../../utils/downloadAsImage';
 import getDashboardUrl from '../util/getDashboardUrl';
@@ -58,41 +59,49 @@ const defaultProps = {
   sliceCanEdit: false,
 };
 
+const MENU_KEYS = {
+  FORCE_REFRESH: 'force_refresh',
+  TOGGLE_CHART_DESCRIPTION: 'toggle_chart_description',
+  EXPLORE_CHART: 'explore_chart',
+  EXPORT_CSV: 'export_csv',
+  RESIZE_LABEL: 'resize_label',
+  SHARE_CHART: 'share_chart',
+  DOWNLOAD_AS_IMAGE: 'download_as_image',
+};
+
+const VerticalDotsContainer = styled.div`
+  padding: ${({ theme }) => theme.gridUnit / 4}px
+    ${({ theme }) => theme.gridUnit * 1.5}px;
+
+  .dot {
+    display: block;
+  }
+
+  &:hover {
+    cursor: pointer;
+  }
+`;
+
 const VerticalDotsTrigger = () => (
-  <div className="vertical-dots-container">
+  <VerticalDotsContainer>
     <span className="dot" />
     <span className="dot" />
     <span className="dot" />
-  </div>
+  </VerticalDotsContainer>
 );
 
 class SliceHeaderControls extends React.PureComponent {
   constructor(props) {
     super(props);
-    this.exportCSV = this.exportCSV.bind(this);
-    this.exploreChart = this.exploreChart.bind(this);
     this.toggleControls = this.toggleControls.bind(this);
     this.refreshChart = this.refreshChart.bind(this);
-    this.toggleExpandSlice = this.props.toggleExpandSlice.bind(
-      this,
-      this.props.slice.slice_id,
-    );
-
-    this.handleToggleFullSize = this.handleToggleFullSize.bind(this);
+    this.handleMenuClick = this.handleMenuClick.bind(this);
 
     this.state = {
       showControls: false,
     };
   }
 
-  exportCSV() {
-    this.props.exportCSV(this.props.slice.slice_id);
-  }
-
-  exploreChart() {
-    this.props.exploreChart(this.props.slice.slice_id);
-  }
-
   refreshChart() {
     if (this.props.updatedDttm) {
       this.props.forceRefresh(
@@ -108,8 +117,32 @@ class SliceHeaderControls extends React.PureComponent {
     }));
   }
 
-  handleToggleFullSize() {
-    this.props.handleToggleFullSize();
+  handleMenuClick({ key, domEvent }) {
+    switch (key) {
+      case MENU_KEYS.FORCE_REFRESH:
+        this.refreshChart();
+        break;
+      case MENU_KEYS.TOGGLE_CHART_DESCRIPTION:
+        this.props.toggleExpandSlice(this.props.slice.slice_id);
+        break;
+      case MENU_KEYS.EXPLORE_CHART:
+        this.props.exploreChart(this.props.slice.slice_id);
+        break;
+      case MENU_KEYS.EXPORT_CSV:
+        this.props.exportCSV(this.props.slice.slice_id);
+        break;
+      case MENU_KEYS.RESIZE_LABEL:
+        this.props.handleToggleFullSize();
+        break;
+      case MENU_KEYS.DOWNLOAD_AS_IMAGE:
+        downloadAsImage(
+          '.dashboard-component-chart-holder',
+          this.props.slice.slice_name,
+        )(domEvent);
+        break;
+      default:
+        break;
+    }
   }
 
   render() {
@@ -129,21 +162,21 @@ class SliceHeaderControls extends React.PureComponent {
       : (updatedWhen && t('Fetched %s', updatedWhen)) || '';
     const resizeLabel = isFullSize ? t('Minimize') : t('Maximize');
     return (
-      <Dropdown
+      <DropdownButton
         id={`slice_${slice.slice_id}-controls`}
         pullRight
+        noCaret
+        title={<VerticalDotsTrigger />}
+        style={{ padding: 0 }}
         // react-bootstrap handles visibility, but call toggle to force a re-render
         // and update the fetched/cached timestamps
         onToggle={this.toggleControls}
       >
-        <Dropdown.Toggle className="slice-header-controls-trigger" noCaret>
-          <VerticalDotsTrigger />
-        </Dropdown.Toggle>
-
-        <Dropdown.Menu>
-          <MenuItem
-            onClick={this.refreshChart}
+        <Menu onClick={this.handleMenuClick} selectable={false}>
+          <Menu.Item
+            key={MENU_KEYS.FORCE_REFRESH}
             disabled={this.props.chartStatus === 'loading'}
+            style={{ height: 'auto', lineHeight: 'initial' }}
           >
             {t('Force refresh')}
             <div
@@ -152,50 +185,46 @@ class SliceHeaderControls extends React.PureComponent {
             >
               {refreshTooltip}
             </div>
-          </MenuItem>
+          </Menu.Item>
 
-          <MenuItem divider />
+          <Menu.Divider />
 
           {slice.description && (
-            <MenuItem onClick={this.toggleExpandSlice}>
+            <Menu.Item key={MENU_KEYS.TOGGLE_CHART_DESCRIPTION}>
               {t('Toggle chart description')}
-            </MenuItem>
+            </Menu.Item>
           )}
 
           {this.props.supersetCanExplore && (
-            <MenuItem onClick={this.exploreChart}>
+            <Menu.Item key={MENU_KEYS.EXPLORE_CHART}>
               {t('Explore chart')}
-            </MenuItem>
+            </Menu.Item>
           )}
 
           {this.props.supersetCanCSV && (
-            <MenuItem onClick={this.exportCSV}>{t('Export CSV')}</MenuItem>
+            <Menu.Item key={MENU_KEYS.EXPORT_CSV}>{t('Export CSV')}</Menu.Item>
           )}
 
-          <MenuItem onClick={this.handleToggleFullSize}>{resizeLabel}</MenuItem>
-
-          <URLShortLinkModal
-            url={getDashboardUrl(
-              window.location.pathname,
-              getActiveFilters(),
-              componentId,
-            )}
-            addDangerToast={addDangerToast}
-            isMenuItem
-            title={t('Share chart')}
-            triggerNode={<span>{t('Share chart')}</span>}
-          />
-
-          <MenuItem
-            onClick={downloadAsImage(
-              '.dashboard-component-chart-holder',
-              slice.slice_name,
-            )}
-          >
+          <Menu.Item key={MENU_KEYS.RESIZE_LABEL}>{resizeLabel}</Menu.Item>
+
+          <Menu.Item key={MENU_KEYS.SHARE_CHART}>
+            <URLShortLinkModal
+              url={getDashboardUrl(
+                window.location.pathname,
+                getActiveFilters(),
+                componentId,
+              )}
+              addDangerToast={addDangerToast}
+              title={t('Share chart')}
+              triggerNode={<span>{t('Share chart')}</span>}
+            />
+          </Menu.Item>
+
+          <Menu.Item key={MENU_KEYS.DOWNLOAD_AS_IMAGE}>
             {t('Download as image')}
-          </MenuItem>
-        </Dropdown.Menu>
-      </Dropdown>
+          </Menu.Item>
+        </Menu>
+      </DropdownButton>
     );
   }
 }
diff --git a/superset-frontend/src/dashboard/stylesheets/components/chart.less b/superset-frontend/src/dashboard/stylesheets/components/chart.less
index b23ace5..ebf2e93 100644
--- a/superset-frontend/src/dashboard/stylesheets/components/chart.less
+++ b/superset-frontend/src/dashboard/stylesheets/components/chart.less
@@ -112,14 +112,6 @@
   }
 }
 
-.slice-header-controls-trigger {
-  padding: 2px 6px;
-
-  &:hover {
-    cursor: pointer;
-  }
-}
-
 .dot {
   @dot-diameter: 4px;
 
@@ -131,10 +123,6 @@
   background-color: @gray;
   display: inline-block;
 
-  .vertical-dots-container & {
-    display: block;
-  }
-
   a[role='menuitem'] & {
     width: 8px;
     height: 8px;
diff --git a/superset-frontend/src/dashboard/stylesheets/dashboard.less b/superset-frontend/src/dashboard/stylesheets/dashboard.less
index c86a7d9..02f718c 100644
--- a/superset-frontend/src/dashboard/stylesheets/dashboard.less
+++ b/superset-frontend/src/dashboard/stylesheets/dashboard.less
@@ -138,16 +138,8 @@ body {
     padding: 9px 0;
   }
 
-  .dropdown-menu li a {
-    padding: 3px 16px;
-    color: @almost-black;
-    font-size: @font-size-m;
-
-    &:hover,
-    &:focus {
-      background: @menu-hover;
-      color: @almost-black;
-    }
+  .dropdown-menu li {
+    font-weight: @font-weight-normal;
   }
 }
 
diff --git a/superset-frontend/src/explore/components/DisplayQueryButton.jsx b/superset-frontend/src/explore/components/DisplayQueryButton.jsx
index d97c8e6..0b4d993 100644
--- a/superset-frontend/src/explore/components/DisplayQueryButton.jsx
+++ b/superset-frontend/src/explore/components/DisplayQueryButton.jsx
@@ -26,16 +26,11 @@ import markdownSyntax from 'react-syntax-highlighter/dist/cjs/languages/hljs/mar
 import sqlSyntax from 'react-syntax-highlighter/dist/cjs/languages/hljs/sql';
 import jsonSyntax from 'react-syntax-highlighter/dist/cjs/languages/hljs/json';
 import github from 'react-syntax-highlighter/dist/cjs/styles/hljs/github';
-import {
-  DropdownButton,
-  MenuItem,
-  Row,
-  Col,
-  FormControl,
-} from 'react-bootstrap';
+import { DropdownButton, Row, Col, FormControl } from 'react-bootstrap';
 import { Table } from 'reactable-arc';
 import { t } from '@superset-ui/core';
 
+import { Menu } from 'src/common/components';
 import Button from 'src/components/Button';
 import getClientErrorObject from '../../utils/getClientErrorObject';
 import CopyToClipboard from '../../components/CopyToClipboard';
@@ -65,6 +60,12 @@ const propTypes = {
   slice: PropTypes.object,
 };
 
+const MENU_KEYS = {
+  EDIT_PROPERTIES: 'edit_properties',
+  RUN_IN_SQL_LAB: 'run_in_sql_lab',
+  DOWNLOAD_AS_IMAGE: 'download_as_image',
+};
+
 export class DisplayQueryButton extends React.PureComponent {
   constructor(props) {
     super(props);
@@ -81,8 +82,8 @@ export class DisplayQueryButton extends React.PureComponent {
     };
     this.beforeOpen = this.beforeOpen.bind(this);
     this.changeFilterText = this.changeFilterText.bind(this);
-    this.openPropertiesModal = this.openPropertiesModal.bind(this);
     this.closePropertiesModal = this.closePropertiesModal.bind(this);
+    this.handleMenuClick = this.handleMenuClick.bind(this);
   }
 
   beforeOpen(resultType) {
@@ -118,10 +119,6 @@ export class DisplayQueryButton extends React.PureComponent {
     this.setState({ filterText: event.target.value });
   }
 
-  redirectSQLLab() {
-    this.props.onOpenInEditor(this.props.latestQueryFormData);
-  }
-
   openPropertiesModal() {
     this.setState({ isPropertiesModalOpen: true });
   }
@@ -130,6 +127,35 @@ export class DisplayQueryButton extends React.PureComponent {
     this.setState({ isPropertiesModalOpen: false });
   }
 
+  handleMenuClick({ key, domEvent }) {
+    const {
+      chartHeight,
+      slice,
+      onOpenInEditor,
+      latestQueryFormData,
+    } = this.props;
+    switch (key) {
+      case MENU_KEYS.EDIT_PROPERTIES:
+        this.openPropertiesModal();
+        break;
+      case MENU_KEYS.RUN_IN_SQL_LAB:
+        onOpenInEditor(latestQueryFormData);
+        break;
+      case MENU_KEYS.DOWNLOAD_AS_IMAGE:
+        downloadAsImage(
+          '.chart-container',
+          // eslint-disable-next-line camelcase
+          slice?.slice_name ?? t('New chart'),
+          {
+            height: parseInt(chartHeight, 10),
+          },
+        )(domEvent);
+        break;
+      default:
+        break;
+    }
+  }
+
   renderQueryModalBody() {
     if (this.state.isLoading) {
       return <Loading />;
@@ -230,7 +256,7 @@ export class DisplayQueryButton extends React.PureComponent {
   }
 
   render() {
-    const { chartHeight, slice } = this.props;
+    const { slice } = this.props;
     return (
       <DropdownButton
         noCaret
@@ -245,62 +271,56 @@ export class DisplayQueryButton extends React.PureComponent {
         pullRight
         id="query"
       >
-        {slice && (
-          <>
-            <MenuItem onClick={this.openPropertiesModal}>
+        <Menu onClick={this.handleMenuClick} selectable={false}>
+          {slice && [
+            <Menu.Item key={MENU_KEYS.EDIT_PROPERTIES}>
               {t('Edit properties')}
-            </MenuItem>
+            </Menu.Item>,
             <PropertiesModal
               slice={slice}
               show={this.state.isPropertiesModalOpen}
               onHide={this.closePropertiesModal}
               onSave={this.props.sliceUpdated}
+            />,
+          ]}
+          <Menu.Item>
+            <ModalTrigger
+              triggerNode={
+                <span data-test="view-query-menu-item">{t('View query')}</span>
+              }
+              modalTitle={t('View query')}
+              beforeOpen={() => this.beforeOpen('query')}
+              modalBody={this.renderQueryModalBody()}
+              responsive
             />
-          </>
-        )}
-        <ModalTrigger
-          isMenuItem
-          triggerNode={
-            <span data-test="view-query-menu-item">{t('View query')}</span>
-          }
-          modalTitle={t('View query')}
-          beforeOpen={() => this.beforeOpen('query')}
-          modalBody={this.renderQueryModalBody()}
-          responsive
-        />
-        <ModalTrigger
-          isMenuItem
-          triggerNode={<span>{t('View results')}</span>}
-          modalTitle={t('View results')}
-          beforeOpen={() => this.beforeOpen('results')}
-          modalBody={this.renderResultsModalBody()}
-          responsive
-        />
-        <ModalTrigger
-          isMenuItem
-          triggerNode={<span>{t('View samples')}</span>}
-          modalTitle={t('View samples')}
-          beforeOpen={() => this.beforeOpen('samples')}
-          modalBody={this.renderSamplesModalBody()}
-          responsive
-        />
-        {this.state.sqlSupported && (
-          <MenuItem eventKey="3" onClick={this.redirectSQLLab.bind(this)}>
-            {t('Run in SQL Lab')}
-          </MenuItem>
-        )}
-        <MenuItem
-          onClick={downloadAsImage(
-            '.chart-container',
-            // eslint-disable-next-line camelcase
-            slice?.slice_name ?? t('New chart'),
-            {
-              height: parseInt(chartHeight, 10),
-            },
+          </Menu.Item>
+          <Menu.Item>
+            <ModalTrigger
+              triggerNode={<span>{t('View results')}</span>}
+              modalTitle={t('View results')}
+              beforeOpen={() => this.beforeOpen('results')}
+              modalBody={this.renderResultsModalBody()}
+              responsive
+            />
+          </Menu.Item>
+          <Menu.Item>
+            <ModalTrigger
+              triggerNode={<span>{t('View samples')}</span>}
+              modalTitle={t('View samples')}
+              beforeOpen={() => this.beforeOpen('samples')}
+              modalBody={this.renderSamplesModalBody()}
+              responsive
+            />
+          </Menu.Item>
+          {this.state.sqlSupported && (
+            <Menu.Item key={MENU_KEYS.RUN_IN_SQL_LAB}>
+              {t('Run in SQL Lab')}
+            </Menu.Item>
           )}
-        >
-          {t('Download as image')}
-        </MenuItem>
+          <Menu.Item key={MENU_KEYS.DOWNLOAD_AS_IMAGE}>
+            {t('Download as image')}
+          </Menu.Item>
+        </Menu>
       </DropdownButton>
     );
   }
diff --git a/superset-frontend/src/explore/components/controls/DateFilterControl.jsx b/superset-frontend/src/explore/components/controls/DateFilterControl.jsx
index 335f963..d596490 100644
--- a/superset-frontend/src/explore/components/controls/DateFilterControl.jsx
+++ b/superset-frontend/src/explore/components/controls/DateFilterControl.jsx
@@ -88,8 +88,6 @@ const FREEFORM_TOOLTIP = t(
     '`2 weeks from now` can be used.',
 );
 
-const DATE_FILTER_POPOVER_STYLE = { width: '250px' };
-
 const propTypes = {
   animation: PropTypes.bool,
   name: PropTypes.string.isRequired,
@@ -173,12 +171,34 @@ function getStateFromCustomRange(value) {
   };
 }
 
-const Styles = styled.div`
+const TimeFramesStyles = styled.div`
   .radio {
     margin: ${({ theme }) => theme.gridUnit}px 0;
   }
 `;
 
+const PopoverContentStyles = styled.div`
+  width: ${({ theme }) => theme.gridUnit * 60}px;
+
+  .timeframes-container {
+    margin-left: ${({ theme }) => theme.gridUnit * 2}px;
+  }
+
+  .relative-timerange-container {
+    display: flex;
+    margin-top: ${({ theme }) => theme.gridUnit * 2}px;
+  }
+
+  .timerange-input {
+    width: ${({ theme }) => theme.gridUnit * 15}px;
+    margin: 0 ${({ theme }) => theme.gridUnit}px;
+  }
+
+  .datetime {
+    margin: ${({ theme }) => theme.gridUnit}px 0;
+  }
+`;
+
 class DateFilterControl extends React.Component {
   constructor(props) {
     super(props);
@@ -399,7 +419,7 @@ class DateFilterControl extends React.Component {
       const timeRange = buildTimeRangeString(nextState.since, nextState.until);
 
       return (
-        <Styles key={timeFrame}>
+        <TimeFramesStyles key={timeFrame}>
           <OverlayTrigger
             key={timeFrame}
             placement="right"
@@ -419,14 +439,13 @@ class DateFilterControl extends React.Component {
               </Radio>
             </div>
           </OverlayTrigger>
-        </Styles>
+        </TimeFramesStyles>
       );
     });
 
     return (
-      <div
+      <PopoverContentStyles
         id="filter-popover"
-        style={DATE_FILTER_POPOVER_STYLE}
         ref={ref => {
           this.popoverContainer = ref;
         }}
@@ -438,7 +457,7 @@ class DateFilterControl extends React.Component {
           onSelect={this.changeTab}
         >
           <Tabs.TabPane key="1" tab="Defaults" forceRender>
-            <div style={{ marginLeft: '8px' }}>
+            <div className="timeframes-container">
               <FormGroup>{timeFrames}</FormGroup>
             </div>
           </Tabs.TabPane>
@@ -449,48 +468,37 @@ class DateFilterControl extends React.Component {
                 isSelected={this.state.type === TYPES.CUSTOM_RANGE}
                 onSelect={this.setTypeCustomRange}
               >
-                <div
-                  className="clearfix centered"
-                  style={{ marginTop: '8px', display: 'flex' }}
-                >
-                  <div className="input-inline">
-                    <Select
-                      value={this.state.rel}
-                      onSelect={value => this.setCustomRange('rel', value)}
-                      onFocus={this.setTypeCustomRange}
-                    >
-                      <Select.Option value={RELATIVE_TIME_OPTIONS.LAST}>
-                        Last
-                      </Select.Option>
-                      <Select.Option value={RELATIVE_TIME_OPTIONS.NEXT}>
-                        Next
-                      </Select.Option>
-                    </Select>
-                  </div>
-                  <div
-                    style={{ width: '60px' }}
-                    className="input-inline m-l-5 m-r-3"
+                <div className="relative-timerange-container clearfix centered">
+                  <Select
+                    value={this.state.rel}
+                    onSelect={value => this.setCustomRange('rel', value)}
+                    onFocus={this.setTypeCustomRange}
                   >
-                    <Input
-                      type="text"
-                      onChange={event =>
-                        this.setCustomRange('num', event.target.value)
-                      }
-                      onFocus={this.setTypeCustomRange}
-                      onPressEnter={this.close}
-                      value={this.state.num}
-                    />
-                  </div>
-                  <div className="input-inline">
-                    <Select
-                      value={this.state.grain}
-                      onFocus={this.setTypeCustomRange}
-                      onSelect={value => this.setCustomRange('grain', value)}
-                      dropdownMatchSelectWidth={false}
-                    >
-                      {grainOptions}
-                    </Select>
-                  </div>
+                    <Select.Option value={RELATIVE_TIME_OPTIONS.LAST}>
+                      Last
+                    </Select.Option>
+                    <Select.Option value={RELATIVE_TIME_OPTIONS.NEXT}>
+                      Next
+                    </Select.Option>
+                  </Select>
+                  <Input
+                    className="timerange-input"
+                    type="text"
+                    onChange={event =>
+                      this.setCustomRange('num', event.target.value)
+                    }
+                    onFocus={this.setTypeCustomRange}
+                    onPressEnter={this.close}
+                    value={this.state.num}
+                  />
+                  <Select
+                    value={this.state.grain}
+                    onFocus={this.setTypeCustomRange}
+                    onSelect={value => this.setCustomRange('grain', value)}
+                    dropdownMatchSelectWidth={false}
+                  >
+                    {grainOptions}
+                  </Select>
                 </div>
               </PopoverSection>
               <PopoverSection
@@ -505,48 +513,42 @@ class DateFilterControl extends React.Component {
                   }}
                 >
                   <InputGroup data-test="date-input-group">
-                    <div style={{ margin: '5px 0' }}>
-                      <Datetime
-                        inputProps={{ 'data-test': 'date-from-input' }}
-                        value={this.state.since}
-                        defaultValue={this.state.since}
-                        viewDate={this.state.since}
-                        onChange={value =>
-                          this.setCustomStartEnd('since', value)
-                        }
-                        isValidDate={this.isValidSince}
-                        onClick={this.setTypeCustomStartEnd}
-                        renderInput={props =>
-                          this.renderInput(props, 'showSinceCalendar')
-                        }
-                        open={this.state.showSinceCalendar}
-                        viewMode={this.state.sinceViewMode}
-                        onViewModeChange={sinceViewMode =>
-                          this.setState({ sinceViewMode })
-                        }
-                      />
-                    </div>
-                    <div style={{ margin: '5px 0' }}>
-                      <Datetime
-                        inputProps={{ 'data-test': 'date-to-input' }}
-                        value={this.state.until}
-                        defaultValue={this.state.until}
-                        viewDate={this.state.until}
-                        onChange={value =>
-                          this.setCustomStartEnd('until', value)
-                        }
-                        isValidDate={this.isValidUntil}
-                        onClick={this.setTypeCustomStartEnd}
-                        renderInput={props =>
-                          this.renderInput(props, 'showUntilCalendar')
-                        }
-                        open={this.state.showUntilCalendar}
-                        viewMode={this.state.untilViewMode}
-                        onViewModeChange={untilViewMode =>
-                          this.setState({ untilViewMode })
-                        }
-                      />
-                    </div>
+                    <Datetime
+                      className="datetime"
+                      inputProps={{ 'data-test': 'date-from-input' }}
+                      value={this.state.since}
+                      defaultValue={this.state.since}
+                      viewDate={this.state.since}
+                      onChange={value => this.setCustomStartEnd('since', value)}
+                      isValidDate={this.isValidSince}
+                      onClick={this.setTypeCustomStartEnd}
+                      renderInput={props =>
+                        this.renderInput(props, 'showSinceCalendar')
+                      }
+                      open={this.state.showSinceCalendar}
+                      viewMode={this.state.sinceViewMode}
+                      onViewModeChange={sinceViewMode =>
+                        this.setState({ sinceViewMode })
+                      }
+                    />
+                    <Datetime
+                      className="datetime"
+                      inputProps={{ 'data-test': 'date-to-input' }}
+                      value={this.state.until}
+                      defaultValue={this.state.until}
+                      viewDate={this.state.until}
+                      onChange={value => this.setCustomStartEnd('until', value)}
+                      isValidDate={this.isValidUntil}
+                      onClick={this.setTypeCustomStartEnd}
+                      renderInput={props =>
+                        this.renderInput(props, 'showUntilCalendar')
+                      }
+                      open={this.state.showUntilCalendar}
+                      viewMode={this.state.untilViewMode}
+                      onViewModeChange={untilViewMode =>
+                        this.setState({ untilViewMode })
+                      }
+                    />
                   </InputGroup>
                 </div>
               </PopoverSection>
@@ -564,7 +566,7 @@ class DateFilterControl extends React.Component {
             Ok
           </Button>
         </div>
-      </div>
+      </PopoverContentStyles>
     );
   }