From e974a23f90af3475025602cefc19dfda4037a0b0 Mon Sep 17 00:00:00 2001 From: Kim Truong <47833996+khtruong@users.noreply.github.com> Date: Fri, 29 Mar 2019 06:35:22 -0700 Subject: [PATCH] [Lyft-GA] Enable color consistency in a dashboard (#7135) * Enable color consistency in a dashboard Moved actions, minor UI, allowed dashboard copy Fix linting errors Undo unintentional change Updated and added unit tests Fail quietly if package has not been updated Fail quietly on dashboard copy if package is old * Update packages * Remove unnecessary code * Addressed Grace's comments * Small fix for item key * Reset chart's color during exploration * Do not reset chart form data when exploring chart --- superset/assets/package-lock.json | 6 +- .../components/DashboardBuilder_spec.jsx | 26 +++- .../dashboard/components/Header_spec.jsx | 8 +- .../dashboard/fixtures/mockDashboardState.js | 3 +- .../dashboard/reducers/dashboardState_spec.js | 25 ++-- superset/assets/src/chart/ChartRenderer.jsx | 3 +- .../src/dashboard/actions/dashboardLayout.js | 3 +- .../src/dashboard/actions/dashboardState.js | 18 ++- .../components/BuilderComponentPane.jsx | 112 +++++------------ .../components/ColorComponentPane.jsx | 107 ++++++++++++++++ .../dashboard/components/DashboardBuilder.jsx | 26 +++- .../src/dashboard/components/Header.jsx | 96 ++++++++++---- .../components/HeaderActionsDropdown.jsx | 11 +- .../components/InsertComponentPane.jsx | 118 ++++++++++++++++++ .../src/dashboard/components/SaveModal.jsx | 15 +++ .../assets/src/dashboard/containers/Chart.jsx | 3 +- .../dashboard/containers/DashboardBuilder.jsx | 11 +- .../dashboard/containers/DashboardHeader.jsx | 8 +- .../src/dashboard/reducers/dashboardState.js | 21 +++- .../src/dashboard/reducers/getInitialState.js | 20 ++- .../stylesheets/builder-sidepane.less | 14 +++ .../src/dashboard/stylesheets/dashboard.less | 9 +- .../charts/getFormDataWithExtraFilters.js | 4 + .../assets/src/dashboard/util/constants.js | 7 ++ .../assets/src/dashboard/util/propShapes.jsx | 5 +- .../controls/ColorSchemeControl.jsx | 28 +++-- superset/views/core.py | 6 + tests/dashboard_tests.py | 43 +++++++ 28 files changed, 586 insertions(+), 170 deletions(-) create mode 100644 superset/assets/src/dashboard/components/ColorComponentPane.jsx create mode 100644 superset/assets/src/dashboard/components/InsertComponentPane.jsx diff --git a/superset/assets/package-lock.json b/superset/assets/package-lock.json index 17a4f3356c..18c3acf1ca 100644 --- a/superset/assets/package-lock.json +++ b/superset/assets/package-lock.json @@ -2232,9 +2232,9 @@ } }, "@superset-ui/color": { - "version": "0.10.1", - "resolved": "https://registry.npmjs.org/@superset-ui/color/-/color-0.10.1.tgz", - "integrity": "sha512-GblA9+h947un4K6s6v3uRTYCDEBi8GAp3wyEHVXfhSv/YXwyzpyhvhXoF8APSz+8cDVkKYY2svZVOALE0QDI1Q==", + "version": "0.10.8", + "resolved": "https://registry.npmjs.org/@superset-ui/color/-/color-0.10.8.tgz", + "integrity": "sha512-H1M8V9OKO3fCmOHQvW1rN9pRw2t/L1LKHvxzEj/Kccw+osckdmF8RtKEp7DaBuKMO6PF2Kq2FWNIiqNtin9whA==", "requires": { "@types/d3-scale": "^2.0.2", "d3-scale": "^2.1.2" diff --git a/superset/assets/spec/javascripts/dashboard/components/DashboardBuilder_spec.jsx b/superset/assets/spec/javascripts/dashboard/components/DashboardBuilder_spec.jsx index bf24644e14..16dc33dea0 100644 --- a/superset/assets/spec/javascripts/dashboard/components/DashboardBuilder_spec.jsx +++ b/superset/assets/spec/javascripts/dashboard/components/DashboardBuilder_spec.jsx @@ -31,6 +31,7 @@ import DashboardComponent from '../../../../src/dashboard/containers/DashboardCo import DashboardHeader from '../../../../src/dashboard/containers/DashboardHeader'; import DashboardGrid from '../../../../src/dashboard/containers/DashboardGrid'; import * as dashboardStateActions from '../../../../src/dashboard/actions/dashboardState'; +import { BUILDER_PANE_TYPE } from '../../../../src/dashboard/util/constants'; import WithDragDropContext from '../helpers/WithDragDropContext'; import { @@ -61,7 +62,10 @@ describe('DashboardBuilder', () => { dashboardLayout, deleteTopLevelTabs() {}, editMode: false, - showBuilderPane: false, + showBuilderPane() {}, + builderPaneType: BUILDER_PANE_TYPE.NONE, + setColorSchemeAndUnsavedChanges() {}, + colorScheme: undefined, handleComponentDrop() {}, toggleBuilderPane() {}, }; @@ -143,11 +147,27 @@ describe('DashboardBuilder', () => { expect(parentSize.find(DashboardGrid)).toHaveLength(expectedCount); }); - it('should render a BuilderComponentPane if editMode=showBuilderPane=true', () => { + it('should render a BuilderComponentPane if editMode=true and user selects "Insert Components" pane', () => { const wrapper = setup(); expect(wrapper.find(BuilderComponentPane)).toHaveLength(0); - wrapper.setProps({ ...props, editMode: true, showBuilderPane: true }); + wrapper.setProps({ + ...props, + editMode: true, + builderPaneType: BUILDER_PANE_TYPE.ADD_COMPONENTS, + }); + expect(wrapper.find(BuilderComponentPane)).toHaveLength(1); + }); + + it('should render a BuilderComponentPane if editMode=true and user selects "Colors" pane', () => { + const wrapper = setup(); + expect(wrapper.find(BuilderComponentPane)).toHaveLength(0); + + wrapper.setProps({ + ...props, + editMode: true, + builderPaneType: BUILDER_PANE_TYPE.COLORS, + }); expect(wrapper.find(BuilderComponentPane)).toHaveLength(1); }); diff --git a/superset/assets/spec/javascripts/dashboard/components/Header_spec.jsx b/superset/assets/spec/javascripts/dashboard/components/Header_spec.jsx index 2fff4638ca..69e57da8df 100644 --- a/superset/assets/spec/javascripts/dashboard/components/Header_spec.jsx +++ b/superset/assets/spec/javascripts/dashboard/components/Header_spec.jsx @@ -24,6 +24,7 @@ import FaveStar from '../../../../src/components/FaveStar'; import HeaderActionsDropdown from '../../../../src/dashboard/components/HeaderActionsDropdown'; import Button from '../../../../src/components/Button'; import UndoRedoKeylisteners from '../../../../src/dashboard/components/UndoRedoKeylisteners'; +import { BUILDER_PANE_TYPE } from '../../../../src/dashboard/util/constants'; describe('Header', () => { const props = { @@ -46,7 +47,8 @@ describe('Header', () => { updateDashboardTitle: () => {}, editMode: false, setEditMode: () => {}, - showBuilderPane: false, + showBuilderPane: () => {}, + builderPaneType: BUILDER_PANE_TYPE.NONE, toggleBuilderPane: () => {}, updateCss: () => {}, hasUnsavedChanges: false, @@ -150,9 +152,9 @@ describe('Header', () => { expect(wrapper.find(HeaderActionsDropdown)).toHaveLength(1); }); - it('should render four Buttons', () => { + it('should render five Buttons', () => { const wrapper = setup(overrideProps); - expect(wrapper.find(Button)).toHaveLength(4); + expect(wrapper.find(Button)).toHaveLength(5); }); it('should set up undo/redo', () => { diff --git a/superset/assets/spec/javascripts/dashboard/fixtures/mockDashboardState.js b/superset/assets/spec/javascripts/dashboard/fixtures/mockDashboardState.js index f326a76ee6..3763ef41a4 100644 --- a/superset/assets/spec/javascripts/dashboard/fixtures/mockDashboardState.js +++ b/superset/assets/spec/javascripts/dashboard/fixtures/mockDashboardState.js @@ -17,6 +17,7 @@ * under the License. */ import { id as sliceId } from './mockChartQueries'; +import { BUILDER_PANE_TYPE } from '../../../../src/dashboard/util/constants'; export default { sliceIds: [sliceId], @@ -24,7 +25,7 @@ export default { filters: {}, expandedSlices: {}, editMode: false, - showBuilderPane: false, + builderPaneType: BUILDER_PANE_TYPE.NONE, hasUnsavedChanges: false, maxUndoHistoryExceeded: false, isStarred: true, diff --git a/superset/assets/spec/javascripts/dashboard/reducers/dashboardState_spec.js b/superset/assets/spec/javascripts/dashboard/reducers/dashboardState_spec.js index c3e385580a..dadcf06c8b 100644 --- a/superset/assets/spec/javascripts/dashboard/reducers/dashboardState_spec.js +++ b/superset/assets/spec/javascripts/dashboard/reducers/dashboardState_spec.js @@ -25,12 +25,12 @@ import { SET_EDIT_MODE, SET_MAX_UNDO_HISTORY_EXCEEDED, SET_UNSAVED_CHANGES, - TOGGLE_BUILDER_PANE, TOGGLE_EXPAND_SLICE, TOGGLE_FAVE_STAR, } from '../../../../src/dashboard/actions/dashboardState'; import dashboardStateReducer from '../../../../src/dashboard/reducers/dashboardState'; +import { BUILDER_PANE_TYPE } from '../../../../src/dashboard/util/constants'; describe('dashboardState reducer', () => { it('should return initial state', () => { @@ -79,23 +79,10 @@ describe('dashboardState reducer', () => { { editMode: false }, { type: SET_EDIT_MODE, editMode: true }, ), - ).toEqual({ editMode: true, showBuilderPane: true }); - }); - - it('should toggle builder pane', () => { - expect( - dashboardStateReducer( - { showBuilderPane: false }, - { type: TOGGLE_BUILDER_PANE }, - ), - ).toEqual({ showBuilderPane: true }); - - expect( - dashboardStateReducer( - { showBuilderPane: true }, - { type: TOGGLE_BUILDER_PANE }, - ), - ).toEqual({ showBuilderPane: false }); + ).toEqual({ + editMode: true, + builderPaneType: BUILDER_PANE_TYPE.ADD_COMPONENTS, + }); }); it('should toggle expanded slices', () => { @@ -150,6 +137,8 @@ describe('dashboardState reducer', () => { hasUnsavedChanges: false, maxUndoHistoryExceeded: false, editMode: false, + builderPaneType: BUILDER_PANE_TYPE.NONE, + updatedColorScheme: false, }); }); diff --git a/superset/assets/src/chart/ChartRenderer.jsx b/superset/assets/src/chart/ChartRenderer.jsx index e0a01f14cf..e46f941ffd 100644 --- a/superset/assets/src/chart/ChartRenderer.jsx +++ b/superset/assets/src/chart/ChartRenderer.jsx @@ -87,7 +87,8 @@ class ChartRenderer extends React.Component { nextProps.height !== this.props.height || nextProps.width !== this.props.width || nextState.tooltip !== this.state.tooltip || - nextProps.triggerRender) { + nextProps.triggerRender || + nextProps.formData.color_scheme !== this.props.formData.color_scheme) { return true; } } diff --git a/superset/assets/src/dashboard/actions/dashboardLayout.js b/superset/assets/src/dashboard/actions/dashboardLayout.js index 5b163d9619..2716b006eb 100644 --- a/superset/assets/src/dashboard/actions/dashboardLayout.js +++ b/superset/assets/src/dashboard/actions/dashboardLayout.js @@ -209,7 +209,8 @@ export function undoLayoutAction() { if ( dashboardLayout.past.length === 0 && - !dashboardState.maxUndoHistoryExceeded + !dashboardState.maxUndoHistoryExceeded && + !dashboardState.updatedColorScheme ) { dispatch(setUnsavedChanges(false)); } diff --git a/superset/assets/src/dashboard/actions/dashboardState.js b/superset/assets/src/dashboard/actions/dashboardState.js index 0864819196..5a036449bb 100644 --- a/superset/assets/src/dashboard/actions/dashboardState.js +++ b/superset/assets/src/dashboard/actions/dashboardState.js @@ -225,9 +225,9 @@ export function startPeriodicRender(interval) { }; } -export const TOGGLE_BUILDER_PANE = 'TOGGLE_BUILDER_PANE'; -export function toggleBuilderPane() { - return { type: TOGGLE_BUILDER_PANE }; +export const SHOW_BUILDER_PANE = 'SHOW_BUILDER_PANE'; +export function showBuilderPane(builderPaneType) { + return { type: SHOW_BUILDER_PANE, builderPaneType }; } export function addSliceToDashboard(id) { @@ -266,6 +266,18 @@ export function removeSliceFromDashboard(id) { }; } +export const SET_COLOR_SCHEME = 'SET_COLOR_SCHEME'; +export function setColorScheme(colorScheme) { + return { type: SET_COLOR_SCHEME, colorScheme }; +} + +export function setColorSchemeAndUnsavedChanges(colorScheme) { + return dispatch => { + dispatch(setColorScheme(colorScheme)); + dispatch(setUnsavedChanges(true)); + }; +} + // Undo history --------------------------------------------------------------- export const SET_MAX_UNDO_HISTORY_EXCEEDED = 'SET_MAX_UNDO_HISTORY_EXCEEDED'; export function setMaxUndoHistoryExceeded(maxUndoHistoryExceeded = true) { diff --git a/superset/assets/src/dashboard/components/BuilderComponentPane.jsx b/superset/assets/src/dashboard/components/BuilderComponentPane.jsx index 4c2e92ce34..2d2ab08993 100644 --- a/superset/assets/src/dashboard/components/BuilderComponentPane.jsx +++ b/superset/assets/src/dashboard/components/BuilderComponentPane.jsx @@ -19,49 +19,37 @@ /* eslint-env browser */ import PropTypes from 'prop-types'; import React from 'react'; -import cx from 'classnames'; import { StickyContainer, Sticky } from 'react-sticky'; import { ParentSize } from '@vx/responsive'; -import { t } from '@superset-ui/translation'; -import NewColumn from './gridComponents/new/NewColumn'; -import NewDivider from './gridComponents/new/NewDivider'; -import NewHeader from './gridComponents/new/NewHeader'; -import NewRow from './gridComponents/new/NewRow'; -import NewTabs from './gridComponents/new/NewTabs'; -import NewMarkdown from './gridComponents/new/NewMarkdown'; -import SliceAdder from '../containers/SliceAdder'; - -const SUPERSET_HEADER_HEIGHT = 59; +import InsertComponentPane, { + SUPERSET_HEADER_HEIGHT, +} from './InsertComponentPane'; +import ColorComponentPane from './ColorComponentPane'; +import { BUILDER_PANE_TYPE } from '../util/constants'; const propTypes = { topOffset: PropTypes.number, - toggleBuilderPane: PropTypes.func.isRequired, + showBuilderPane: PropTypes.func.isRequired, + builderPaneType: PropTypes.string.isRequired, + setColorSchemeAndUnsavedChanges: PropTypes.func.isRequired, + colorScheme: PropTypes.string, }; const defaultProps = { topOffset: 0, + colorScheme: undefined, }; class BuilderComponentPane extends React.PureComponent { - constructor(props) { - super(props); - this.state = { - slideDirection: 'slide-out', - }; - - this.openSlicesPane = this.slide.bind(this, 'slide-in'); - this.closeSlicesPane = this.slide.bind(this, 'slide-out'); - } - - slide(direction) { - this.setState({ - slideDirection: direction, - }); - } - render() { - const { topOffset } = this.props; + const { + topOffset, + builderPaneType, + showBuilderPane, + setColorSchemeAndUnsavedChanges, + colorScheme, + } = this.props; return (
-
-
-
- {t('Insert components')} - -
-
-
-
- {t('Your charts & filters')} -
- - -
- - - - - - -
-
-
- - {t('Your charts and filters')} -
- -
-
+ {builderPaneType === BUILDER_PANE_TYPE.ADD_COMPONENTS && ( + + )} + {builderPaneType === BUILDER_PANE_TYPE.COLORS && ( + + )}
)} diff --git a/superset/assets/src/dashboard/components/ColorComponentPane.jsx b/superset/assets/src/dashboard/components/ColorComponentPane.jsx new file mode 100644 index 0000000000..ee6aec5852 --- /dev/null +++ b/superset/assets/src/dashboard/components/ColorComponentPane.jsx @@ -0,0 +1,107 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +/* eslint-env browser */ +import PropTypes from 'prop-types'; +import React from 'react'; +import { getCategoricalSchemeRegistry } from '@superset-ui/color'; +import { t } from '@superset-ui/translation'; + +import ColorSchemeControl from '../../explore/components/controls/ColorSchemeControl'; +import { BUILDER_PANE_TYPE } from '../util/constants'; + +const propTypes = { + showBuilderPane: PropTypes.func.isRequired, + setColorSchemeAndUnsavedChanges: PropTypes.func.isRequired, + colorScheme: PropTypes.string, +}; + +const defaultProps = { + colorScheme: undefined, +}; + +class ColorComponentPane extends React.PureComponent { + constructor(props) { + super(props); + this.state = { hovered: false }; + this.categoricalSchemeRegistry = getCategoricalSchemeRegistry(); + this.getChoices = this.getChoices.bind(this); + this.getSchemes = this.getSchemes.bind(this); + this.onCloseButtonClick = this.onCloseButtonClick.bind(this); + this.onMouseEnter = this.setHover.bind(this, true); + this.onMouseLeave = this.setHover.bind(this, false); + } + + onCloseButtonClick() { + this.props.showBuilderPane(BUILDER_PANE_TYPE.NONE); + } + + getChoices() { + return this.categoricalSchemeRegistry.keys().map(s => [s, s]); + } + + getSchemes() { + return this.categoricalSchemeRegistry.getMap(); + } + + setHover(hovered) { + this.setState({ hovered }); + } + + render() { + const { setColorSchemeAndUnsavedChanges, colorScheme } = this.props; + + return ( +
+
+
+ {'Color Settings'} + +
+
+ +
+
+
+ ); + } +} + +ColorComponentPane.propTypes = propTypes; +ColorComponentPane.defaultProps = defaultProps; + +export default ColorComponentPane; diff --git a/superset/assets/src/dashboard/components/DashboardBuilder.jsx b/superset/assets/src/dashboard/components/DashboardBuilder.jsx index e635f902dd..7bd6f6f39b 100644 --- a/superset/assets/src/dashboard/components/DashboardBuilder.jsx +++ b/superset/assets/src/dashboard/components/DashboardBuilder.jsx @@ -38,6 +38,7 @@ import WithPopoverMenu from './menu/WithPopoverMenu'; import getDragDropManager from '../util/getDragDropManager'; import { + BUILDER_PANE_TYPE, DASHBOARD_GRID_ID, DASHBOARD_ROOT_ID, DASHBOARD_ROOT_DEPTH, @@ -51,13 +52,15 @@ const propTypes = { dashboardLayout: PropTypes.object.isRequired, deleteTopLevelTabs: PropTypes.func.isRequired, editMode: PropTypes.bool.isRequired, - showBuilderPane: PropTypes.bool, + showBuilderPane: PropTypes.func.isRequired, + builderPaneType: PropTypes.string.isRequired, + setColorSchemeAndUnsavedChanges: PropTypes.func.isRequired, + colorScheme: PropTypes.string, handleComponentDrop: PropTypes.func.isRequired, - toggleBuilderPane: PropTypes.func.isRequired, }; const defaultProps = { - showBuilderPane: false, + colorScheme: undefined, }; class DashboardBuilder extends React.Component { @@ -102,7 +105,15 @@ class DashboardBuilder extends React.Component { } render() { - const { handleComponentDrop, dashboardLayout, editMode } = this.props; + const { + handleComponentDrop, + dashboardLayout, + editMode, + showBuilderPane, + builderPaneType, + setColorSchemeAndUnsavedChanges, + colorScheme, + } = this.props; const { tabIndex } = this.state; const dashboardRoot = dashboardLayout[DASHBOARD_ROOT_ID]; const rootChildId = dashboardRoot.children[0]; @@ -202,10 +213,13 @@ class DashboardBuilder extends React.Component { )}
- {this.props.editMode && this.props.showBuilderPane && ( + {editMode && builderPaneType !== BUILDER_PANE_TYPE.NONE && ( )} diff --git a/superset/assets/src/dashboard/components/Header.jsx b/superset/assets/src/dashboard/components/Header.jsx index 796a2df09a..366efbe3d8 100644 --- a/superset/assets/src/dashboard/components/Header.jsx +++ b/superset/assets/src/dashboard/components/Header.jsx @@ -19,6 +19,7 @@ /* eslint-env browser */ import React from 'react'; import PropTypes from 'prop-types'; +import { CategoricalColorNamespace } from '@superset-ui/color'; import { t } from '@superset-ui/translation'; import HeaderActionsDropdown from './HeaderActionsDropdown'; @@ -29,6 +30,7 @@ import UndoRedoKeylisteners from './UndoRedoKeylisteners'; import { chartPropShape } from '../util/propShapes'; import { + BUILDER_PANE_TYPE, UNDO_LIMIT, SAVE_TYPE_OVERWRITE, DASHBOARD_POSITION_DATA_LIMIT, @@ -52,6 +54,8 @@ const propTypes = { filters: PropTypes.object.isRequired, expandedSlices: PropTypes.object.isRequired, css: PropTypes.string.isRequired, + colorNamespace: PropTypes.string, + colorScheme: PropTypes.string, isStarred: PropTypes.bool.isRequired, isLoading: PropTypes.bool.isRequired, onSave: PropTypes.func.isRequired, @@ -63,8 +67,8 @@ const propTypes = { updateDashboardTitle: PropTypes.func.isRequired, editMode: PropTypes.bool.isRequired, setEditMode: PropTypes.func.isRequired, - showBuilderPane: PropTypes.bool.isRequired, - toggleBuilderPane: PropTypes.func.isRequired, + showBuilderPane: PropTypes.func.isRequired, + builderPaneType: PropTypes.string.isRequired, updateCss: PropTypes.func.isRequired, logEvent: PropTypes.func.isRequired, hasUnsavedChanges: PropTypes.bool.isRequired, @@ -81,6 +85,11 @@ const propTypes = { setRefreshFrequency: PropTypes.func.isRequired, }; +const defaultProps = { + colorNamespace: undefined, + colorScheme: undefined, +}; + class Header extends React.PureComponent { static discardChanges() { window.location.reload(); @@ -96,6 +105,10 @@ class Header extends React.PureComponent { this.handleChangeText = this.handleChangeText.bind(this); this.handleCtrlZ = this.handleCtrlZ.bind(this); this.handleCtrlY = this.handleCtrlY.bind(this); + this.onInsertComponentsButtonClick = this.onInsertComponentsButtonClick.bind( + this, + ); + this.onColorsButtonClick = this.onColorsButtonClick.bind(this); this.toggleEditMode = this.toggleEditMode.bind(this); this.forceRefresh = this.forceRefresh.bind(this); this.startPeriodicRender = this.startPeriodicRender.bind(this); @@ -128,25 +141,12 @@ class Header extends React.PureComponent { clearTimeout(this.ctrlZTimeout); } - forceRefresh() { - if (!this.props.isLoading) { - const chartList = Object.values(this.props.charts); - this.props.logEvent(LOG_ACTIONS_FORCE_REFRESH_DASHBOARD, { - force: true, - interval: 0, - chartCount: chartList.length, - }); - return this.props.fetchCharts(chartList, true); - } - return false; + onInsertComponentsButtonClick() { + this.props.showBuilderPane(BUILDER_PANE_TYPE.ADD_COMPONENTS); } - startPeriodicRender(interval) { - this.props.logEvent(LOG_ACTIONS_PERIODIC_RENDER_DASHBOARD, { - force: true, - interval, - }); - return this.props.startPeriodicRender(interval); + onColorsButtonClick() { + this.props.showBuilderPane(BUILDER_PANE_TYPE.COLORS); } handleChangeText(nextText) { @@ -177,6 +177,27 @@ class Header extends React.PureComponent { }); } + forceRefresh() { + if (!this.props.isLoading) { + const chartList = Object.values(this.props.charts); + this.props.logEvent(LOG_ACTIONS_FORCE_REFRESH_DASHBOARD, { + force: true, + interval: 0, + chartCount: chartList.length, + }); + return this.props.fetchCharts(chartList, true); + } + return false; + } + + startPeriodicRender(interval) { + this.props.logEvent(LOG_ACTIONS_PERIODIC_RENDER_DASHBOARD, { + force: true, + interval, + }); + return this.props.startPeriodicRender(interval); + } + toggleEditMode() { this.props.logEvent(LOG_ACTIONS_TOGGLE_EDIT_DASHBOARD, { edit_mode: !this.props.editMode, @@ -190,14 +211,24 @@ class Header extends React.PureComponent { layout: positions, expandedSlices, css, + colorNamespace, + colorScheme, filters, dashboardInfo, } = this.props; + const scale = CategoricalColorNamespace.getScale( + colorScheme, + colorNamespace, + ); + const labelColors = scale.getColorMap(); const data = { positions, expanded_slices: expandedSlices, css, + color_namespace: colorNamespace, + color_scheme: colorScheme, + label_colors: labelColors, dashboard_title: dashboardTitle, default_filters: safeStringify(filters), }; @@ -229,6 +260,8 @@ class Header extends React.PureComponent { filters, expandedSlices, css, + colorNamespace, + colorScheme, onUndo, onRedo, undoLength, @@ -237,7 +270,7 @@ class Header extends React.PureComponent { onSave, updateCss, editMode, - showBuilderPane, + builderPaneType, dashboardInfo, hasUnsavedChanges, isLoading, @@ -294,10 +327,22 @@ class Header extends React.PureComponent { )} {editMode && ( - + )} + + {editMode && ( + )} @@ -351,6 +396,8 @@ class Header extends React.PureComponent { filters={filters} expandedSlices={expandedSlices} css={css} + colorNamespace={colorNamespace} + colorScheme={colorScheme} onSave={onSave} onChange={onChange} forceRefreshAllCharts={this.forceRefresh} @@ -371,5 +418,6 @@ class Header extends React.PureComponent { } Header.propTypes = propTypes; +Header.defaultProps = defaultProps; export default Header; diff --git a/superset/assets/src/dashboard/components/HeaderActionsDropdown.jsx b/superset/assets/src/dashboard/components/HeaderActionsDropdown.jsx index 4c36d9a3c6..5fc9d0ede8 100644 --- a/superset/assets/src/dashboard/components/HeaderActionsDropdown.jsx +++ b/superset/assets/src/dashboard/components/HeaderActionsDropdown.jsx @@ -37,6 +37,8 @@ const propTypes = { dashboardTitle: PropTypes.string.isRequired, hasUnsavedChanges: PropTypes.bool.isRequired, css: PropTypes.string.isRequired, + colorNamespace: PropTypes.string, + colorScheme: PropTypes.string, onChange: PropTypes.func.isRequired, updateCss: PropTypes.func.isRequired, forceRefreshAllCharts: PropTypes.func.isRequired, @@ -53,7 +55,10 @@ const propTypes = { onSave: PropTypes.func.isRequired, }; -const defaultProps = {}; +const defaultProps = { + colorNamespace: undefined, + colorScheme: undefined, +}; class HeaderActionsDropdown extends React.PureComponent { static discardChanges() { @@ -111,6 +116,8 @@ class HeaderActionsDropdown extends React.PureComponent { refreshFrequency, editMode, css, + colorNamespace, + colorScheme, hasUnsavedChanges, layout, filters, @@ -145,6 +152,8 @@ class HeaderActionsDropdown extends React.PureComponent { expandedSlices={expandedSlices} refreshFrequency={refreshFrequency} css={css} + colorNamespace={colorNamespace} + colorScheme={colorScheme} onSave={onSave} isMenuItem triggerNode={{t('Save as')}} diff --git a/superset/assets/src/dashboard/components/InsertComponentPane.jsx b/superset/assets/src/dashboard/components/InsertComponentPane.jsx new file mode 100644 index 0000000000..31413471f7 --- /dev/null +++ b/superset/assets/src/dashboard/components/InsertComponentPane.jsx @@ -0,0 +1,118 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +/* eslint-env browser */ +import PropTypes from 'prop-types'; +import React from 'react'; +import cx from 'classnames'; +import { t } from '@superset-ui/translation'; + +import NewColumn from './gridComponents/new/NewColumn'; +import NewDivider from './gridComponents/new/NewDivider'; +import NewHeader from './gridComponents/new/NewHeader'; +import NewRow from './gridComponents/new/NewRow'; +import NewTabs from './gridComponents/new/NewTabs'; +import NewMarkdown from './gridComponents/new/NewMarkdown'; +import SliceAdder from '../containers/SliceAdder'; +import { BUILDER_PANE_TYPE } from '../util/constants'; + +export const SUPERSET_HEADER_HEIGHT = 59; + +const propTypes = { + height: PropTypes.number.isRequired, + isSticky: PropTypes.bool.isRequired, + showBuilderPane: PropTypes.func.isRequired, +}; + +class InsertComponentPane extends React.PureComponent { + constructor(props) { + super(props); + this.state = { + slideDirection: 'slide-out', + }; + + this.onCloseButtonClick = this.onCloseButtonClick.bind(this); + this.openSlicesPane = this.slide.bind(this, 'slide-in'); + this.closeSlicesPane = this.slide.bind(this, 'slide-out'); + } + + onCloseButtonClick() { + this.props.showBuilderPane(BUILDER_PANE_TYPE.NONE); + } + + slide(direction) { + this.setState({ + slideDirection: direction, + }); + } + + render() { + return ( +
+
+
+ {t('Insert components')} + +
+
+
+
+ {t('Your charts & filters')} +
+ + +
+ + + + + + +
+
+
+ + {t('Your charts and filters')} +
+ +
+
+ ); + } +} + +InsertComponentPane.propTypes = propTypes; + +export default InsertComponentPane; diff --git a/superset/assets/src/dashboard/components/SaveModal.jsx b/superset/assets/src/dashboard/components/SaveModal.jsx index aa5436979c..1873f0c313 100644 --- a/superset/assets/src/dashboard/components/SaveModal.jsx +++ b/superset/assets/src/dashboard/components/SaveModal.jsx @@ -20,6 +20,7 @@ import React from 'react'; import PropTypes from 'prop-types'; import { Button, FormControl, FormGroup, Radio } from 'react-bootstrap'; +import { CategoricalColorNamespace } from '@superset-ui/color'; import { t } from '@superset-ui/translation'; import ModalTrigger from '../../components/ModalTrigger'; @@ -38,6 +39,8 @@ const propTypes = { triggerNode: PropTypes.node.isRequired, filters: PropTypes.object.isRequired, css: PropTypes.string.isRequired, + colorNamespace: PropTypes.string, + colorScheme: PropTypes.string, onSave: PropTypes.func.isRequired, isMenuItem: PropTypes.bool, canOverwrite: PropTypes.bool.isRequired, @@ -47,6 +50,8 @@ const propTypes = { const defaultProps = { isMenuItem: false, saveType: SAVE_TYPE_OVERWRITE, + colorNamespace: undefined, + colorScheme: undefined, }; class SaveModal extends React.PureComponent { @@ -93,15 +98,25 @@ class SaveModal extends React.PureComponent { dashboardTitle, layout: positions, css, + colorNamespace, + colorScheme, expandedSlices, filters, dashboardId, refreshFrequency, } = this.props; + const scale = CategoricalColorNamespace.getScale( + colorScheme, + colorNamespace, + ); + const labelColors = scale.getColorMap(); const data = { positions, css, + color_namespace: colorNamespace, + color_scheme: colorScheme, + label_colors: labelColors, expanded_slices: expandedSlices, dashboard_title: saveType === SAVE_TYPE_NEWDASHBOARD ? newDashName : dashboardTitle, diff --git a/superset/assets/src/dashboard/containers/Chart.jsx b/superset/assets/src/dashboard/containers/Chart.jsx index 1e0e64c602..c0926db0a9 100644 --- a/superset/assets/src/dashboard/containers/Chart.jsx +++ b/superset/assets/src/dashboard/containers/Chart.jsx @@ -43,7 +43,7 @@ function mapStateToProps( ) { const { id } = ownProps; const chart = chartQueries[id] || {}; - const { filters } = dashboardState; + const { filters, colorScheme } = dashboardState; return { chart, @@ -58,6 +58,7 @@ function mapStateToProps( chart, dashboardMetadata: dashboardInfo.metadata, filters, + colorScheme, sliceId: id, }), editMode: dashboardState.editMode, diff --git a/superset/assets/src/dashboard/containers/DashboardBuilder.jsx b/superset/assets/src/dashboard/containers/DashboardBuilder.jsx index 3ca70431a4..3c6514a2f7 100644 --- a/superset/assets/src/dashboard/containers/DashboardBuilder.jsx +++ b/superset/assets/src/dashboard/containers/DashboardBuilder.jsx @@ -20,7 +20,10 @@ import { bindActionCreators } from 'redux'; import { connect } from 'react-redux'; import DashboardBuilder from '../components/DashboardBuilder'; -import { toggleBuilderPane } from '../actions/dashboardState'; +import { + setColorSchemeAndUnsavedChanges, + showBuilderPane, +} from '../actions/dashboardState'; import { deleteTopLevelTabs, handleComponentDrop, @@ -30,7 +33,8 @@ function mapStateToProps({ dashboardLayout: undoableLayout, dashboardState }) { return { dashboardLayout: undoableLayout.present, editMode: dashboardState.editMode, - showBuilderPane: dashboardState.showBuilderPane, + builderPaneType: dashboardState.builderPaneType, + colorScheme: dashboardState.colorScheme, }; } @@ -39,7 +43,8 @@ function mapDispatchToProps(dispatch) { { deleteTopLevelTabs, handleComponentDrop, - toggleBuilderPane, + showBuilderPane, + setColorSchemeAndUnsavedChanges, }, dispatch, ); diff --git a/superset/assets/src/dashboard/containers/DashboardHeader.jsx b/superset/assets/src/dashboard/containers/DashboardHeader.jsx index 570d790ae7..05e90fb924 100644 --- a/superset/assets/src/dashboard/containers/DashboardHeader.jsx +++ b/superset/assets/src/dashboard/containers/DashboardHeader.jsx @@ -24,7 +24,7 @@ import isDashboardLoading from '../util/isDashboardLoading'; import { setEditMode, - toggleBuilderPane, + showBuilderPane, fetchFaveStar, saveFaveStar, fetchCharts, @@ -71,6 +71,8 @@ function mapStateToProps({ expandedSlices: dashboardState.expandedSlices, refreshFrequency: dashboardState.refreshFrequency, css: dashboardState.css, + colorNamespace: dashboardState.colorNamespace, + colorScheme: dashboardState.colorScheme, charts, userId: dashboardInfo.userId, isStarred: !!dashboardState.isStarred, @@ -78,7 +80,7 @@ function mapStateToProps({ hasUnsavedChanges: !!dashboardState.hasUnsavedChanges, maxUndoHistoryExceeded: !!dashboardState.maxUndoHistoryExceeded, editMode: !!dashboardState.editMode, - showBuilderPane: !!dashboardState.showBuilderPane, + builderPaneType: dashboardState.builderPaneType, }; } @@ -91,7 +93,7 @@ function mapDispatchToProps(dispatch) { onUndo: undoLayoutAction, onRedo: redoLayoutAction, setEditMode, - toggleBuilderPane, + showBuilderPane, fetchFaveStar, saveFaveStar, fetchCharts, diff --git a/superset/assets/src/dashboard/reducers/dashboardState.js b/superset/assets/src/dashboard/reducers/dashboardState.js index 24066ebca6..007f63a286 100644 --- a/superset/assets/src/dashboard/reducers/dashboardState.js +++ b/superset/assets/src/dashboard/reducers/dashboardState.js @@ -23,15 +23,17 @@ import { ON_CHANGE, ON_SAVE, REMOVE_SLICE, + SET_COLOR_SCHEME, SET_EDIT_MODE, SET_MAX_UNDO_HISTORY_EXCEEDED, SET_UNSAVED_CHANGES, - TOGGLE_BUILDER_PANE, + SHOW_BUILDER_PANE, TOGGLE_EXPAND_SLICE, TOGGLE_FAVE_STAR, UPDATE_CSS, SET_REFRESH_FREQUENCY, } from '../actions/dashboardState'; +import { BUILDER_PANE_TYPE } from '../util/constants'; export default function dashboardStateReducer(state = {}, action) { const actionHandlers = { @@ -73,15 +75,24 @@ export default function dashboardStateReducer(state = {}, action) { return { ...state, editMode: action.editMode, - showBuilderPane: !!action.editMode, + builderPaneType: action.editMode + ? BUILDER_PANE_TYPE.ADD_COMPONENTS + : BUILDER_PANE_TYPE.NONE, }; }, [SET_MAX_UNDO_HISTORY_EXCEEDED]() { const { maxUndoHistoryExceeded = true } = action.payload; return { ...state, maxUndoHistoryExceeded }; }, - [TOGGLE_BUILDER_PANE]() { - return { ...state, showBuilderPane: !state.showBuilderPane }; + [SHOW_BUILDER_PANE]() { + return { ...state, builderPaneType: action.builderPaneType }; + }, + [SET_COLOR_SCHEME]() { + return { + ...state, + colorScheme: action.colorScheme, + updatedColorScheme: true, + }; }, [TOGGLE_EXPAND_SLICE]() { const updatedExpandedSlices = { ...state.expandedSlices }; @@ -102,6 +113,8 @@ export default function dashboardStateReducer(state = {}, action) { hasUnsavedChanges: false, maxUndoHistoryExceeded: false, editMode: false, + builderPaneType: BUILDER_PANE_TYPE.NONE, + updatedColorScheme: false, }; }, diff --git a/superset/assets/src/dashboard/reducers/getInitialState.js b/superset/assets/src/dashboard/reducers/getInitialState.js index a9c4d8f95f..69396de786 100644 --- a/superset/assets/src/dashboard/reducers/getInitialState.js +++ b/superset/assets/src/dashboard/reducers/getInitialState.js @@ -17,6 +17,7 @@ * under the License. */ /* eslint-disable camelcase */ +import { isString } from 'lodash'; import shortid from 'shortid'; import { CategoricalColorNamespace } from '@superset-ui/color'; @@ -28,6 +29,7 @@ import findFirstParentContainerId from '../util/findFirstParentContainer'; import getEmptyLayout from '../util/getEmptyLayout'; import newComponentFactory from '../util/newComponentFactory'; import { + BUILDER_PANE_TYPE, DASHBOARD_HEADER_ID, GRID_DEFAULT_CHART_WIDTH, GRID_COLUMN_COUNT, @@ -55,9 +57,16 @@ export default function(bootstrapData) { // Priming the color palette with user's label-color mapping provided in // the dashboard's JSON metadata if (dashboard.metadata && dashboard.metadata.label_colors) { - const colorMap = dashboard.metadata.label_colors; + const scheme = dashboard.metadata.color_scheme; + const namespace = dashboard.metadata.color_namespace; + const colorMap = isString(dashboard.metadata.label_colors) + ? JSON.parse(dashboard.metadata.label_colors) + : dashboard.metadata.label_colors; Object.keys(colorMap).forEach(label => { - CategoricalColorNamespace.getScale().setColor(label, colorMap[label]); + CategoricalColorNamespace.getScale(scheme, namespace).setColor( + label, + colorMap[label], + ); }); } @@ -188,8 +197,13 @@ export default function(bootstrapData) { expandedSlices: dashboard.metadata.expanded_slices || {}, refreshFrequency: dashboard.metadata.refresh_frequency || 0, css: dashboard.css || '', + colorNamespace: dashboard.metadata.color_namespace, + colorScheme: dashboard.metadata.color_scheme, editMode: dashboard.dash_edit_perm && editMode, - showBuilderPane: dashboard.dash_edit_perm && editMode, + builderPaneType: + dashboard.dash_edit_perm && editMode + ? BUILDER_PANE_TYPE.ADD_COMPONENTS + : BUILDER_PANE_TYPE.NONE, hasUnsavedChanges: false, maxUndoHistoryExceeded: false, }, diff --git a/superset/assets/src/dashboard/stylesheets/builder-sidepane.less b/superset/assets/src/dashboard/stylesheets/builder-sidepane.less index 3b850c84fa..6bf6f6fafd 100644 --- a/superset/assets/src/dashboard/stylesheets/builder-sidepane.less +++ b/superset/assets/src/dashboard/stylesheets/builder-sidepane.less @@ -185,4 +185,18 @@ outline: none; } } + + .color-scheme-container { + list-style: none; + margin: 0; + padding: 0; + display: flex; + align-items: center; + } + + .color-scheme-container li { + flex-basis: 9px; + height: 10px; + margin: 9px 1px; + } } diff --git a/superset/assets/src/dashboard/stylesheets/dashboard.less b/superset/assets/src/dashboard/stylesheets/dashboard.less index 3dc9672b19..16541db1ee 100644 --- a/superset/assets/src/dashboard/stylesheets/dashboard.less +++ b/superset/assets/src/dashboard/stylesheets/dashboard.less @@ -120,7 +120,14 @@ body { display: flex; flex-direction: row; flex-wrap: nowrap; - & > :not(:last-child) { + & > :nth-child(3) { + border-radius: 2px 0px 0px 2px; + border-right: none; + } + & > :nth-child(4) { + border-radius: 0px 2px 2px 0px; + } + & > :not(:nth-child(3)):not(:last-child) { margin-right: 8px; } } diff --git a/superset/assets/src/dashboard/util/charts/getFormDataWithExtraFilters.js b/superset/assets/src/dashboard/util/charts/getFormDataWithExtraFilters.js index f397a937f8..a928a12b94 100644 --- a/superset/assets/src/dashboard/util/charts/getFormDataWithExtraFilters.js +++ b/superset/assets/src/dashboard/util/charts/getFormDataWithExtraFilters.js @@ -28,12 +28,15 @@ export default function getFormDataWithExtraFilters({ chart = {}, dashboardMetadata, filters, + colorScheme, sliceId, }) { // if dashboard metadata + filters have not changed, use cache if possible if ( (cachedDashboardMetadataByChart[sliceId] || {}) === dashboardMetadata && (cachedFiltersByChart[sliceId] || {}) === filters && + (colorScheme == null || + cachedFormdataByChart[sliceId].color_scheme === colorScheme) && !!cachedFormdataByChart[sliceId] ) { return cachedFormdataByChart[sliceId]; @@ -41,6 +44,7 @@ export default function getFormDataWithExtraFilters({ const formData = { ...chart.formData, + ...(colorScheme && { color_scheme: colorScheme }), extra_filters: getEffectiveExtraFilters({ dashboardMetadata, filters, diff --git a/superset/assets/src/dashboard/util/constants.js b/superset/assets/src/dashboard/util/constants.js index 5cce3ae379..9b33ca8991 100644 --- a/superset/assets/src/dashboard/util/constants.js +++ b/superset/assets/src/dashboard/util/constants.js @@ -62,3 +62,10 @@ export const SAVE_TYPE_NEWDASHBOARD = 'newDashboard'; // default dashboard layout data size limit // could be overwritten by server-side config export const DASHBOARD_POSITION_DATA_LIMIT = 65535; + +// Dashboard pane types +export const BUILDER_PANE_TYPE = { + NONE: 'NONE', + ADD_COMPONENTS: 'ADD_COMPONENTS', + COLORS: 'COLORS', +}; diff --git a/superset/assets/src/dashboard/util/propShapes.jsx b/superset/assets/src/dashboard/util/propShapes.jsx index c433de924e..c50ffc6a09 100644 --- a/superset/assets/src/dashboard/util/propShapes.jsx +++ b/superset/assets/src/dashboard/util/propShapes.jsx @@ -72,7 +72,10 @@ export const dashboardStatePropShape = PropTypes.shape({ filters: PropTypes.object.isRequired, expandedSlices: PropTypes.object, editMode: PropTypes.bool, - showBuilderPane: PropTypes.bool, + builderPaneType: PropTypes.string.isRequired, + colorNamespace: PropTypes.string, + colorScheme: PropTypes.string, + updatedColorScheme: PropTypes.bool, hasUnsavedChanges: PropTypes.bool, }); diff --git a/superset/assets/src/explore/components/controls/ColorSchemeControl.jsx b/superset/assets/src/explore/components/controls/ColorSchemeControl.jsx index 1e1e67718e..34a4d3cf1f 100644 --- a/superset/assets/src/explore/components/controls/ColorSchemeControl.jsx +++ b/superset/assets/src/explore/components/controls/ColorSchemeControl.jsx @@ -21,6 +21,7 @@ import PropTypes from 'prop-types'; import { isFunction } from 'lodash'; import { Creatable } from 'react-select'; import ControlHeader from '../ControlHeader'; +import TooltipWrapper from '../../../components/TooltipWrapper'; const propTypes = { description: PropTypes.string, @@ -77,17 +78,22 @@ export default class ColorSchemeControl extends React.PureComponent { } return ( -
    - {colors.map((color, i) => ( -
  •  
  • - ))} -
+ +
    + {colors.map((color, i) => ( +
  •  
  • + ))} +
+
); } diff --git a/superset/views/core.py b/superset/views/core.py index 39343d67c7..794bcb36c7 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -1741,6 +1741,12 @@ class Superset(BaseSupersetView): {key: v for key, v in default_filters_data.items() if int(key) in slice_ids} md['default_filters'] = json.dumps(applicable_filters) + if data.get('color_namespace'): + md['color_namespace'] = data.get('color_namespace') + if data.get('color_scheme'): + md['color_scheme'] = data.get('color_scheme') + if data.get('label_colors'): + md['label_colors'] = data.get('label_colors') dashboard.json_metadata = json.dumps(md) @api diff --git a/tests/dashboard_tests.py b/tests/dashboard_tests.py index c436753dd0..04dcd5907a 100644 --- a/tests/dashboard_tests.py +++ b/tests/dashboard_tests.py @@ -191,17 +191,60 @@ class DashboardTests(SupersetTestCase): data['dashboard_title'] = origin_title self.get_resp(url, data=dict(data=json.dumps(data))) + def test_save_dash_with_colors(self, username='admin'): + self.login(username=username) + dash = ( + db.session.query(models.Dashboard) + .filter_by(slug='births') + .first() + ) + positions = self.get_mock_positions(dash) + new_label_colors = { + 'data value': 'random color', + } + data = { + 'css': '', + 'expanded_slices': {}, + 'positions': positions, + 'dashboard_title': dash.dashboard_title, + 'color_namespace': 'Color Namespace Test', + 'color_scheme': 'Color Scheme Test', + 'label_colors': new_label_colors, + + } + url = '/superset/save_dash/{}/'.format(dash.id) + self.get_resp(url, data=dict(data=json.dumps(data))) + updatedDash = ( + db.session.query(models.Dashboard) + .filter_by(slug='births') + .first() + ) + self.assertIn('color_namespace', updatedDash.json_metadata) + self.assertIn('color_scheme', updatedDash.json_metadata) + self.assertIn('label_colors', updatedDash.json_metadata) + # bring back original dashboard + del data['color_namespace'] + del data['color_scheme'] + del data['label_colors'] + self.get_resp(url, data=dict(data=json.dumps(data))) + def test_copy_dash(self, username='admin'): self.login(username=username) dash = db.session.query(models.Dashboard).filter_by( slug='births').first() positions = self.get_mock_positions(dash) + new_label_colors = { + 'data value': 'random color', + } data = { 'css': '', 'duplicate_slices': False, 'expanded_slices': {}, 'positions': positions, 'dashboard_title': 'Copy Of Births', + 'color_namespace': 'Color Namespace Test', + 'color_scheme': 'Color Scheme Test', + 'label_colors': new_label_colors, } # Save changes to Births dashboard and retrieve updated dash