Skip to content

Commit

Permalink
Code review fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
kgabryje committed Apr 19, 2022
1 parent a50e0f0 commit 7114545
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 34 deletions.
17 changes: 4 additions & 13 deletions superset-frontend/src/components/Chart/Chart.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import { getUrlParam } from 'src/utils/urlUtils';
import { ResourceStatus } from 'src/hooks/apiResources/apiResources';
import ChartRenderer from './ChartRenderer';
import { ChartErrorMessage } from './ChartErrorMessage';
import { getChartRequiredFieldsMissingMessage } from '../../utils/getChartRequiredFieldsMissingMessage';

const propTypes = {
annotationData: PropTypes.object,
Expand Down Expand Up @@ -114,16 +115,6 @@ const MonospaceDiv = styled.div`
white-space: pre-wrap;
`;

const requiredFieldsMissingWarning = isFeatureEnabled(
FeatureFlag.ENABLE_EXPLORE_DRAG_AND_DROP,
)
? t(
'Drag and drop values into highlighted field(s) in the control panel. Then run the query by clicking on the "Create chart" button.',
)
: t(
'Select values in highlighted field(s) in the control panel. Then run the query by clicking on the "Create chart" button.',
);

class Chart extends React.PureComponent {
constructor(props) {
super(props);
Expand Down Expand Up @@ -259,7 +250,6 @@ class Chart extends React.PureComponent {
} = this.props;

const isLoading = chartStatus === 'loading';
const isFaded = chartIsStale && !errorMessage;
this.renderContainerStartTime = Logger.getTimestamp();
if (chartStatus === 'failed') {
return queriesResponse.map(item => this.renderErrorMessage(item));
Expand All @@ -269,7 +259,7 @@ class Chart extends React.PureComponent {
return (
<EmptyStateBig
title={t('Add required control values to preview chart')}
description={requiredFieldsMissingWarning}
description={getChartRequiredFieldsMissingMessage(true)}
image="chart.svg"
/>
);
Expand All @@ -278,7 +268,8 @@ class Chart extends React.PureComponent {
if (
!isLoading &&
!chartAlert &&
isFaded &&
!errorMessage &&
chartIsStale &&
ensureIsArray(queriesResponse).length === 0
) {
return (
Expand Down
30 changes: 9 additions & 21 deletions superset-frontend/src/explore/components/ExploreChartPanel.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ import Split from 'react-split';
import {
css,
ensureIsArray,
FeatureFlag,
isFeatureEnabled,
styled,
SupersetClient,
t,
Expand All @@ -41,6 +39,7 @@ import { DataTablesPane } from './DataTablesPane';
import { buildV1ChartDataPayload } from '../exploreUtils';
import { ChartPills } from './ChartPills';
import { ExploreAlert } from './ExploreAlert';
import { getChartRequiredFieldsMissingMessage } from '../../utils/getChartRequiredFieldsMissingMessage';

const propTypes = {
actions: PropTypes.object.isRequired,
Expand Down Expand Up @@ -114,16 +113,6 @@ const Styles = styled.div`
}
`;

const requiredFieldsMissingWarning = isFeatureEnabled(
FeatureFlag.ENABLE_EXPLORE_DRAG_AND_DROP,
)
? t(
'Drag and drop values into highlighted field(s) in the control panel. Then run the query by clicking on the "Update chart" button.',
)
: t(
'Select values in highlighted field(s) in the control panel. Then run the query by clicking on the "Update chart" button.',
);

const ExploreChartPanel = ({
chart,
slice,
Expand All @@ -135,7 +124,6 @@ const ExploreChartPanel = ({
errorMessage,
form_data: formData,
onQuery,
refreshOverlayVisible,
actions,
timeout,
standalone,
Expand Down Expand Up @@ -250,7 +238,7 @@ const ExploreChartPanel = ({
formData={formData}
onQuery={onQuery}
queriesResponse={chart.queriesResponse}
refreshOverlayVisible={refreshOverlayVisible}
chartIsStale={chartIsStale}
setControlValue={actions.setControlValue}
timeout={timeout}
triggerQuery={chart.triggerQuery}
Expand All @@ -277,7 +265,6 @@ const ExploreChartPanel = ({
formData,
onQuery,
ownState,
refreshOverlayVisible,
timeout,
triggerRender,
vizType,
Expand All @@ -302,7 +289,7 @@ const ExploreChartPanel = ({
}
bodyText={
errorMessage ? (
requiredFieldsMissingWarning
getChartRequiredFieldsMissingMessage(false)
) : (
<span>
{t(
Expand Down Expand Up @@ -333,15 +320,16 @@ const ExploreChartPanel = ({
</div>
),
[
chartPanelRef,
showAlertBanner,
errorMessage,
onQuery,
chart.queriesResponse,
chart.chartStatus,
chart.chartUpdateEndTime,
chart.chartUpdateStartTime,
chart.queriesResponse,
formData?.row_limit,
chart.chartUpdateEndTime,
refreshCachedQuery,
formData?.row_limit,
renderChart,
showAlertBanner,
],
);

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/**
* 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 { t } from '@superset-ui/core';

export const getChartRequiredFieldsMissingMessage = (isCreating: boolean) =>
t(
'Select values in highlighted field(s) in the control panel. Then run the query by clicking on the %s button.',
isCreating ? '"Create chart"' : '"Update chart"',
);

0 comments on commit 7114545

Please sign in to comment.