Revert "feat(explore): Show confirmation modal if user exits Explore without saving changes (#19993)" (#20092)

This reverts commit ca9766c109.
This commit is contained in:
Kamil Gabryjelski 2022-05-17 14:56:31 +02:00 committed by GitHub
parent e69f6292c2
commit 9cdaa28042
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 76 additions and 119 deletions

View File

@ -308,4 +308,34 @@ describe('AlteredSliceTag', () => {
).toBe(expected);
});
});
describe('isEqualish', () => {
it('considers null, undefined, {} and [] as equal', () => {
const inst = wrapper.instance();
expect(inst.isEqualish(null, undefined)).toBe(true);
expect(inst.isEqualish(null, [])).toBe(true);
expect(inst.isEqualish(null, {})).toBe(true);
expect(inst.isEqualish(undefined, {})).toBe(true);
});
it('considers empty strings are the same as null', () => {
const inst = wrapper.instance();
expect(inst.isEqualish(undefined, '')).toBe(true);
expect(inst.isEqualish(null, '')).toBe(true);
});
it('considers deeply equal objects as equal', () => {
const inst = wrapper.instance();
expect(inst.isEqualish('', '')).toBe(true);
expect(inst.isEqualish({ a: 1, b: 2, c: 3 }, { a: 1, b: 2, c: 3 })).toBe(
true,
);
// Out of order
expect(inst.isEqualish({ a: 1, b: 2, c: 3 }, { b: 2, a: 1, c: 3 })).toBe(
true,
);
// Actually not equal
expect(inst.isEqualish({ a: 1, b: 2, z: 9 }, { a: 1, b: 2, c: 3 })).toBe(
false,
);
});
});
});

View File

@ -20,7 +20,7 @@ import React from 'react';
import PropTypes from 'prop-types';
import { isEqual, isEmpty } from 'lodash';
import { styled, t } from '@superset-ui/core';
import { getFormDataDiffs } from 'src/explore/exploreUtils/formData';
import { sanitizeFormData } from 'src/explore/exploreUtils/formData';
import getControlsForVizType from 'src/utils/getControlsForVizType';
import { safeStringify } from 'src/utils/safeStringify';
import { Tooltip } from 'src/components/Tooltip';
@ -44,6 +44,24 @@ const StyledLabel = styled.span`
`}
`;
function alterForComparison(value) {
// Considering `[]`, `{}`, `null` and `undefined` as identical
// for this purpose
if (value === undefined || value === null || value === '') {
return null;
}
if (typeof value === 'object') {
if (Array.isArray(value) && value.length === 0) {
return null;
}
const keys = Object.keys(value);
if (keys && keys.length === 0) {
return null;
}
}
return value;
}
export default class AlteredSliceTag extends React.Component {
constructor(props) {
super(props);
@ -77,7 +95,27 @@ export default class AlteredSliceTag extends React.Component {
getDiffs(props) {
// Returns all properties that differ in the
// current form data and the saved form data
return getFormDataDiffs(props.origFormData, props.currentFormData);
const ofd = sanitizeFormData(props.origFormData);
const cfd = sanitizeFormData(props.currentFormData);
const fdKeys = Object.keys(cfd);
const diffs = {};
fdKeys.forEach(fdKey => {
if (!ofd[fdKey] && !cfd[fdKey]) {
return;
}
if (['filters', 'having', 'having_filters', 'where'].includes(fdKey)) {
return;
}
if (!this.isEqualish(ofd[fdKey], cfd[fdKey])) {
diffs[fdKey] = { before: ofd[fdKey], after: cfd[fdKey] };
}
});
return diffs;
}
isEqualish(val1, val2) {
return isEqual(alterForComparison(val1), alterForComparison(val2));
}
formatValue(value, key, controlsMap) {

View File

@ -17,18 +17,12 @@
* under the License.
*/
/* eslint camelcase: 0 */
import React, {
useCallback,
useEffect,
useMemo,
useRef,
useState,
} from 'react';
import React, { useCallback, useEffect, useMemo, useState } from 'react';
import PropTypes from 'prop-types';
import { bindActionCreators } from 'redux';
import { connect } from 'react-redux';
import { styled, t, css, useTheme, logging } from '@superset-ui/core';
import { debounce, pick, isEmpty } from 'lodash';
import { debounce, pick } from 'lodash';
import { Resizable } from 're-resizable';
import { useChangeEffect } from 'src/hooks/useChangeEffect';
import { usePluginContext } from 'src/components/DynamicPlugins';
@ -49,11 +43,7 @@ import * as chartActions from 'src/components/Chart/chartAction';
import { fetchDatasourceMetadata } from 'src/dashboard/actions/datasources';
import { chartPropShape } from 'src/dashboard/util/propShapes';
import { mergeExtraFormData } from 'src/dashboard/components/nativeFilters/utils';
import {
getFormDataDiffs,
postFormData,
putFormData,
} from 'src/explore/exploreUtils/formData';
import { postFormData, putFormData } from 'src/explore/exploreUtils/formData';
import { useTabId } from 'src/hooks/useTabId';
import ExploreChartPanel from '../ExploreChartPanel';
import ConnectedControlPanelsContainer from '../ControlPanelsContainer';
@ -226,11 +216,6 @@ const updateHistory = debounce(
1000,
);
const handleUnloadEvent = e => {
e.preventDefault();
e.returnValue = 'Controls changed';
};
function ExploreViewContainer(props) {
const dynamicPluginContext = usePluginContext();
const dynamicPlugin = dynamicPluginContext.dynamicPlugins[props.vizType];
@ -251,9 +236,6 @@ function ExploreViewContainer(props) {
const theme = useTheme();
const isBeforeUnloadActive = useRef(false);
const initialFormData = useRef(props.form_data);
const defaultSidebarsWidth = {
controls_width: 320,
datasource_width: 300,
@ -383,27 +365,6 @@ function ExploreViewContainer(props) {
};
}, [handleKeydown, previousHandleKeyDown]);
useEffect(() => {
const formDataChanged = !isEmpty(
getFormDataDiffs(initialFormData.current, props.form_data),
);
if (formDataChanged && !isBeforeUnloadActive.current) {
window.addEventListener('beforeunload', handleUnloadEvent);
isBeforeUnloadActive.current = true;
}
if (!formDataChanged && isBeforeUnloadActive.current) {
window.removeEventListener('beforeunload', handleUnloadEvent);
isBeforeUnloadActive.current = false;
}
}, [props.form_data]);
// cleanup beforeunload event listener
// we use separate useEffect to call it only on component unmount instead of on every form data change
useEffect(
() => () => window.removeEventListener('beforeunload', handleUnloadEvent),
[],
);
useEffect(() => {
if (wasDynamicPluginLoading && !isDynamicPluginLoading) {
// reload the controls now that we actually have the control config

View File

@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
import { sanitizeFormData, isEqualish } from './formData';
import { sanitizeFormData } from './formData';
test('sanitizeFormData removes temporary control values', () => {
expect(
@ -26,25 +26,3 @@ test('sanitizeFormData removes temporary control values', () => {
}),
).toEqual({ metrics: ['foo', 'bar'] });
});
test('isEqualish', () => {
// considers null, undefined, {} and [] as equal
expect(isEqualish(null, undefined)).toBe(true);
expect(isEqualish(null, [])).toBe(true);
expect(isEqualish(null, {})).toBe(true);
expect(isEqualish(undefined, {})).toBe(true);
// considers empty strings are the same as null
expect(isEqualish(undefined, '')).toBe(true);
expect(isEqualish(null, '')).toBe(true);
// considers deeply equal objects as equal
expect(isEqualish('', '')).toBe(true);
expect(isEqualish({ a: 1, b: 2, c: 3 }, { a: 1, b: 2, c: 3 })).toBe(true);
// Out of order
expect(isEqualish({ a: 1, b: 2, c: 3 }, { b: 2, a: 1, c: 3 })).toBe(true);
// Actually not equal
expect(isEqualish({ a: 1, b: 2, z: 9 }, { a: 1, b: 2, c: 3 })).toBe(false);
});

View File

@ -16,8 +16,8 @@
* specific language governing permissions and limitations
* under the License.
*/
import { isEqual, omit } from 'lodash';
import { SupersetClient, JsonObject, JsonValue } from '@superset-ui/core';
import { omit } from 'lodash';
import { SupersetClient, JsonObject } from '@superset-ui/core';
type Payload = {
dataset_id: number;
@ -30,56 +30,6 @@ const TEMPORARY_CONTROLS = ['url_params'];
export const sanitizeFormData = (formData: JsonObject): JsonObject =>
omit(formData, TEMPORARY_CONTROLS);
export const alterForComparison = (value: JsonValue | undefined) => {
// Considering `[]`, `{}`, `null` and `undefined` as identical
// for this purpose
if (
value === undefined ||
value === null ||
value === '' ||
(Array.isArray(value) && value.length === 0) ||
(typeof value === 'object' && Object.keys(value).length === 0)
) {
return null;
}
if (Array.isArray(value)) {
// omit prototype for comparison of class instances with json objects
return value.map(v => (typeof v === 'object' ? omit(v, ['__proto__']) : v));
}
if (typeof value === 'object') {
return omit(value, ['__proto__']);
}
return value;
};
export const isEqualish = (
val1: JsonValue | undefined,
val2: JsonValue | undefined,
) => isEqual(alterForComparison(val1), alterForComparison(val2));
export const getFormDataDiffs = (
formData1: JsonObject,
formData2: JsonObject,
) => {
const ofd = sanitizeFormData(formData1);
const cfd = sanitizeFormData(formData2);
const fdKeys = Object.keys(cfd);
const diffs = {};
fdKeys.forEach(fdKey => {
if (!ofd[fdKey] && !cfd[fdKey]) {
return;
}
if (['filters', 'having', 'having_filters', 'where'].includes(fdKey)) {
return;
}
if (!isEqualish(ofd[fdKey], cfd[fdKey])) {
diffs[fdKey] = { before: ofd[fdKey], after: cfd[fdKey] };
}
});
return diffs;
};
const assembleEndpoint = (key?: string, tabId?: string) => {
let endpoint = 'api/v1/explore/form_data';
if (key) {