Skip to content

Commit

Permalink
isDirty working for cancel flow
Browse files Browse the repository at this point in the history
Signed-off-by: abbyhu2000 <abigailhu2000@gmail.com>
  • Loading branch information
abbyhu2000 committed Jul 1, 2023
1 parent 6a96470 commit e9aba8f
Show file tree
Hide file tree
Showing 11 changed files with 133 additions and 74 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,15 @@ export const DashboardEditor = () => {
services,
isChromeVisible,
eventEmitter,
dashboard,
savedDashboardInstance,
appState
);

const { isEmbeddableRendered, currentAppState } = useEditorUpdates(
services,
eventEmitter,
dashboard,
savedDashboardInstance,
dashboardContainer,
appState
Expand All @@ -59,21 +61,27 @@ export const DashboardEditor = () => {
console.log('appStateData', appState?.getState());

Check failure on line 61 in src/plugins/dashboard/public/application/components/dashboard_editor.tsx

View workflow job for this annotation

GitHub Actions / Build and Verify on Linux

Unexpected console statement

Check failure on line 61 in src/plugins/dashboard/public/application/components/dashboard_editor.tsx

View workflow job for this annotation

GitHub Actions / Build and Verify on Windows

Unexpected console statement
console.log('currentAppState', currentAppState);

Check failure on line 62 in src/plugins/dashboard/public/application/components/dashboard_editor.tsx

View workflow job for this annotation

GitHub Actions / Build and Verify on Linux

Unexpected console statement

Check failure on line 62 in src/plugins/dashboard/public/application/components/dashboard_editor.tsx

View workflow job for this annotation

GitHub Actions / Build and Verify on Windows

Unexpected console statement
console.log('isEmbeddableRendered', isEmbeddableRendered);

Check failure on line 63 in src/plugins/dashboard/public/application/components/dashboard_editor.tsx

View workflow job for this annotation

GitHub Actions / Build and Verify on Linux

Unexpected console statement

Check failure on line 63 in src/plugins/dashboard/public/application/components/dashboard_editor.tsx

View workflow job for this annotation

GitHub Actions / Build and Verify on Windows

Unexpected console statement
console.log('app state isDirty', appState?.getState().isDirty);

Check failure on line 64 in src/plugins/dashboard/public/application/components/dashboard_editor.tsx

View workflow job for this annotation

GitHub Actions / Build and Verify on Linux

Unexpected console statement

Check failure on line 64 in src/plugins/dashboard/public/application/components/dashboard_editor.tsx

View workflow job for this annotation

GitHub Actions / Build and Verify on Windows

Unexpected console statement
console.log('dashboardContainer', dashboardContainer);

Check failure on line 65 in src/plugins/dashboard/public/application/components/dashboard_editor.tsx

View workflow job for this annotation

GitHub Actions / Build and Verify on Linux

Unexpected console statement

Check failure on line 65 in src/plugins/dashboard/public/application/components/dashboard_editor.tsx

View workflow job for this annotation

GitHub Actions / Build and Verify on Windows

Unexpected console statement

return (
<div>
<div>
{savedDashboardInstance && appState && dashboardContainer && currentAppState && (
<DashboardTopNav
isChromeVisible={isChromeVisible}
savedDashboardInstance={savedDashboardInstance}
stateContainer={appState}
currentAppState={currentAppState}
isEmbeddableRendered={isEmbeddableRendered}
dashboardContainer={dashboardContainer}
/>
)}
{savedDashboardInstance &&
appState &&
dashboardContainer &&
currentAppState &&
dashboard && (
<DashboardTopNav
isChromeVisible={isChromeVisible}
savedDashboardInstance={savedDashboardInstance}
stateContainer={appState}
dashboard={dashboard}
currentAppState={currentAppState}
isEmbeddableRendered={isEmbeddableRendered}
dashboardContainer={dashboardContainer}
/>
)}
</div>
</div>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,13 @@ import { DashboardAppStateContainer, DashboardAppState, DashboardServices } from
import { getNavActions } from '../utils/get_nav_actions';
import { DashboardContainer } from '../embeddable';
import { isErrorEmbeddable } from '../../embeddable_plugin';
import { Dashboard } from '../../dashboard';

interface DashboardTopNavProps {
isChromeVisible: boolean;
savedDashboardInstance: any;
stateContainer: DashboardAppStateContainer;
dashboard: Dashboard;
currentAppState: DashboardAppState;
isEmbeddableRendered: boolean;
dashboardContainer?: DashboardContainer;
Expand All @@ -36,6 +38,7 @@ const TopNav = ({
isChromeVisible,
savedDashboardInstance,
stateContainer,
dashboard,
currentAppState,
isEmbeddableRendered,
dashboardContainer,
Expand Down Expand Up @@ -83,6 +86,7 @@ const TopNav = ({
stateContainer,
savedDashboardInstance,
services,
dashboard,
dashboardContainer
);
setTopNavMenu(
Expand All @@ -101,6 +105,7 @@ const TopNav = ({
savedDashboardInstance,
stateContainer,
isEmbeddableRendered,
dashboard

Check failure on line 108 in src/plugins/dashboard/public/application/components/dashboard_top_nav.tsx

View workflow job for this annotation

GitHub Actions / Build and Verify on Linux

Insert `,`

Check failure on line 108 in src/plugins/dashboard/public/application/components/dashboard_top_nav.tsx

View workflow job for this annotation

GitHub Actions / Build and Verify on Windows

Insert `,`
]);

useEffect(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,5 +46,6 @@ export function getAppStateDefaults(
query: savedDashboard.getQuery(),
filters: savedDashboard.getFilters(),
viewMode: savedDashboard.id || hideWriteControls ? ViewMode.VIEW : ViewMode.EDIT,
isDirty: false,
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ export const createDashboardAppState = ({
[option]: value,
},
}),
// setDashboard: (state)
} as DashboardAppStateTransitions;
/*
make sure url ('_a') matches initial state
Expand Down
56 changes: 34 additions & 22 deletions src/plugins/dashboard/public/application/utils/get_nav_actions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import { DashboardContainer } from '../embeddable/dashboard_container';
import { DashboardConstants, createDashboardEditUrl } from '../../dashboard_constants';
import { unhashUrl } from '../../../../opensearch_dashboards_utils/public';
import { UrlParams } from '../components/dashboard_top_nav';
import { Dashboard } from '../../dashboard';

interface UrlParamsSelectedMap {
[UrlParams.SHOW_TOP_MENU]: boolean;
Expand All @@ -46,6 +47,7 @@ export const getNavActions = (
stateContainer: DashboardAppStateContainer,
savedDashboard: any,
services: DashboardServices,
dashboard: Dashboard,
dashboardContainer?: DashboardContainer
) => {
const {
Expand Down Expand Up @@ -292,40 +294,50 @@ export const getNavActions = (
function onChangeViewMode(newMode: ViewMode) {
const isPageRefresh = newMode === appState.viewMode;
const isLeavingEditMode = !isPageRefresh && newMode === ViewMode.VIEW;
// TODO: check if any query and filter changed
const willLoseChanges = isLeavingEditMode;
const willLoseChanges = isLeavingEditMode && stateContainer.getState().isDirty === true;

// If there are no changes, do not show the discard window
if (!willLoseChanges) {
stateContainer.transitions.set('viewMode', newMode);
return;
}

// If there are changes, show the discard window, and reset the states to original
function revertChangesAndExitEditMode() {
stateContainer.transitions.set('viewMode', ViewMode.VIEW);
const pathname = savedDashboard.id
? createDashboardEditUrl(savedDashboard.id)
: DashboardConstants.CREATE_NEW_DASHBOARD_URL;
history.push(pathname);

/* dashboardStateManager.resetState();
// This is only necessary for new dashboards, which will default to Edit mode.
updateViewMode(ViewMode.VIEW);
// We need to do a hard reset of the timepicker. appState will not reload like
// it does on 'open' because it's been saved to the url and the getAppState.previouslyStored() check on
// reload will cause it not to sync.
if (dashboardStateManager.getIsTimeSavedWithDashboard()) {
dashboardStateManager.syncTimefilterWithDashboardTime(timefilter);
dashboardStateManager.syncTimefilterWithDashboardRefreshInterval(timefilter);
}
// Angular's $location skips this update because of history updates from syncState which happen simultaneously
// when calling osdUrl.change() angular schedules url update and when angular finally starts to process it,
// the update is considered outdated and angular skips it
// so have to use implementation of dashboardStateManager.changeDashboardUrl, which workarounds those issues
dashboardStateManager.changeDashboardUrl(
dash.id ? createDashboardEditUrl(dash.id) : DashboardConstants.CREATE_NEW_DASHBOARD_URL
);*/
// This is only necessary for new dashboards, which will default to Edit mode.
stateContainer.transitions.set('viewMode', ViewMode.VIEW);

// We need to reset the app state to its original state
if (dashboard.panels) {
stateContainer.transitions.set('panels', dashboard.panels);
}

stateContainer.transitions.set('filters', dashboard.filters);
stateContainer.transitions.set('query', dashboard.query);
stateContainer.transitions.setOption('hidePanelTitles', dashboard.options.hidePanelTitles);
stateContainer.transitions.setOption('useMargins', dashboard.options.useMargins);

// Need to see if needed
stateContainer.transitions.set('timeRestore', dashboard.timeRestore);

// Since time filters are not tracked by app state, we need to manually reset it
if (stateContainer.getState().timeRestore) {
queryService.timefilter.timefilter.setTime({
from: dashboard.timeFrom,
to: dashboard.timeTo,
});
if (dashboard.refreshInterval) {
queryService.timefilter.timefilter.setRefreshInterval(dashboard.refreshInterval);
}
}

// Set the isDirty flag back to false since we discard all the changes
stateContainer.transitions.set('isDirty', false);
}

overlays
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import { DashboardAppStateContainer } from '../../../types';
import { migrateAppState, getAppStateDefaults } from '../../lib';
import { createDashboardAppState } from '../create_dashboard_app_state';
import { SavedObjectDashboard } from '../../../saved_dashboards';
import { Dashboard, DashboardParams } from '../../../dashboard';

/**
* This effect is responsible for instantiating the dashboard app state container,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import deepEqual from 'fast-deep-equal';
import { EventEmitter } from 'stream';
import { useEffect } from 'react';
import { i18n } from '@osd/i18n';
import _ from 'lodash';
import { IndexPattern, opensearchFilters } from '../../../../../data/public';
import {
DASHBOARD_CONTAINER_TYPE,
Expand Down Expand Up @@ -50,11 +51,13 @@ import { migrateLegacyQuery } from '../../lib/migrate_legacy_query';
import { getSavedObjectFinder } from '../../../../../saved_objects/public';
import { DashboardConstants } from '../../../dashboard_constants';
import { SavedObjectDashboard } from '../../../saved_dashboards';
import { Dashboard } from '../../../dashboard';

export const useDashboardContainer = (
services: DashboardServices,
isChromeVisible: boolean,
eventEmitter: EventEmitter,
dashboard?: Dashboard,
savedDashboardInstance?: SavedObjectDashboard,
appState?: DashboardAppStateContainer
) => {
Expand All @@ -63,11 +66,12 @@ export const useDashboardContainer = (
useEffect(() => {
const getDashboardContainer = async () => {
try {
if (savedDashboardInstance && appState) {
if (savedDashboardInstance && appState && dashboard) {
const dashboardContainerEmbeddable = await createDashboardEmbeddable(
savedDashboardInstance,
services,
appState
appState,
dashboard
);

setDashboardContainer(dashboardContainerEmbeddable);
Expand All @@ -83,7 +87,7 @@ export const useDashboardContainer = (
};

getDashboardContainer();
}, [savedDashboardInstance, appState, services]);
}, [savedDashboardInstance, appState, services, dashboard]);

useEffect(() => {
const incomingEmbeddable = services.embeddable
Expand All @@ -107,7 +111,8 @@ export const useDashboardContainer = (
const createDashboardEmbeddable = (
savedDash: any,
dashboardServices: DashboardServices,
appState: DashboardAppStateContainer
appState: DashboardAppStateContainer,
dashboard: Dashboard
) => {
let dashboardContainer: DashboardContainer;
let inputSubscription: Subscription | undefined;
Expand Down Expand Up @@ -343,7 +348,7 @@ const createDashboardEmbeddable = (
appState.transitions.set('query', queryStringManager.getQuery());
}
// triggered when dashboard embeddable container has changes, and update the appState
handleDashboardContainerChanges(container, appState, dashboardServices);
handleDashboardContainerChanges(container, appState, dashboardServices, dashboard);
});
return dashboardContainer;
}
Expand All @@ -355,7 +360,8 @@ const createDashboardEmbeddable = (
const handleDashboardContainerChanges = (
dashboardContainer: DashboardContainer,
appState: DashboardAppStateContainer,
dashboardServices: DashboardServices
dashboardServices: DashboardServices,
dashboard: Dashboard
) => {
let dirty = false;
let dirtyBecauseOfInitialStateMigration = false;
Expand All @@ -371,6 +377,7 @@ const handleDashboardContainerChanges = (
dirty = true;
}
});

const convertedPanelStateMap: { [key: string]: SavedDashboardPanel } = {};
Object.values(input.panels).forEach((panelState) => {
if (savedDashboardPanelMap[panelState.explicitInput.id] === undefined) {
Expand All @@ -397,8 +404,8 @@ const handleDashboardContainerChanges = (
});
if (dirty) {
appState.transitions.set('panels', Object.values(convertedPanelStateMap));
if (dirtyBecauseOfInitialStateMigration) {
// this.saveState({ replace: true });
if (!dirtyBecauseOfInitialStateMigration) {
appState.transitions.set('isDirty', true);
}
}
if (input.isFullScreenMode !== appStateData.fullScreenMode) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,21 @@ import { useEffect, useState } from 'react';
import { merge } from 'rxjs';
import { DashboardAppState, DashboardAppStateContainer, DashboardServices } from '../../../types';
import { DashboardContainer } from '../../embeddable';
import { Dashboard } from '../../../dashboard';

export const useEditorUpdates = (
services: DashboardServices,
eventEmitter: EventEmitter,
dashboard?: Dashboard,
dashboardInstance?: any,
dashboardContainer?: DashboardContainer,
appState?: DashboardAppStateContainer
) => {
const [isEmbeddableRendered, setIsEmbeddableRendered] = useState(false);
// We only mark dirty when there is changes in the panels, query, and filters
// We do not mark dirty for embed mode, view mode, full screen and etc
// The specific behaviors need to check the functional tests and previous dashboard
// const [hasUnsavedChanges, setHasUnsavedChanges] = useState(false);
const [currentAppState, setCurrentAppState] = useState<DashboardAppState>();
const dashboardDom = document.getElementById('dashboardViewport');

Expand All @@ -25,7 +31,7 @@ export const useEditorUpdates = (
} = services.data.query;

useEffect(() => {
if (appState && dashboardInstance && dashboardContainer) {
if (appState && dashboardInstance && dashboardContainer && dashboard) {
const initialState = appState.getState();
setCurrentAppState(initialState);

Expand All @@ -36,11 +42,17 @@ export const useEditorUpdates = (
);
if (changes) {
dashboardContainer.updateInput(changes);

if (changes.filters || changes.query || changes.timeRange || changes.refreshConfig) {
appState.transitions.set('isDirty', true);
}
}
}
};

const unsubscribeStateUpdates = appState.subscribe((state) => {
// If app state is changes, then set unsaved changes to true
// the only thing app state is not tracking is the time filter, need to check the previous dashboard if they count time filter change or not
setCurrentAppState(state);
refreshDashboardContainer();
});
Expand Down Expand Up @@ -69,6 +81,7 @@ export const useEditorUpdates = (
dashboardContainer,
isEmbeddableRendered,
timefilter,
dashboard,
]);

useEffect(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,10 @@ export const useSavedDashboardInstance = (

const getSavedDashboardInstance = async () => {
try {
let savedDashboardInstanceWithClass: any;
let savedDashboardInstance: any;

Check failure on line 47 in src/plugins/dashboard/public/application/utils/use/use_saved_dashboard_instance.ts

View workflow job for this annotation

GitHub Actions / Build and Verify on Linux

'savedDashboardInstance' is already declared in the upper scope

Check failure on line 47 in src/plugins/dashboard/public/application/utils/use/use_saved_dashboard_instance.ts

View workflow job for this annotation

GitHub Actions / Build and Verify on Windows

'savedDashboardInstance' is already declared in the upper scope
if (history.location.pathname === '/create') {
try {
savedDashboardInstanceWithClass = await getDashboardInstance(services);
savedDashboardInstance = await getDashboardInstance(services);
} catch {
redirectWhenMissing({
history,
Expand All @@ -61,11 +61,11 @@ export const useSavedDashboardInstance = (
}
} else if (dashboardIdFromUrl) {
try {
savedDashboardInstanceWithClass = await getDashboardInstance(
savedDashboardInstance = await getDashboardInstance(
services,
dashboardIdFromUrl
);
const { savedDashboard } = savedDashboardInstanceWithClass;
const { savedDashboard } = savedDashboardInstance;
// Update time filter to match the saved dashboard if time restore has been set to true when saving the dashboard
// We should only set the time filter according to time restore once when we are loading the dashboard
if (savedDashboard.timeRestore) {
Expand Down
Loading

0 comments on commit e9aba8f

Please sign in to comment.