You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2022/01/25 04:38:13 UTC
[GitHub] [superset] lyndsiWilliams commented on a change in pull request #17970: refactor(explorechartheader): convert class to functional component
lyndsiWilliams commented on a change in pull request #17970:
URL: https://github.com/apache/superset/pull/17970#discussion_r791344323
##########
File path: superset-frontend/src/explore/components/ExploreChartHeader/index.jsx
##########
@@ -169,53 +159,42 @@ export class ExploreChartHeader extends React.PureComponent {
}
})
.catch(() => {});
- }
+ };
- getSliceName() {
- const { sliceName, table_name: tableName } = this.props;
+ const getSliceName = () => {
const title = sliceName || t('%s - untitled', tableName);
return title;
- }
+ };
Review comment:
This function can be cleaned to one line like this:
`const getSliceName = () => sliceName || t('%s - untitled', tableName);`
##########
File path: superset-frontend/src/explore/components/ExploreChartHeader/index.jsx
##########
@@ -169,53 +159,42 @@ export class ExploreChartHeader extends React.PureComponent {
}
})
.catch(() => {});
- }
+ };
- getSliceName() {
- const { sliceName, table_name: tableName } = this.props;
+ const getSliceName = () => {
const title = sliceName || t('%s - untitled', tableName);
return title;
- }
+ };
- postChartFormData() {
- this.props.actions.postChartFormData(
- this.props.form_data,
- true,
- this.props.timeout,
- this.props.chart.id,
- this.props.ownState,
- );
- }
+ const postChartFormData = () => {
+ actions.postChartFormData(formData, true, timeout, chart.id, ownState);
+ };
- openPropertiesModal() {
- this.setState({
- isPropertiesModalOpen: true,
- });
- }
+ const openPropertiesModal = () => {
+ setIsPropertiesModalOpen(true);
+ };
- closePropertiesModal() {
- this.setState({
- isPropertiesModalOpen: false,
- });
- }
+ const closePropertiesModal = () => {
+ setIsPropertiesModalOpen(false);
+ };
Review comment:
```suggestion
const closePropertiesModal = () => setIsPropertiesModalOpen(false);
```
##########
File path: superset-frontend/src/explore/components/ExploreChartHeader/index.jsx
##########
@@ -169,53 +159,42 @@ export class ExploreChartHeader extends React.PureComponent {
}
})
.catch(() => {});
- }
+ };
- getSliceName() {
- const { sliceName, table_name: tableName } = this.props;
+ const getSliceName = () => {
const title = sliceName || t('%s - untitled', tableName);
return title;
- }
+ };
- postChartFormData() {
- this.props.actions.postChartFormData(
- this.props.form_data,
- true,
- this.props.timeout,
- this.props.chart.id,
- this.props.ownState,
- );
- }
+ const postChartFormData = () => {
+ actions.postChartFormData(formData, true, timeout, chart.id, ownState);
+ };
- openPropertiesModal() {
- this.setState({
- isPropertiesModalOpen: true,
- });
- }
+ const openPropertiesModal = () => {
+ setIsPropertiesModalOpen(true);
+ };
Review comment:
```suggestion
const openPropertiesModal = () => setIsPropertiesModalOpen(true);
```
##########
File path: superset-frontend/src/explore/components/ExploreChartHeader/index.jsx
##########
@@ -248,134 +226,142 @@ export class ExploreChartHeader extends React.PureComponent {
),
);
return permissions[0].length > 0;
- }
+ };
- render() {
- const { user, form_data: formData, slice } = this.props;
- const {
- chartStatus,
- chartUpdateEndTime,
- chartUpdateStartTime,
- latestQueryFormData,
- queriesResponse,
- } = this.props.chart;
- // TODO: when will get appropriate design for multi queries use all results and not first only
- const queryResponse = queriesResponse?.[0];
- const chartFinished = ['failed', 'rendered', 'success'].includes(
- this.props.chart.chartStatus,
- );
- return (
- <StyledHeader id="slice-header" className="panel-title-large">
- <div className="title-panel">
- {slice?.certified_by && (
- <>
- <CertifiedBadge
- certifiedBy={slice.certified_by}
- details={slice.certification_details}
- />{' '}
- </>
- )}
- <EditableTitle
- title={this.getSliceName()}
- canEdit={
- !this.props.slice ||
- this.props.can_overwrite ||
- (this.props.slice?.owners || []).includes(
- this.props?.user?.userId,
- )
- }
- onSaveTitle={this.props.actions.updateChartTitle}
- />
+ useEffect(() => {
+ if (canAddReports()) {
+ fetchUISpecificReport(user.userId, 'chart_id', 'charts', chart.id);
+ }
+ if (dashboardId) {
+ fetchChartDashboardData();
+ }
+ }, [
+ canAddReports,
+ fetchUISpecificReport,
+ user.userId,
+ chart.id,
+ dashboardId,
+ fetchChartDashboardData,
+ ]);
- {this.props.slice && (
- <StyledButtons>
- {user.userId && (
- <FaveStar
- itemId={this.props.slice.slice_id}
- fetchFaveStar={this.props.actions.fetchFaveStar}
- saveFaveStar={this.props.actions.saveFaveStar}
- isStarred={this.props.isStarred}
- showTooltip
- />
- )}
- {this.state.isPropertiesModalOpen && (
- <PropertiesModal
- show={this.state.isPropertiesModalOpen}
- onHide={this.closePropertiesModal}
- onSave={this.props.sliceUpdated}
- slice={this.props.slice}
- />
- )}
- <Tooltip
- id="edit-desc-tooltip"
- title={t('Edit chart properties')}
+ const {
+ chartStatus,
+ chartUpdateEndTime,
+ chartUpdateStartTime,
+ latestQueryFormData,
+ queriesResponse,
+ } = chart;
+ // TODO: when will get appropriate design for multi queries use all results and not first only
+ const queryResponse = queriesResponse?.[0];
+ const chartFinished = ['failed', 'rendered', 'success'].includes(
+ chart.chartStatus,
+ );
+ return (
+ <StyledHeader id="slice-header" className="panel-title-large">
+ <div className="title-panel">
+ {slice?.certified_by && (
+ <>
+ <CertifiedBadge
+ certifiedBy={slice.certified_by}
+ details={slice.certification_details}
+ />{' '}
+ </>
+ )}
+ <EditableTitle
+ title={getSliceName()}
Review comment:
```suggestion
title={getSliceName}
```
This doesn't need to be instantiated.
##########
File path: superset-frontend/src/explore/components/ExploreChartHeader/index.jsx
##########
@@ -169,53 +159,42 @@ export class ExploreChartHeader extends React.PureComponent {
}
})
.catch(() => {});
- }
+ };
- getSliceName() {
- const { sliceName, table_name: tableName } = this.props;
+ const getSliceName = () => {
const title = sliceName || t('%s - untitled', tableName);
return title;
- }
+ };
- postChartFormData() {
- this.props.actions.postChartFormData(
- this.props.form_data,
- true,
- this.props.timeout,
- this.props.chart.id,
- this.props.ownState,
- );
- }
+ const postChartFormData = () => {
+ actions.postChartFormData(formData, true, timeout, chart.id, ownState);
+ };
- openPropertiesModal() {
- this.setState({
- isPropertiesModalOpen: true,
- });
- }
+ const openPropertiesModal = () => {
+ setIsPropertiesModalOpen(true);
+ };
- closePropertiesModal() {
- this.setState({
- isPropertiesModalOpen: false,
- });
- }
+ const closePropertiesModal = () => {
+ setIsPropertiesModalOpen(false);
+ };
- showReportModal() {
- this.setState({ showingReportModal: true });
- }
+ const showReportModal = () => {
+ setShowingReportModal(true);
+ };
Review comment:
```suggestion
const showReportModal = () => setShowingReportModal(true);
```
##########
File path: superset-frontend/src/explore/components/ExploreChartHeader/index.jsx
##########
@@ -248,134 +226,142 @@ export class ExploreChartHeader extends React.PureComponent {
),
);
return permissions[0].length > 0;
- }
+ };
- render() {
- const { user, form_data: formData, slice } = this.props;
- const {
- chartStatus,
- chartUpdateEndTime,
- chartUpdateStartTime,
- latestQueryFormData,
- queriesResponse,
- } = this.props.chart;
- // TODO: when will get appropriate design for multi queries use all results and not first only
- const queryResponse = queriesResponse?.[0];
- const chartFinished = ['failed', 'rendered', 'success'].includes(
- this.props.chart.chartStatus,
- );
- return (
- <StyledHeader id="slice-header" className="panel-title-large">
- <div className="title-panel">
- {slice?.certified_by && (
- <>
- <CertifiedBadge
- certifiedBy={slice.certified_by}
- details={slice.certification_details}
- />{' '}
- </>
- )}
- <EditableTitle
- title={this.getSliceName()}
- canEdit={
- !this.props.slice ||
- this.props.can_overwrite ||
- (this.props.slice?.owners || []).includes(
- this.props?.user?.userId,
- )
- }
- onSaveTitle={this.props.actions.updateChartTitle}
- />
+ useEffect(() => {
+ if (canAddReports()) {
+ fetchUISpecificReport(user.userId, 'chart_id', 'charts', chart.id);
+ }
+ if (dashboardId) {
+ fetchChartDashboardData();
+ }
+ }, [
+ canAddReports,
+ fetchUISpecificReport,
+ user.userId,
+ chart.id,
+ dashboardId,
+ fetchChartDashboardData,
+ ]);
- {this.props.slice && (
- <StyledButtons>
- {user.userId && (
- <FaveStar
- itemId={this.props.slice.slice_id}
- fetchFaveStar={this.props.actions.fetchFaveStar}
- saveFaveStar={this.props.actions.saveFaveStar}
- isStarred={this.props.isStarred}
- showTooltip
- />
- )}
- {this.state.isPropertiesModalOpen && (
- <PropertiesModal
- show={this.state.isPropertiesModalOpen}
- onHide={this.closePropertiesModal}
- onSave={this.props.sliceUpdated}
- slice={this.props.slice}
- />
- )}
- <Tooltip
- id="edit-desc-tooltip"
- title={t('Edit chart properties')}
+ const {
+ chartStatus,
+ chartUpdateEndTime,
+ chartUpdateStartTime,
+ latestQueryFormData,
+ queriesResponse,
+ } = chart;
+ // TODO: when will get appropriate design for multi queries use all results and not first only
+ const queryResponse = queriesResponse?.[0];
+ const chartFinished = ['failed', 'rendered', 'success'].includes(
+ chart.chartStatus,
+ );
+ return (
+ <StyledHeader id="slice-header" className="panel-title-large">
+ <div className="title-panel">
+ {slice?.certified_by && (
+ <>
+ <CertifiedBadge
+ certifiedBy={slice.certified_by}
+ details={slice.certification_details}
+ />{' '}
+ </>
+ )}
+ <EditableTitle
+ title={getSliceName()}
+ canEdit={
+ !slice ||
+ canOverwrite ||
+ (slice?.owners || []).includes(user?.userId)
+ }
+ onSaveTitle={actions.updateChartTitle}
+ />
+
+ {slice && (
+ <StyledButtons>
+ {user.userId && (
+ <FaveStar
+ itemId={slice.slice_id}
+ fetchFaveStar={actions.fetchFaveStar}
+ saveFaveStar={actions.saveFaveStar}
+ isStarred={isStarred}
+ showTooltip
+ />
+ )}
+ {isPropertiesModalOpen && (
+ <PropertiesModal
+ show={isPropertiesModalOpen}
+ onHide={closePropertiesModal}
+ onSave={sliceUpdated}
+ slice={slice}
+ />
+ )}
+ <Tooltip id="edit-desc-tooltip" title={t('Edit chart properties')}>
+ <span
+ aria-label={t('Edit chart properties')}
+ role="button"
+ tabIndex={0}
+ className="edit-desc-icon"
+ onClick={openPropertiesModal}
>
- <span
- aria-label={t('Edit chart properties')}
- role="button"
- tabIndex={0}
- className="edit-desc-icon"
- onClick={this.openPropertiesModal}
- >
- <i className="fa fa-edit" />
- </span>
- </Tooltip>
- {this.props.chart.sliceFormData && (
- <AlteredSliceTag
- className="altered"
- origFormData={this.props.chart.sliceFormData}
- currentFormData={formData}
- />
- )}
- </StyledButtons>
- )}
- </div>
- <div className="right-button-panel">
- {chartFinished && queryResponse && (
- <RowCountLabel
- rowcount={Number(queryResponse.rowcount) || 0}
- limit={Number(formData.row_limit) || 0}
- />
- )}
- {chartFinished && queryResponse && queryResponse.is_cached && (
- <CachedLabel
- onClick={this.postChartFormData.bind(this)}
- cachedTimestamp={queryResponse.cached_dttm}
- />
- )}
- <Timer
- startTime={chartUpdateStartTime}
- endTime={chartUpdateEndTime}
- isRunning={chartStatus === 'loading'}
- status={CHART_STATUS_MAP[chartStatus]}
+ <i className="fa fa-edit" />
+ </span>
+ </Tooltip>
+ {chart.sliceFormData && (
+ <AlteredSliceTag
+ className="altered"
+ origFormData={chart.sliceFormData}
+ currentFormData={formData}
+ />
+ )}
+ </StyledButtons>
+ )}
+ </div>
+ <div className="right-button-panel">
+ {chartFinished && queryResponse && (
+ <RowCountLabel
+ rowcount={Number(queryResponse.rowcount) || 0}
+ limit={Number(formData.row_limit) || 0}
/>
- {this.canAddReports() && this.renderReportModal()}
- <ReportModal
- show={this.state.showingReportModal}
- onHide={this.hideReportModal}
- props={{
- userId: this.props.user.userId,
- userEmail: this.props.user.email,
- chart: this.props.chart,
- creationMethod: 'charts',
- }}
+ )}
+ {chartFinished && queryResponse && queryResponse.is_cached && (
Review comment:
```suggestion
{chartFinished && queryResponse?.is_cached && (
```
This can be cleaned with [optional chaining](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Optional_chaining).
##########
File path: superset-frontend/src/explore/components/ExploreChartHeader/index.jsx
##########
@@ -169,53 +159,42 @@ export class ExploreChartHeader extends React.PureComponent {
}
})
.catch(() => {});
- }
+ };
- getSliceName() {
- const { sliceName, table_name: tableName } = this.props;
+ const getSliceName = () => {
const title = sliceName || t('%s - untitled', tableName);
return title;
- }
+ };
- postChartFormData() {
- this.props.actions.postChartFormData(
- this.props.form_data,
- true,
- this.props.timeout,
- this.props.chart.id,
- this.props.ownState,
- );
- }
+ const postChartFormData = () => {
+ actions.postChartFormData(formData, true, timeout, chart.id, ownState);
+ };
- openPropertiesModal() {
- this.setState({
- isPropertiesModalOpen: true,
- });
- }
+ const openPropertiesModal = () => {
+ setIsPropertiesModalOpen(true);
+ };
- closePropertiesModal() {
- this.setState({
- isPropertiesModalOpen: false,
- });
- }
+ const closePropertiesModal = () => {
+ setIsPropertiesModalOpen(false);
+ };
- showReportModal() {
- this.setState({ showingReportModal: true });
- }
+ const showReportModal = () => {
+ setShowingReportModal(true);
+ };
- hideReportModal() {
- this.setState({ showingReportModal: false });
- }
+ const hideReportModal = () => {
+ setShowingReportModal(false);
+ };
Review comment:
```suggestion
const hideReportModal = () => setShowingReportModal(false);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org