Skip to content

Commit

Permalink
chore: Notify user of custom label colors and related Dashboard color…
Browse files Browse the repository at this point in the history
… scheme (apache#17422)

* Add alert for custom label colors

* Fix duplicate linear scheme

* Implement dashboard alert

* Remove trailing space

* Update Cypress

* Simplify check
  • Loading branch information
geido authored Nov 16, 2021
1 parent c829614 commit c2d8b0e
Show file tree
Hide file tree
Showing 5 changed files with 186 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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: () => {},
};
Expand All @@ -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}
Expand All @@ -63,6 +64,7 @@ class ColorSchemeControlWrapper extends React.PureComponent {
clearable
schemes={this.schemes}
hovered={this.state.hovered}
hasCustomLabelColors={hasCustomLabelColors}
/>
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,20 +132,30 @@ 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() {
this.fetchDashboardDetails();
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)) {
Expand Down Expand Up @@ -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}>
Expand All @@ -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}
Expand All @@ -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>
Expand Down Expand Up @@ -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}
Expand Down
Original file line number Diff line number Diff line change
@@ -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();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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) {
Expand All @@ -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',
Expand Down Expand Up @@ -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}
/>
);
}
}
Expand Down

0 comments on commit c2d8b0e

Please sign in to comment.