Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Notify user of custom label colors and related Dashboard color scheme #17422

Merged
merged 8 commits into from
Nov 16, 2021
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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,93 @@ 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 !== undefined ? this.props.default : undefined);
geido marked this conversation as resolved.
Show resolved Hide resolved

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