Skip to content

Commit

Permalink
Fix calling cleanup func unnecessarily
Browse files Browse the repository at this point in the history
  • Loading branch information
kgabryje committed May 10, 2022
1 parent b303aaa commit b061dd4
Showing 1 changed file with 14 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,11 @@ 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];
Expand Down Expand Up @@ -378,24 +383,26 @@ function ExploreViewContainer(props) {
}, [handleKeydown, previousHandleKeyDown]);

useEffect(() => {
const handleCloseEvent = e => {
e.preventDefault();
e.returnValue = 'Controls changed';
};
const formDataChanged = !isEmpty(
getFormDataDiffs(props.chart.sliceFormData, props.form_data),
);
if (formDataChanged && !isBeforeUnloadActive.current) {
window.addEventListener('beforeunload', handleCloseEvent);
window.addEventListener('beforeunload', handleUnloadEvent);
isBeforeUnloadActive.current = true;
}
if (!formDataChanged && isBeforeUnloadActive.current) {
window.removeEventListener('beforeunload', handleCloseEvent);
window.removeEventListener('beforeunload', handleUnloadEvent);
isBeforeUnloadActive.current = false;
}
return () => window.removeEventListener('beforeunload', handleCloseEvent);
}, [props.chart.sliceFormData, 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
Expand Down

0 comments on commit b061dd4

Please sign in to comment.