You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by di...@apache.org on 2021/11/16 15:47:56 UTC
[superset] branch master updated: chore: Notify user of custom label colors and related Dashboard color scheme (#17422)
This is an automated email from the ASF dual-hosted git repository.
diegopucci pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new c2d8b0e chore: Notify user of custom label colors and related Dashboard color scheme (#17422)
c2d8b0e is described below
commit c2d8b0e72f5146bca0b7ef59f8fe28972c7664e4
Author: Geido <60...@users.noreply.github.com>
AuthorDate: Tue Nov 16 17:46:39 2021 +0200
chore: Notify user of custom label colors and related Dashboard color scheme (#17422)
* Add alert for custom label colors
* Fix duplicate linear scheme
* Implement dashboard alert
* Remove trailing space
* Update Cypress
* Simplify check
---
.../explore/visualizations/line.test.ts | 2 +-
.../components/ColorSchemeControlWrapper.jsx | 6 +-
.../dashboard/components/PropertiesModal/index.jsx | 30 +++++-
.../ColorSchemeControl/ColorSchemeControl.test.tsx | 68 +++++++++++++
.../index.jsx} | 112 ++++++++++++++++-----
5 files changed, 186 insertions(+), 32 deletions(-)
diff --git a/superset-frontend/cypress-base/cypress/integration/explore/visualizations/line.test.ts b/superset-frontend/cypress-base/cypress/integration/explore/visualizations/line.test.ts
index 676c3b6..019bad7 100644
--- a/superset-frontend/cypress-base/cypress/integration/explore/visualizations/line.test.ts
+++ b/superset-frontend/cypress-base/cypress/integration/explore/visualizations/line.test.ts
@@ -71,7 +71,7 @@ describe('Visualization > Line', () => {
.focus()
.type('bnbColors{enter}');
cy.get(
- '.Control[data-test="color_scheme"] .ant-select-selection-item > ul[data-test="bnbColors"]',
+ '.Control[data-test="color_scheme"] .ant-select-selection-item ul[data-test="bnbColors"]',
).should('exist');
});
diff --git a/superset-frontend/src/dashboard/components/ColorSchemeControlWrapper.jsx b/superset-frontend/src/dashboard/components/ColorSchemeControlWrapper.jsx
index 18af6f5..4a7839f 100644
--- a/superset-frontend/src/dashboard/components/ColorSchemeControlWrapper.jsx
+++ b/superset-frontend/src/dashboard/components/ColorSchemeControlWrapper.jsx
@@ -27,9 +27,11 @@ const propTypes = {
onChange: PropTypes.func,
labelMargin: PropTypes.number,
colorScheme: PropTypes.string,
+ hasCustomLabelColors: PropTypes.bool,
};
const defaultProps = {
+ hasCustomLabelColors: false,
colorScheme: undefined,
onChange: () => {},
};
@@ -48,13 +50,12 @@ class ColorSchemeControlWrapper extends React.PureComponent {
}
render() {
- const { colorScheme, labelMargin = 0 } = this.props;
+ const { colorScheme, labelMargin = 0, hasCustomLabelColors } = this.props;
return (
<ColorSchemeControl
description={t(
"Any color palette selected here will override the colors applied to this dashboard's individual charts",
)}
- label={t('Color scheme')}
labelMargin={labelMargin}
name="color_scheme"
onChange={this.props.onChange}
@@ -63,6 +64,7 @@ class ColorSchemeControlWrapper extends React.PureComponent {
clearable
schemes={this.schemes}
hovered={this.state.hovered}
+ hasCustomLabelColors={hasCustomLabelColors}
/>
);
}
diff --git a/superset-frontend/src/dashboard/components/PropertiesModal/index.jsx b/superset-frontend/src/dashboard/components/PropertiesModal/index.jsx
index 9597ccf..8acff98 100644
--- a/superset-frontend/src/dashboard/components/PropertiesModal/index.jsx
+++ b/superset-frontend/src/dashboard/components/PropertiesModal/index.jsx
@@ -132,6 +132,7 @@ class PropertiesModal extends React.PureComponent {
this.onColorSchemeChange = this.onColorSchemeChange.bind(this);
this.getRowsWithRoles = this.getRowsWithRoles.bind(this);
this.getRowsWithoutRoles = this.getRowsWithoutRoles.bind(this);
+ this.getJsonMetadata = this.getJsonMetadata.bind(this);
}
componentDidMount() {
@@ -139,13 +140,22 @@ class PropertiesModal extends React.PureComponent {
JsonEditor.preload();
}
+ getJsonMetadata() {
+ const { json_metadata: jsonMetadata } = this.state.values;
+ try {
+ const jsonMetadataObj = jsonMetadata?.length
+ ? JSON.parse(jsonMetadata)
+ : {};
+ return jsonMetadataObj;
+ } catch (_) {
+ return {};
+ }
+ }
+
onColorSchemeChange(colorScheme, { updateMetadata = true } = {}) {
// check that color_scheme is valid
const colorChoices = getCategoricalSchemeRegistry().keys();
- const { json_metadata: jsonMetadata } = this.state.values;
- const jsonMetadataObj = jsonMetadata?.length
- ? JSON.parse(jsonMetadata)
- : {};
+ const jsonMetadataObj = this.getJsonMetadata();
// only fire if the color_scheme is present and invalid
if (colorScheme && !colorChoices.includes(colorScheme)) {
@@ -318,6 +328,11 @@ class PropertiesModal extends React.PureComponent {
getRowsWithoutRoles() {
const { values, isDashboardLoaded } = this.state;
+ const jsonMetadataObj = this.getJsonMetadata();
+ const hasCustomLabelColors = !!Object.keys(
+ jsonMetadataObj?.label_colors || {},
+ ).length;
+
return (
<Row gutter={16}>
<Col xs={24} md={12}>
@@ -343,6 +358,7 @@ class PropertiesModal extends React.PureComponent {
<Col xs={24} md={12}>
<h3 style={{ marginTop: '1em' }}>{t('Colors')}</h3>
<ColorSchemeControlWrapper
+ hasCustomLabelColors={hasCustomLabelColors}
onChange={this.onColorSchemeChange}
colorScheme={values.colorScheme}
labelMargin={4}
@@ -354,6 +370,11 @@ class PropertiesModal extends React.PureComponent {
getRowsWithRoles() {
const { values, isDashboardLoaded } = this.state;
+ const jsonMetadataObj = this.getJsonMetadata();
+ const hasCustomLabelColors = !!Object.keys(
+ jsonMetadataObj?.label_colors || {},
+ ).length;
+
return (
<>
<Row>
@@ -404,6 +425,7 @@ class PropertiesModal extends React.PureComponent {
<Row>
<Col xs={24} md={12}>
<ColorSchemeControlWrapper
+ hasCustomLabelColors={hasCustomLabelColors}
onChange={this.onColorSchemeChange}
colorScheme={values.colorScheme}
labelMargin={4}
diff --git a/superset-frontend/src/explore/components/controls/ColorSchemeControl/ColorSchemeControl.test.tsx b/superset-frontend/src/explore/components/controls/ColorSchemeControl/ColorSchemeControl.test.tsx
new file mode 100644
index 0000000..1f19d8a
--- /dev/null
+++ b/superset-frontend/src/explore/components/controls/ColorSchemeControl/ColorSchemeControl.test.tsx
@@ -0,0 +1,68 @@
+/**
+ * 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.
+ */
+import React from 'react';
+import { render, screen, waitFor } from 'spec/helpers/testing-library';
+import ColorSchemeControl from '.';
+
+const defaultProps = {
+ hasCustomLabelColors: false,
+ label: 'Color scheme',
+ labelMargin: 0,
+ name: 'color',
+ value: 'supersetDefault',
+ clearable: true,
+ choices: [],
+ schemes: () => null,
+ isLinear: false,
+};
+
+const setup = (overrides?: Record<string, any>) =>
+ render(<ColorSchemeControl {...defaultProps} {...overrides} />);
+
+test('should render', async () => {
+ const { container } = setup();
+ await waitFor(() => expect(container).toBeVisible());
+});
+
+test('should display a label', async () => {
+ setup();
+ expect(await screen.findByText('Color scheme')).toBeTruthy();
+});
+
+test('should not display an alert icon if hasCustomLabelColors=false', async () => {
+ setup();
+ await waitFor(() => {
+ expect(
+ screen.queryByRole('img', { name: 'alert-solid' }),
+ ).not.toBeInTheDocument();
+ });
+});
+
+test('should display an alert icon if hasCustomLabelColors=true', async () => {
+ const hasCustomLabelColorsProps = {
+ ...defaultProps,
+ hasCustomLabelColors: true,
+ };
+ setup(hasCustomLabelColorsProps);
+ await waitFor(() => {
+ expect(
+ screen.getByRole('img', { name: 'alert-solid' }),
+ ).toBeInTheDocument();
+ });
+});
diff --git a/superset-frontend/src/explore/components/controls/ColorSchemeControl.jsx b/superset-frontend/src/explore/components/controls/ColorSchemeControl/index.jsx
similarity index 53%
rename from superset-frontend/src/explore/components/controls/ColorSchemeControl.jsx
rename to superset-frontend/src/explore/components/controls/ColorSchemeControl/index.jsx
index 20be45d..f0ccc12 100644
--- a/superset-frontend/src/explore/components/controls/ColorSchemeControl.jsx
+++ b/superset-frontend/src/explore/components/controls/ColorSchemeControl/index.jsx
@@ -21,12 +21,15 @@ import PropTypes from 'prop-types';
import { isFunction } from 'lodash';
import { Select } from 'src/components';
import { Tooltip } from 'src/components/Tooltip';
-import { t } from '@superset-ui/core';
-import ControlHeader from '../ControlHeader';
+import { styled, t } from '@superset-ui/core';
+import Icons from 'src/components/Icons';
+import ControlHeader from 'src/explore/components/ControlHeader';
const propTypes = {
+ hasCustomLabelColors: PropTypes.bool,
+ dashboardId: PropTypes.number,
description: PropTypes.string,
- label: PropTypes.string.isRequired,
+ label: PropTypes.string,
labelMargin: PropTypes.number,
name: PropTypes.string.isRequired,
onChange: PropTypes.func,
@@ -43,16 +46,27 @@ const propTypes = {
const defaultProps = {
choices: [],
+ hasCustomLabelColors: false,
+ label: t('Color scheme'),
schemes: {},
clearable: false,
onChange: () => {},
};
+const StyledAlert = styled(Icons.AlertSolid)`
+ color: ${({ theme }) => theme.colors.alert.base};
+`;
+
export default class ColorSchemeControl extends React.PureComponent {
constructor(props) {
super(props);
this.onChange = this.onChange.bind(this);
this.renderOption = this.renderOption.bind(this);
+ this.renderLabel = this.renderLabel.bind(this);
+ this.dashboardColorSchemeAlert = t(
+ `The color scheme is determined by the related dashboard.
+ Edit the color scheme in the dashboard properties.`,
+ );
}
onChange(value) {
@@ -72,7 +86,7 @@ export default class ColorSchemeControl extends React.PureComponent {
}
return (
- <Tooltip id={`${currentScheme.id}-tooltip`} title={currentScheme.label}>
+ <span key={currentScheme.id} title={currentScheme.label}>
<ul
css={{
listStyle: 'none',
@@ -101,43 +115,91 @@ export default class ColorSchemeControl extends React.PureComponent {
</li>
))}
</ul>
- </Tooltip>
+ </span>
);
}
+ renderLabel() {
+ const { dashboardId, hasCustomLabelColors, label } = this.props;
+
+ if (hasCustomLabelColors || dashboardId) {
+ const alertTitle = hasCustomLabelColors
+ ? t(
+ `This color scheme is being overriden by custom label colors.
+ Check the JSON metadata in the Advanced settings`,
+ )
+ : this.dashboardColorSchemeAlert;
+ return (
+ <>
+ {label}{' '}
+ <Tooltip title={alertTitle}>
+ <StyledAlert iconSize="s" />
+ </Tooltip>
+ </>
+ );
+ }
+ return label;
+ }
+
render() {
- const { schemes, choices } = this.props;
- // save parsed schemes for later
- this.schemes = isFunction(schemes) ? schemes() : schemes;
+ const { choices, dashboardId, schemes } = this.props;
+ let options = dashboardId
+ ? [
+ {
+ value: 'dashboard',
+ label: 'dashboard',
+ customLabel: (
+ <Tooltip title={this.dashboardColorSchemeAlert}>
+ {t('Dashboard scheme')}
+ </Tooltip>
+ ),
+ },
+ ]
+ : [];
+ let currentScheme = dashboardId ? 'dashboard' : undefined;
- const allColorOptions = (isFunction(choices) ? choices() : choices).filter(
- o => o[0] !== 'SUPERSET_DEFAULT',
- );
- const options = allColorOptions.map(([value]) => ({
- value,
- label: this.schemes?.[value]?.label || value,
- customLabel: this.renderOption(value),
- }));
-
- let currentScheme =
- this.props.value ||
- (this.props.default !== undefined ? this.props.default : undefined);
-
- if (currentScheme === 'SUPERSET_DEFAULT') {
- currentScheme = this.schemes?.SUPERSET_DEFAULT?.id;
+ // if related to a dashboard the scheme is dictated by the dashboard
+ if (!dashboardId) {
+ this.schemes = isFunction(schemes) ? schemes() : schemes;
+ const controlChoices = isFunction(choices) ? choices() : choices;
+ const allColorOptions = [];
+ const filteredColorOptions = controlChoices.filter(o => {
+ const option = o[0];
+ const isValidColorOption =
+ option !== 'SUPERSET_DEFAULT' && !allColorOptions.includes(option);
+ allColorOptions.push(option);
+ return isValidColorOption;
+ });
+
+ options = filteredColorOptions.map(([value]) => ({
+ customLabel: this.renderOption(value),
+ label: this.schemes?.[value]?.label || value,
+ value,
+ }));
+
+ currentScheme = this.props.value || this.props.default;
+
+ if (currentScheme === 'SUPERSET_DEFAULT') {
+ currentScheme = this.schemes?.SUPERSET_DEFAULT?.id;
+ }
}
const selectProps = {
ariaLabel: t('Select color scheme'),
allowClear: this.props.clearable,
+ disabled: !!dashboardId,
name: `select-${this.props.name}`,
onChange: this.onChange,
options,
- placeholder: `Select (${options.length})`,
+ placeholder: t('Select scheme'),
value: currentScheme,
};
+
return (
- <Select header={<ControlHeader {...this.props} />} {...selectProps} />
+ <Select
+ header={<ControlHeader {...this.props} label={this.renderLabel()} />}
+ {...selectProps}
+ />
);
}
}