diff --git a/.gitignore b/.gitignore index b3ef8b6db115..56a0086730eb 100644 --- a/.gitignore +++ b/.gitignore @@ -126,3 +126,5 @@ docker/*local* test-report.html superset/static/stats/statistics.html .aider* + +.cursor \ No newline at end of file diff --git a/superset-frontend/package-lock.json b/superset-frontend/package-lock.json index 793ca61edfdd..48240a0b4888 100644 --- a/superset-frontend/package-lock.json +++ b/superset-frontend/package-lock.json @@ -321,7 +321,10 @@ "eslint-rules/eslint-plugin-icons": { "version": "1.0.0", "dev": true, - "license": "Apache-2.0" + "license": "Apache-2.0", + "peerDependencies": { + "eslint": ">=0.8.0" + } }, "eslint-rules/eslint-plugin-theme-colors": { "version": "1.0.0", diff --git a/superset-frontend/src/components/UnsavedChangesModal/index.tsx b/superset-frontend/src/components/UnsavedChangesModal/index.tsx new file mode 100644 index 000000000000..de1d01e84546 --- /dev/null +++ b/superset-frontend/src/components/UnsavedChangesModal/index.tsx @@ -0,0 +1,100 @@ +/** + * 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, styled } from '@superset-ui/core'; +import { ReactNode } from 'react'; +import Modal from 'src/components/Modal'; +import Button from 'src/components/Button'; + +const DescriptionContainer = styled.div` + line-height: ${({ theme }) => theme.gridUnit * 4}px; + padding-top: ${({ theme }) => theme.gridUnit * 2}px; +`; + +interface UnsavedChangesModalProps { + description?: ReactNode; + onSave: () => void; + onDiscard: () => void; + onCancel: () => void; + show: boolean; + title?: ReactNode; + primaryButtonLoading?: boolean; +} + +export default function UnsavedChangesModal({ + description, + onSave, + onDiscard, + onCancel, + show, + title, + primaryButtonLoading = false, +}: UnsavedChangesModalProps) { + const defaultTitle = t('Unsaved changes'); + const defaultDescription = t( + 'You have unsaved changes. What would you like to do?', + ); + + const customFooter = ( + <> + + + + + ); + + return ( + + + {description || defaultDescription} + + + ); +} + + diff --git a/superset-frontend/src/dashboard/containers/DashboardPage.tsx b/superset-frontend/src/dashboard/containers/DashboardPage.tsx index fb8bb6c05d82..410d886b9436 100644 --- a/superset-frontend/src/dashboard/containers/DashboardPage.tsx +++ b/superset-frontend/src/dashboard/containers/DashboardPage.tsx @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import { createContext, lazy, FC, useEffect, useMemo, useRef } from 'react'; +import { createContext, lazy, FC, useEffect, useMemo, useRef, useState } from 'react'; import { Global } from '@emotion/react'; import { useHistory } from 'react-router-dom'; import { t, useTheme } from '@superset-ui/core'; @@ -24,6 +24,7 @@ import { useDispatch, useSelector } from 'react-redux'; import { createSelector } from '@reduxjs/toolkit'; import { useToasts } from 'src/components/MessageToasts/withToasts'; import Loading from 'src/components/Loading'; +import UnsavedChangesModal from 'src/components/UnsavedChangesModal'; import { useDashboard, useDashboardCharts, @@ -40,12 +41,17 @@ import { getActiveFilters } from 'src/dashboard/util/activeDashboardFilters'; import { LocalStorageKeys, setItem } from 'src/utils/localStorageHelpers'; import { URL_PARAMS } from 'src/constants'; import { getUrlParam } from 'src/utils/urlUtils'; -import { setDatasetsStatus } from 'src/dashboard/actions/dashboardState'; +import { + setDatasetsStatus, + saveDashboardRequest, + setUnsavedChanges, +} from 'src/dashboard/actions/dashboardState'; import { getFilterValue, getPermalinkValue, } from 'src/dashboard/components/nativeFilters/FilterBar/keyValue'; import DashboardContainer from 'src/dashboard/containers/Dashboard'; +import { SAVE_TYPE_OVERWRITE, DASHBOARD_HEADER_ID } from 'src/dashboard/util/constants'; import { nanoid } from 'nanoid'; import { RootState } from '../types'; @@ -214,7 +220,7 @@ export const DashboardPage: FC = ({ idOrSlug }: PageProps) => { // when dashboard unmounts or changes return injectCustomCss(css); } - return () => {}; + return () => { }; }, [css]); useEffect(() => { @@ -230,6 +236,160 @@ export const DashboardPage: FC = ({ idOrSlug }: PageProps) => { const relevantDataMask = useSelector(selectRelevantDatamask); const activeFilters = useSelector(selectActiveFilters); + // Navigation blocking for unsaved changes + const hasUnsavedChanges = useSelector( + state => !!state.dashboardState.hasUnsavedChanges, + ); + const editMode = useSelector( + state => !!state.dashboardState.editMode, + ); + const dashboardInfo = useSelector( + state => state.dashboardInfo, + ); + const layout = useSelector( + state => state.dashboardLayout.present, + ); + const dashboardState = useSelector( + state => state.dashboardState, + ); + + const [showUnsavedModal, setShowUnsavedModal] = useState(false); + const [pendingNavigation, setPendingNavigation] = useState( + null, + ); + const [isSaving, setIsSaving] = useState(false); + const unblockRef = useRef<(() => void) | null>(null); + + // Set up navigation blocking + useEffect(() => { + if (editMode && hasUnsavedChanges) { + // Block navigation when in edit mode with unsaved changes + // history.block() returns an unblock function + unblockRef.current = history.block((location, action) => { + // Only block if navigating to a different route + if (location.pathname !== history.location.pathname) { + setPendingNavigation(location.pathname); + setShowUnsavedModal(true); + // Return false to prevent navigation (this prevents browser prompt) + // We'll handle navigation manually via our modal + return ''; + } + // Allow navigation within the same route (e.g., hash changes) + return true; + }); + } else { + // Unblock navigation when not in edit mode or no unsaved changes + if (unblockRef.current) { + unblockRef.current(); + unblockRef.current = null; + } + } + + return () => { + if (unblockRef.current) { + unblockRef.current(); + unblockRef.current = null; + } + }; + }, [editMode, hasUnsavedChanges, history]); + + // Handle Save action + const handleSave = useMemo( + () => () => { + if (!dashboardInfo?.id) { + return; + } + + setIsSaving(true); + const currentColorNamespace = + dashboardInfo?.metadata?.color_namespace || + dashboardState.colorNamespace; + const currentColorScheme = + dashboardInfo?.metadata?.color_scheme || dashboardState.colorScheme; + const dashboardTitle = + layout[DASHBOARD_HEADER_ID]?.meta?.text || dashboardInfo.dashboard_title; + + const data = { + certified_by: dashboardInfo.certified_by, + certification_details: dashboardInfo.certification_details, + css: dashboardState.css || '', + dashboard_title: dashboardTitle, + last_modified_time: dashboardInfo.last_modified_time, + owners: dashboardInfo.owners, + roles: dashboardInfo.roles, + slug: dashboardInfo.slug, + metadata: { + ...dashboardInfo?.metadata, + color_namespace: currentColorNamespace, + color_scheme: currentColorScheme, + positions: layout, + refresh_frequency: dashboardState.shouldPersistRefreshFrequency + ? dashboardState.refreshFrequency + : dashboardInfo.metadata?.refresh_frequency, + }, + }; + + // Temporarily unblock to allow navigation after save + const currentUnblock = unblockRef.current; + if (currentUnblock) { + currentUnblock(); + unblockRef.current = null; + } + + dispatch( + saveDashboardRequest(data, dashboardInfo.id, SAVE_TYPE_OVERWRITE), + ) + .then(() => { + setIsSaving(false); + setShowUnsavedModal(false); + // Proceed with navigation after save + const navPath = pendingNavigation; + setPendingNavigation(null); + if (navPath) { + history.push(navPath); + } + }) + .catch(() => { + // Save failed or requires confirmation - keep modal open + setIsSaving(false); + // Don't navigate if save failed + }); + }, + [dashboardInfo, layout, dashboardState, dispatch, history, pendingNavigation], + ); + + // Handle Discard action + const handleDiscard = useMemo( + () => () => { + // Temporarily unblock to allow navigation + const currentUnblock = unblockRef.current; + if (currentUnblock) { + currentUnblock(); + unblockRef.current = null; + } + + // Clear unsaved changes flag + dispatch(setUnsavedChanges(false)); + setShowUnsavedModal(false); + // Proceed with navigation + const navPath = pendingNavigation; + setPendingNavigation(null); + if (navPath) { + history.push(navPath); + } + }, + [dispatch, history, pendingNavigation], + ); + + // Handle Cancel action + const handleCancel = useMemo( + () => () => { + setShowUnsavedModal(false); + setPendingNavigation(null); + }, + [], + ); + if (error) throw error; // caught in error boundary const globalStyles = useMemo( @@ -260,6 +420,13 @@ export const DashboardPage: FC = ({ idOrSlug }: PageProps) => { {DashboardBuilderComponent} + ) : ( diff --git a/superset-frontend/src/explore/components/PropertiesModal/index.tsx b/superset-frontend/src/explore/components/PropertiesModal/index.tsx index dba9031e81ea..5c401c09fdeb 100644 --- a/superset-frontend/src/explore/components/PropertiesModal/index.tsx +++ b/superset-frontend/src/explore/components/PropertiesModal/index.tsx @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import { ChangeEvent, useMemo, useState, useCallback, useEffect } from 'react'; +import { ChangeEvent, useMemo, useState, useCallback, useEffect, useRef } from 'react'; import Modal from 'src/components/Modal'; import { Input, TextArea } from 'src/components/Input'; @@ -51,6 +51,7 @@ export type PropertiesModalProps = { permissionsError?: string; existingOwners?: SelectValue; addSuccessToast: (msg: string) => void; + onFormChange?: (hasChanges: boolean) => void; }; const FormItem = AntdForm.Item; @@ -69,6 +70,7 @@ function PropertiesModal({ onSave, show, addSuccessToast, + onFormChange, }: PropertiesModalProps) { const theme = useTheme(); const [submitting, setSubmitting] = useState(false); @@ -80,6 +82,17 @@ function PropertiesModal({ ); const [tags, setTags] = useState([]); + + // Track original values for change detection + const originalValuesRef = useRef<{ + name: string; + description: string; + cache_timeout: number | null; + certified_by: string; + certification_details: string; + owners: SelectValue | null; + tags: TagType[]; + } | null>(null); const tagsAsSelectValues = useMemo(() => { const selectTags = tags.map((tag: { id: number; name: string }) => ({ @@ -209,11 +222,100 @@ function PropertiesModal({ fetchChartOwners(); }, [fetchChartOwners]); + // Track original values when modal opens and data is loaded + useEffect(() => { + if (show && selectedOwners !== null) { + // Wait for owners to be loaded before setting original values + originalValuesRef.current = { + name: slice.slice_name || '', + description: slice.description || '', + cache_timeout: slice.cache_timeout != null ? slice.cache_timeout : null, + certified_by: slice.certified_by || '', + certification_details: slice.certification_details || '', + owners: selectedOwners, + tags: [...tags], + }; + } else if (!show) { + originalValuesRef.current = null; + } + }, [show, slice.slice_id, selectedOwners, tags]); + // update name after it's changed in another modal useEffect(() => { setName(slice.slice_name || ''); }, [slice.slice_name]); + // Detect form changes and notify parent + useEffect(() => { + if (!show || !onFormChange || !originalValuesRef.current) { + if (onFormChange && !show) { + // Reset when modal closes + console.log('[PropertiesModal] Modal closed, resetting changes'); + onFormChange(false); + } + return; + } + + const checkForChanges = () => { + const formValues = form.getFieldsValue(); + const original = originalValuesRef.current!; + + // Compare owners by IDs + const currentOwnerIds = Array.isArray(selectedOwners) + ? (selectedOwners as { value: number }[]) + .map(o => o.value) + .sort() + : []; + const originalOwnerIds = Array.isArray(original.owners) + ? (original.owners as { value: number }[]) + .map(o => o.value) + .sort() + : []; + + // Compare tags by IDs + const currentTagIds = tags.map(t => t.id).sort(); + const originalTagIds = original.tags.map(t => t.id).sort(); + + const hasChanges = + name !== original.name || + (formValues.description || '') !== (original.description || '') || + (formValues.cache_timeout ?? null) !== original.cache_timeout || + (formValues.certified_by || '') !== (original.certified_by || '') || + (formValues.certification_details || '') !== (original.certification_details || '') || + JSON.stringify(currentOwnerIds) !== JSON.stringify(originalOwnerIds) || + JSON.stringify(currentTagIds) !== JSON.stringify(originalTagIds); + + console.log('[PropertiesModal] Change detection:', { + hasChanges, + nameChanged: name !== original.name, + descriptionChanged: (formValues.description || '') !== (original.description || ''), + cacheTimeoutChanged: (formValues.cache_timeout ?? null) !== original.cache_timeout, + ownersChanged: JSON.stringify(currentOwnerIds) !== JSON.stringify(originalOwnerIds), + tagsChanged: JSON.stringify(currentTagIds) !== JSON.stringify(originalTagIds), + originalValues: original, + currentValues: { + name, + description: formValues.description, + cache_timeout: formValues.cache_timeout, + owners: currentOwnerIds, + tags: currentTagIds, + }, + }); + + onFormChange(hasChanges); + }; + + // Check when values change + checkForChanges(); + + // Subscribe to form field changes + const unsubscribe = form.getFieldsValue(); + // Also check periodically for form field changes (AntdForm doesn't expose a change event) + const interval = setInterval(checkForChanges, 500); + + return () => clearInterval(interval); + }, [show, name, selectedOwners, tags, form, onFormChange]); + useEffect(() => { if (!isFeatureEnabled(FeatureFlag.TaggingSystem)) return; try { diff --git a/superset-frontend/src/pages/Chart/index.tsx b/superset-frontend/src/pages/Chart/index.tsx index 5bbeb5fa86c3..c3fb4f7aa745 100644 --- a/superset-frontend/src/pages/Chart/index.tsx +++ b/superset-frontend/src/pages/Chart/index.tsx @@ -16,9 +16,10 @@ * specific language governing permissions and limitations * under the License. */ -import { useEffect, useRef, useState } from 'react'; -import { useDispatch } from 'react-redux'; -import { useLocation } from 'react-router-dom'; +import { useEffect, useRef, useState, useMemo } from 'react'; +import { useDispatch, useSelector } from 'react-redux'; +import { useLocation, useHistory } from 'react-router-dom'; +import { isEqual, omit } from 'lodash'; import { getLabelsColorMap, isDefined, @@ -37,10 +38,16 @@ import { getAppliedFilterValues } from 'src/dashboard/util/activeDashboardFilter import { getParsedExploreURLParams } from 'src/explore/exploreUtils/getParsedExploreURLParams'; import { hydrateExplore } from 'src/explore/actions/hydrateExplore'; import ExploreViewContainer from 'src/explore/components/ExploreViewContainer'; -import { ExploreResponsePayload, SaveActionType } from 'src/explore/types'; +import { ExploreResponsePayload, SaveActionType, ExplorePageState } from 'src/explore/types'; import { fallbackExploreInitialData } from 'src/explore/fixtures'; import { getItem, LocalStorageKeys } from 'src/utils/localStorageHelpers'; import { getFormDataWithDashboardContext } from 'src/explore/controlUtils/getFormDataWithDashboardContext'; +import { getFormDataFromControls } from 'src/explore/controlUtils'; +import { QUERY_MODE_REQUISITES } from 'src/explore/constants'; +import { mergeExtraFormData } from 'src/dashboard/components/nativeFilters/utils'; +import UnsavedChangesModal from 'src/components/UnsavedChangesModal'; +import * as saveModalActions from 'src/explore/actions/saveModalActions'; +import { Location } from 'history'; const isValidResult = (rv: JsonObject): boolean => rv?.result?.form_data && rv?.result?.dataset; @@ -80,8 +87,8 @@ const fetchExploreData = async (exploreUrlParams: URLSearchParams) => { const clientError = await getClientErrorObject(err); throw new Error( clientError.message || - clientError.error || - t('Failed to load chart data.'), + clientError.error || + t('Failed to load chart data.'), ); } }; @@ -131,11 +138,237 @@ const getDashboardContextFormData = () => { return null; }; +const retainQueryModeRequirements = (hiddenFormData: any) => + Object.keys(hiddenFormData ?? {}).filter( + key => !QUERY_MODE_REQUISITES.has(key), + ); + export default function ExplorePage() { + console.log('🔵 EXPLORE PAGE LOADED - This should appear immediately'); + console.log('[Explore] Component rendered/loaded'); const [isLoaded, setIsLoaded] = useState(false); const isExploreInitialized = useRef(false); const dispatch = useDispatch(); const location = useLocation(); + const history = useHistory(); + + // Get explore state to detect unsaved changes + const explore = useSelector( + state => state.explore || ({} as ExplorePageState['explore']), + ); + const charts = useSelector( + state => state.charts || {}, + ); + const dataMask = useSelector( + state => state.dataMask || {}, + ); + + // Compute current form_data (same logic as ExploreViewContainer) + const currentFormData = useMemo(() => { + if (!explore.controls || !explore.datasource) { + return null; + } + const { controls, slice, hiddenFormData } = explore; + const hasQueryMode = !!controls.query_mode?.value; + const fieldsToOmit = hasQueryMode + ? retainQueryModeRequirements(hiddenFormData) + : Object.keys(hiddenFormData ?? {}); + const form_data: any = omit(getFormDataFromControls(controls), fieldsToOmit); + const slice_id = (form_data.slice_id ?? slice?.slice_id ?? 0) as number; + form_data.extra_form_data = mergeExtraFormData( + { ...form_data.extra_form_data }, + { + ...dataMask[slice_id]?.ownState, + }, + ); + return form_data; + }, [explore.controls, explore.slice, explore.hiddenFormData, dataMask]); + + // Get saved form_data (sliceFormData from chart) + const savedFormData = useMemo(() => { + if (!currentFormData) { + return null; + } + const slice_id = ((currentFormData as any).slice_id ?? explore.slice?.slice_id ?? 0) as number; + const chart = charts[slice_id]; + return chart?.sliceFormData || null; + }, [currentFormData, charts, explore.slice]); + + // Track initial form_data for new charts (no saved state) + const initialFormDataRef = useRef(null); + useEffect(() => { + if (currentFormData && !savedFormData && !initialFormDataRef.current) { + // Store initial form_data for new charts + initialFormDataRef.current = JSON.parse(JSON.stringify(currentFormData)); + } else if (savedFormData) { + // Reset initial form_data ref when chart is saved + initialFormDataRef.current = null; + } + }, [currentFormData, savedFormData]); + + // Detect unsaved changes + const hasUnsavedChanges = useMemo(() => { + if (!currentFormData) { + console.log('[Explore] No currentFormData, hasUnsavedChanges = false'); + return false; + } + const savedData = savedFormData || initialFormDataRef.current; + if (!savedData) { + // For new charts without initial data, check if form_data has meaningful content + // (e.g., has datasource and viz_type) + const hasContent = !!( + (currentFormData as any).datasource && + (currentFormData as any).viz_type && + Object.keys(currentFormData).length > 2 + ); + console.log('[Explore] No saved data, hasContent:', hasContent); + return hasContent; + } + // Compare current form_data with saved/initial form_data + // Exclude certain fields that change frequently but don't represent user changes + const fieldsToIgnore = ['url_params', 'extra_form_data']; + const current = omit(currentFormData, fieldsToIgnore); + const saved = omit(savedData, fieldsToIgnore); + const hasChanges = !isEqual(current, saved); + console.log('[Explore] Comparing form data:', { + hasChanges, + currentKeys: Object.keys(current).length, + savedKeys: Object.keys(saved).length, + }); + return hasChanges; + }, [currentFormData, savedFormData]); + + // Navigation blocking state + const [showUnsavedModal, setShowUnsavedModal] = useState(false); + const [pendingNavigation, setPendingNavigation] = useState( + null, + ); + const unblockRef = useRef<(() => void) | null>(null); + + // Set up navigation blocking + useEffect(() => { + console.log('[Explore] Navigation blocking effect:', { + hasUnsavedChanges, + currentPath: history.location.pathname, + }); + + if (hasUnsavedChanges) { + console.log('[Explore] Setting up navigation block'); + // Block navigation when there are unsaved changes + unblockRef.current = history.block((location: Location, action: string) => { + console.log('[Explore] Navigation blocked:', { + from: history.location.pathname, + to: location.pathname, + action, + }); + // Only block if navigating to a different route + if (location.pathname !== history.location.pathname) { + setPendingNavigation(location.pathname); + setShowUnsavedModal(true); + // Return empty string to prevent navigation (this prevents browser prompt) + // We'll handle navigation manually via our modal + return ''; + } + // Allow navigation within the same route (e.g., hash changes) + return true; + }); + } else { + console.log('[Explore] Unblocking navigation'); + // Unblock navigation when no unsaved changes + if (unblockRef.current) { + unblockRef.current(); + unblockRef.current = null; + } + } + + return () => { + if (unblockRef.current) { + console.log('[Explore] Cleaning up navigation block'); + unblockRef.current(); + unblockRef.current = null; + } + }; + }, [hasUnsavedChanges, history]); + + // Handle Save action - open save modal + const handleSave = useMemo( + () => () => { + // Open the save modal (ExploreViewContainer will handle the actual save) + dispatch(saveModalActions.setSaveChartModalVisibility(true)); + // Close unsaved changes modal + setShowUnsavedModal(false); + // Note: Navigation will be handled after save completes + // The save modal will handle navigation after successful save + }, + [dispatch], + ); + + // Handle Discard action - allow navigation without saving + const handleDiscard = useMemo( + () => () => { + // Temporarily unblock to allow navigation + const currentUnblock = unblockRef.current; + if (currentUnblock) { + currentUnblock(); + unblockRef.current = null; + } + + setShowUnsavedModal(false); + // Proceed with navigation (user explicitly chose to discard changes) + const navPath = pendingNavigation; + setPendingNavigation(null); + if (navPath) { + history.push(navPath); + } + }, + [history, pendingNavigation], + ); + + // Handle Cancel action + const handleCancel = useMemo( + () => () => { + setShowUnsavedModal(false); + setPendingNavigation(null); + }, + [], + ); + + // Debug: Log render state + useEffect(() => { + console.log('[Explore] Render state:', { + showUnsavedModal, + hasUnsavedChanges, + pendingNavigation, + }); + }, [showUnsavedModal, hasUnsavedChanges, pendingNavigation]); + + // Handle browser tab close/refresh (like Dashboard does) + // Only show browser prompt when custom modal is NOT showing + // This prevents double prompts when user clicks Cancel on custom modal + useEffect(() => { + const handleBeforeUnload = (e: BeforeUnloadEvent): string | undefined => { + // Don't show browser prompt if custom modal is already showing + // The custom modal handles in-app navigation, browser prompt only handles tab close/refresh + if (hasUnsavedChanges && !showUnsavedModal) { + const message = t('You have unsaved changes.'); + e.preventDefault(); + e.returnValue = message; // For Chrome + return message; // For Safari + } + return undefined; + }; + + // Only add listener if not in Cypress (as Dashboard does) + if (!(window as any).Cypress) { + window.addEventListener('beforeunload', handleBeforeUnload); + } + + return () => { + if (!(window as any).Cypress) { + window.removeEventListener('beforeunload', handleBeforeUnload); + } + }; + }, [hasUnsavedChanges, showUnsavedModal]); useEffect(() => { const exploreUrlParams = getParsedExploreURLParams(location); @@ -149,9 +382,9 @@ export default function ExplorePage() { const formData = !isExploreInitialized.current && dashboardContextFormData ? getFormDataWithDashboardContext( - result.form_data, - dashboardContextFormData, - ) + result.form_data, + dashboardContextFormData, + ) : result.form_data; dispatch( hydrateExplore({ @@ -176,5 +409,16 @@ export default function ExplorePage() { if (!isLoaded) { return ; } - return ; + return ( + <> + + + + ); } diff --git a/superset-frontend/src/pages/ChartList/index.tsx b/superset-frontend/src/pages/ChartList/index.tsx index b2158b09a2b1..5c426b2310e6 100644 --- a/superset-frontend/src/pages/ChartList/index.tsx +++ b/superset-frontend/src/pages/ChartList/index.tsx @@ -27,10 +27,11 @@ import { useTheme, css, } from '@superset-ui/core'; -import { useState, useMemo, useCallback } from 'react'; +import { useState, useMemo, useCallback, useEffect, useRef } from 'react'; import rison from 'rison'; import { uniqBy } from 'lodash'; import { useSelector } from 'react-redux'; +import { Location } from 'history'; import { createErrorHandler, createFetchRelated, @@ -59,6 +60,7 @@ import { dangerouslyGetItemDoNotUse } from 'src/utils/localStorageHelpers'; import withToasts from 'src/components/MessageToasts/withToasts'; import PropertiesModal from 'src/explore/components/PropertiesModal'; import ImportModelsModal from 'src/components/ImportModal/index'; +import UnsavedChangesModal from 'src/components/UnsavedChangesModal'; import Chart from 'src/types/Chart'; import Tag from 'src/types/TagType'; import { Tooltip } from 'src/components/Tooltip'; @@ -95,15 +97,15 @@ const FlexRowContainer = styled.div` const PAGE_SIZE = 25; const PASSWORDS_NEEDED_MESSAGE = t( 'The passwords for the databases below are needed in order to ' + - 'import them together with the charts. Please note that the ' + - '"Secure Extra" and "Certificate" sections of ' + - 'the database configuration are not present in export files, and ' + - 'should be added manually after the import if they are needed.', + 'import them together with the charts. Please note that the ' + + '"Secure Extra" and "Certificate" sections of ' + + 'the database configuration are not present in export files, and ' + + 'should be added manually after the import if they are needed.', ); const CONFIRM_OVERWRITE_MESSAGE = t( 'You are importing one or more charts that already exist. ' + - 'Overwriting might cause you to lose some of your work. Are you ' + - 'sure you want to overwrite?', + 'Overwriting might cause you to lose some of your work. Are you ' + + 'sure you want to overwrite?', ); const registry = getChartMetadataRegistry(); @@ -159,6 +161,8 @@ const StyledActions = styled.div` `; function ChartList(props: ChartListProps) { + console.log('🟢 CHART LIST LOADED - This should appear immediately'); + console.log('[ChartList] Component rendered/loaded'); const theme = useTheme(); const { addDangerToast, @@ -195,11 +199,163 @@ function ChartList(props: ChartListProps) { ); const { sliceCurrentlyEditing, - handleChartUpdated, + handleChartUpdated: originalHandleChartUpdated, openChartEditModal, closeChartEditModal, } = useChartEditModal(setCharts, charts); + // Wrap handleChartUpdated to reset unsaved changes + const handleChartUpdated = useCallback( + (chart: Chart) => { + originalHandleChartUpdated(chart); + setHasUnsavedChanges(false); + }, + [originalHandleChartUpdated], + ); + + // Unsaved changes tracking + const [hasUnsavedChanges, setHasUnsavedChanges] = useState(false); + const [showUnsavedModal, setShowUnsavedModal] = useState(false); + const [pendingNavigation, setPendingNavigation] = useState( + null, + ); + const unblockRef = useRef<(() => void) | null>(null); + + // Handle form change notification from PropertiesModal + const handleFormChange = useCallback((hasChanges: boolean) => { + console.log('[ChartList] handleFormChange called:', hasChanges, 'sliceCurrentlyEditing:', sliceCurrentlyEditing); + setHasUnsavedChanges(hasChanges); + }, [sliceCurrentlyEditing]); + + // Set up navigation blocking + useEffect(() => { + console.log('[ChartList] Navigation blocking effect:', { + hasUnsavedChanges, + sliceCurrentlyEditing, + currentPath: history.location.pathname, + }); + + if (hasUnsavedChanges && sliceCurrentlyEditing) { + console.log('[ChartList] Setting up navigation block'); + // Block navigation when there are unsaved changes + unblockRef.current = history.block((location: Location, action: string) => { + console.log('[ChartList] Navigation blocked:', { + from: history.location.pathname, + to: location.pathname, + action, + }); + // Only block if navigating to a different route + if (location.pathname !== history.location.pathname) { + setPendingNavigation(location.pathname); + setShowUnsavedModal(true); + // Return empty string to prevent navigation + return ''; + } + // Allow navigation within the same route + return true; + }); + } else { + console.log('[ChartList] Unblocking navigation'); + // Unblock navigation when no unsaved changes + if (unblockRef.current) { + unblockRef.current(); + unblockRef.current = null; + } + } + + return () => { + if (unblockRef.current) { + console.log('[ChartList] Cleaning up navigation block'); + unblockRef.current(); + unblockRef.current = null; + } + }; + }, [hasUnsavedChanges, sliceCurrentlyEditing, history]); + + // Reset unsaved changes when modal closes + useEffect(() => { + if (!sliceCurrentlyEditing) { + setHasUnsavedChanges(false); + setShowUnsavedModal(false); + setPendingNavigation(null); + } + }, [sliceCurrentlyEditing]); + + // Debug: Log render state + useEffect(() => { + console.log('[ChartList] Render state:', { + showUnsavedModal, + hasUnsavedChanges, + sliceCurrentlyEditing: !!sliceCurrentlyEditing, + pendingNavigation, + }); + }, [showUnsavedModal, hasUnsavedChanges, sliceCurrentlyEditing, pendingNavigation]); + + // Handle browser tab close/refresh (like Dashboard does) + // Only show browser prompt when custom modal is NOT showing + // This prevents double prompts when user clicks Cancel on custom modal + useEffect(() => { + const handleBeforeUnload = (e: BeforeUnloadEvent): string | undefined => { + // Don't show browser prompt if custom modal is already showing + // The custom modal handles in-app navigation, browser prompt only handles tab close/refresh + if (hasUnsavedChanges && sliceCurrentlyEditing && !showUnsavedModal) { + const message = t('You have unsaved changes.'); + e.preventDefault(); + e.returnValue = message; // For Chrome + return message; // For Safari + } + return undefined; + }; + + // Only add listener if not in Cypress (as Dashboard does) + if (!(window as any).Cypress) { + window.addEventListener('beforeunload', handleBeforeUnload); + } + + return () => { + if (!(window as any).Cypress) { + window.removeEventListener('beforeunload', handleBeforeUnload); + } + }; + }, [hasUnsavedChanges, sliceCurrentlyEditing, showUnsavedModal]); + + // Handle Save action - save via PropertiesModal + const handleSave = useCallback(() => { + // Close unsaved changes modal + setShowUnsavedModal(false); + setPendingNavigation(null); + // The PropertiesModal will handle the actual save + // User needs to click Save in the PropertiesModal + }, []); + + // Handle Discard action - close PropertiesModal and allow navigation + const handleDiscard = useCallback(() => { + // Temporarily unblock to allow navigation + const currentUnblock = unblockRef.current; + if (currentUnblock) { + currentUnblock(); + unblockRef.current = null; + } + + // Close PropertiesModal (discarding changes) + closeChartEditModal(); + setHasUnsavedChanges(false); + setShowUnsavedModal(false); + + // Proceed with navigation + const navPath = pendingNavigation; + setPendingNavigation(null); + if (navPath) { + history.push(navPath); + } + }, [history, pendingNavigation, closeChartEditModal]); + + // Handle Cancel action + const handleCancel = useCallback(() => { + setShowUnsavedModal(false); + setPendingNavigation(null); + }, []); + const [importingChart, showImportModal] = useState(false); const [passwordFields, setPasswordFields] = useState([]); const [preparingExport, setPreparingExport] = useState(false); @@ -271,14 +427,14 @@ function ChartList(props: ChartListProps) { // add filters if filterValue const filters = filterValue ? { - filters: [ - { - col: 'dashboard_title', - opr: FilterOperator.StartsWith, - value: filterValue, - }, - ], - } + filters: [ + { + col: 'dashboard_title', + opr: FilterOperator.StartsWith, + value: filterValue, + }, + ], + } : {}; const queryParams = rison.encode({ columns: ['dashboard_title', 'id'], @@ -615,16 +771,16 @@ function ChartList(props: ChartListProps) { }, ...(isFeatureEnabled(FeatureFlag.TaggingSystem) && canReadTag ? [ - { - Header: t('Tag'), - key: 'tags', - id: 'tags', - input: 'select', - operator: FilterOperator.ChartTagById, - unfilteredLabel: t('All'), - fetchSelects: loadTags, - }, - ] + { + Header: t('Tag'), + key: 'tags', + id: 'tags', + input: 'select', + operator: FilterOperator.ChartTagById, + unfilteredLabel: t('All'), + fetchSelects: loadTags, + }, + ] : []), { Header: t('Owner'), @@ -805,8 +961,19 @@ function ChartList(props: ChartListProps) { onSave={handleChartUpdated} show slice={sliceCurrentlyEditing} + onFormChange={(hasChanges: boolean) => { + console.log('[ChartList] PropertiesModal onFormChange:', hasChanges); + handleFormChange(hasChanges); + }} /> )} +