fix: undo/redo (microsoft#4688)

* Bot proj breadcrumbs (#4)

* remove old breadcrumbs and start making new ones

* Update DesignPage.tsx

* Update DesignPage.tsx

* update unit tests to remove breadcrumb things

* fix duplicate key bug in breadcrumbs

* fix e2e test

* detect and display action names in breadcrumb

* rewrite to make typechecker happy

* make new DesignPage unit tests

* Update publisher.ts

* Update publisher.ts

* restore navigation in undo

* retrieve breadcrumb from URL on location change

* read double-nested $designer fields

* navigate to trigger[0] on OpenDialog node events

* fix typo and unit tests

* Update validateDialogName.test.ts

* better error-checking for invalid URLs

* make special "beginDialog" trigger

* Update en-US.json

* Update DesignPage.tsx

Co-authored-by: Ben Yackley <61990921+beyackle@users.noreply.github.com>
Co-authored-by: Chris Whitten <christopher.whitten@microsoft.com>

* fix: undo/redo

* add location check and add fix unit tests

* Revert "Bot proj breadcrumbs (#4)"

This reverts commit 9a846eaf5c68e2ca2762e7b8938d9ce81d2947d3.

* add selector for undo

* move status

* fix unit test

Co-authored-by: Ben Yackley <61990921+beyackle@users.noreply.github.com>
Co-authored-by: Chris Whitten <christopher.whitten@microsoft.com>
Co-authored-by: Dong Lei <donglei@microsoft.com>

Composer/packages/client/src/pages/design/DesignPage.tsx

@@ -49,6 +49,7 @@ import { triggerNotSupported } from '../../utils/dialogValidator';
 import { undoFunctionState, undoVersionState } from '../../recoilModel/undo/history';
 import { decodeDesignerPathToArrayPath } from '../../utils/convertUtils/designerPathEncoder';
 import { useTriggerApi } from '../../shell/triggerApi';
+import { undoStatusSelectorFamily } from '../../recoilModel/selectors/undo';
 
 import { WarningMessage } from './WarningMessage';
 import {
@@ -131,7 +132,8 @@ const DesignPage: React.FC<RouteComponentProps<{ dialogId: string; projectId: st
   const undoVersion = useRecoilValue(undoVersionState(skillId ?? projectId));
   const rootProjectId = useRecoilValue(rootBotProjectIdSelector) ?? projectId;
 
-  const { undo, redo, canRedo, canUndo, commitChanges, clearUndo } = undoFunction;
+  const { undo, redo, commitChanges, clearUndo } = undoFunction;
+  const [canUndo, canRedo] = useRecoilValue(undoStatusSelectorFamily(projectId));
   const visualEditorSelection = useRecoilValue(visualEditorSelectionState);
   const {
     removeDialog,
@@ -384,8 +386,7 @@ const DesignPage: React.FC<RouteComponentProps<{ dialogId: string; projectId: st
     return { actionSelected, showDisableBtn, showEnableBtn };
   }, [visualEditorSelection, currentDialog?.content]);
 
-  const { onFocusFlowEditor, onBlurFlowEditor } = useElectronFeatures(actionSelected, canUndo?.(), canRedo?.());
-
+  const { onFocusFlowEditor, onBlurFlowEditor } = useElectronFeatures(actionSelected, canUndo, canRedo);
   const EditorAPI = getEditorAPI();
   const toolbarItems: IToolbarItem[] = [
     {
@@ -444,13 +445,13 @@ const DesignPage: React.FC<RouteComponentProps<{ dialogId: string; projectId: st
           {
             key: 'edit.undo',
             text: formatMessage('Undo'),
-            disabled: !canUndo?.(),
+            disabled: !canUndo,
             onClick: undo,
           },
           {
             key: 'edit.redo',
             text: formatMessage('Redo'),
-            disabled: !canRedo?.(),
+            disabled: !canRedo,
             onClick: redo,
           },
           {

Composer/packages/client/src/pages/design/PropertyEditor.tsx

@@ -38,7 +38,6 @@ const PropertyEditor: React.FC = () => {
   }, [currentDialog, focusedSteps[0]]);
 
   const [localData, setLocalData] = useState(dialogData as MicrosoftAdaptiveDialog);
-
   const syncData = useRef(
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
     debounce((shellData: any, localData: any) => {
@@ -103,8 +102,6 @@ const PropertyEditor: React.FC = () => {
     const id = setTimeout(() => {
       if (!isEqual(dialogData, localData)) {
         shellApi.saveData(localData, focusedSteps[0]);
-      } else {
-        shellApi.commitChanges?.();
       }
     }, 300);
 

Composer/packages/client/src/recoilModel/atoms/botState.ts

@@ -354,3 +354,13 @@ export const botNameIdentifierState = atomFamily<string, string>({
   key: getFullyQualifiedKey('botNameIdentifier'),
   default: '',
 });
+
+export const canUndoState = atomFamily<boolean, string>({
+  key: getFullyQualifiedKey('canUndoState'),
+  default: false,
+});
+
+export const canRedoState = atomFamily<boolean, string>({
+  key: getFullyQualifiedKey('canRedoState'),
+  default: false,
+});

Composer/packages/client/src/recoilModel/selectors/undo.ts

@@ -0,0 +1,14 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+import { selectorFamily } from 'recoil';
+
+import { canRedoState, canUndoState } from '../atoms/botState';
+
+export const undoStatusSelectorFamily = selectorFamily<[boolean, boolean], string>({
+  key: 'undoStatus',
+  get: (projectId: string) => ({ get }) => {
+    const canUndo = get(canUndoState(projectId));
+    const canRedo = get(canRedoState(projectId));
+    return [canUndo, canRedo];
+  },
+});

Composer/packages/client/src/recoilModel/undo/__test__/history.test.tsx

@@ -13,10 +13,14 @@ import {
   projectMetaDataState,
   currentProjectIdState,
   botProjectIdsState,
+  designPageLocationState,
+  canUndoState,
+  canRedoState,
 } from '../../atoms';
 import { dialogsSelectorFamily } from '../../selectors';
 import { renderRecoilHook } from '../../../../__tests__/testUtils/react-recoil-hooks-testing-library';
 import UndoHistory from '../undoHistory';
+import { undoStatusSelectorFamily } from '../../selectors/undo';
 const projectId = '123-asd';
 
 export const UndoRedoWrapper = () => {
@@ -30,11 +34,12 @@ describe('<UndoRoot/>', () => {
 
   beforeEach(() => {
     const useRecoilTestHook = () => {
-      const { undo, redo, canRedo, canUndo, commitChanges, clearUndo } = useRecoilValue(undoFunctionState(projectId));
+      const { undo, redo, commitChanges, clearUndo } = useRecoilValue(undoFunctionState(projectId));
       const [dialogs, setDialogs] = useRecoilState(dialogsSelectorFamily(projectId));
       const setProjectIdState = useSetRecoilState(currentProjectIdState);
+      const setDesignPageLocation = useSetRecoilState(designPageLocationState(projectId));
       const history = useRecoilValue(undoHistoryState(projectId));
-
+      const [canUndo, canRedo] = useRecoilValue(undoStatusSelectorFamily(projectId));
       return {
         undo,
         redo,
@@ -46,6 +51,7 @@ describe('<UndoRoot/>', () => {
         setDialogs,
         dialogs,
         history,
+        setDesignPageLocation,
       };
     };
 
@@ -60,18 +66,19 @@ describe('<UndoRoot/>', () => {
       },
       states: [
         { recoilState: botProjectIdsState, initialValue: [projectId] },
-        { recoilState: dialogsSelectorFamily(projectId), initialValue: [{ id: '1' }] },
+        { recoilState: dialogsSelectorFamily(projectId), initialValue: [{ id: '1', content: '' }] },
         { recoilState: lgFilesState(projectId), initialValue: [{ id: '1.lg' }, { id: '2' }] },
         { recoilState: luFilesState(projectId), initialValue: [{ id: '1.lu' }, { id: '2' }] },
         { recoilState: currentProjectIdState, initialValue: projectId },
         { recoilState: undoHistoryState(projectId), initialValue: new UndoHistory(projectId) },
+        { recoilState: canUndoState(projectId), initialValue: false },
+        { recoilState: canRedoState(projectId), initialValue: false },
+        { recoilState: designPageLocationState(projectId), initialValue: { dialogId: '1', focused: '', selected: '' } },
         {
           recoilState: undoFunctionState(projectId),
           initialValue: {
             undo: jest.fn(),
             redo: jest.fn(),
-            canUndo: jest.fn(),
-            canRedo: jest.fn(),
             commitChanges: jest.fn(),
             clearUndo: jest.fn(),
           },
@@ -107,14 +114,14 @@ describe('<UndoRoot/>', () => {
       renderedComponent.current.commitChanges();
     });
 
-    expect(renderedComponent.current.canUndo()).toBeTruthy();
+    expect(renderedComponent.current.canUndo).toBeTruthy();
 
     act(() => {
       renderedComponent.current.undo();
     });
     expect(renderedComponent.current.history.stack.length).toBe(2);
-    expect(renderedComponent.current.dialogs).toStrictEqual([{ id: '1' }]);
-    expect(renderedComponent.current.canRedo()).toBeTruthy();
+    expect(renderedComponent.current.dialogs).toStrictEqual([{ id: '1', content: '' }]);
+    expect(renderedComponent.current.canRedo).toBeTruthy();
   });
 
   it('should remove the items from present when commit a new change', () => {
@@ -132,24 +139,30 @@ describe('<UndoRoot/>', () => {
 
   it('should redo', () => {
     act(() => {
-      renderedComponent.current.setDialogs([{ id: '2' }]);
+      renderedComponent.current.setDialogs([{ id: '2', content: '' }]);
+    });
+
+    act(() => {
+      renderedComponent.current.setDesignPageLocation({ dialogId: '2' });
     });
+
     act(() => {
       renderedComponent.current.commitChanges();
     });
-    expect(renderedComponent.current.canRedo()).toBeFalsy();
+    expect(renderedComponent.current.canRedo).toBeFalsy();
 
     act(() => {
       renderedComponent.current.undo();
     });
-    expect(renderedComponent.current.dialogs).toStrictEqual([{ id: '1' }]);
-    expect(renderedComponent.current.canRedo()).toBeTruthy();
+
+    expect(renderedComponent.current.dialogs).toStrictEqual([{ id: '1', content: '' }]);
+    expect(renderedComponent.current.canRedo).toBeTruthy();
 
     act(() => {
       renderedComponent.current.redo();
     });
     expect(renderedComponent.current.history.stack.length).toBe(2);
-    expect(renderedComponent.current.dialogs).toStrictEqual([{ id: '2' }]);
+    expect(renderedComponent.current.dialogs).toStrictEqual([{ id: '2', content: '' }]);
   });
 
   it('should clear undo history', () => {

Composer/packages/client/src/recoilModel/undo/history.ts

@@ -10,18 +10,21 @@ import {
 import { atomFamily, Snapshot, useRecoilCallback, CallbackInterface, useSetRecoilState } from 'recoil';
 import uniqueId from 'lodash/uniqueId';
 import isEmpty from 'lodash/isEmpty';
+import has from 'lodash/has';
+import { DialogInfo } from '@bfc/shared';
 
 import { navigateTo, getUrlSearch } from '../../utils/navigation';
+import { encodeArrayPathToDesignerPath } from '../../utils/convertUtils/designerPathEncoder';
+import { dialogsSelectorFamily } from '../selectors';
 
-import { designPageLocationState } from './../atoms';
+import { rootBotProjectIdSelector } from './../selectors/project';
+import { canRedoState, canUndoState, designPageLocationState } from './../atoms';
 import { trackedAtoms, AtomAssetsMap } from './trackedAtoms';
 import UndoHistory from './undoHistory';
 
 type IUndoRedo = {
   undo: () => void;
   redo: () => void;
-  canUndo: () => boolean;
-  canRedo: () => boolean;
   commitChanges: () => void;
   clearUndo: () => void;
 };
@@ -44,8 +47,25 @@ export const undoVersionState = atomFamily({
   dangerouslyAllowMutability: true,
 });
 
+const checkLocation = (projectId: string, atomMap: AtomAssetsMap) => {
+  let location = atomMap.get(designPageLocationState(projectId));
+  const { dialogId, selected, focused } = location;
+  const dialog: DialogInfo = atomMap.get(dialogsSelectorFamily(projectId)).find((dialog) => dialogId === dialog.id);
+  if (!dialog) return atomMap;
+
+  const { content } = dialog;
+  if (!has(content, selected)) {
+    location = { ...location, selected: '', focused: '' };
+  } else if (!has(content, focused)) {
+    location = { ...location, focused: '' };
+  }
+
+  atomMap.set(designPageLocationState(projectId), location);
+  return atomMap;
+};
+
 const getAtomAssetsMap = (snap: Snapshot, projectId: string): AtomAssetsMap => {
-  const atomMap = new Map<RecoilState<any>, any>();
+  let atomMap = new Map<RecoilState<any>, any>();
   const atomsToBeTracked = trackedAtoms(projectId);
   atomsToBeTracked.forEach((atom) => {
     const loadable = snap.getLoadable(atom);
@@ -54,6 +74,7 @@ const getAtomAssetsMap = (snap: Snapshot, projectId: string): AtomAssetsMap => {
 
   //should record the location state
   atomMap.set(designPageLocationState(projectId), snap.getLoadable(designPageLocationState(projectId)).contents);
+  atomMap = checkLocation(projectId, atomMap);
   return atomMap;
 };
 
@@ -71,12 +92,22 @@ const checkAtomsChanged = (current: AtomAssetsMap, previous: AtomAssetsMap, atom
   return atoms.some((atom) => checkAtomChanged(current, previous, atom));
 };
 
-function navigate(next: AtomAssetsMap, projectId: string) {
-  const location = next.get(designPageLocationState(projectId));
-  if (location) {
+function navigate(next: AtomAssetsMap, skillId: string, projectId: string) {
+  const location = next.get(designPageLocationState(skillId));
+
+  if (location && projectId) {
     const { dialogId, selected, focused, promptTab } = location;
-    let currentUri = `/bot/${projectId}/dialogs/${dialogId}${getUrlSearch(selected, focused)}`;
-    if (promptTab) {
+    const dialog = next.get(dialogsSelectorFamily(skillId)).find((dialog) => dialogId === dialog.id);
+    const baseUri =
+      skillId == null || skillId === projectId
+        ? `/bot/${projectId}/dialogs/${dialogId}`
+        : `/bot/${projectId}/skill/${skillId}/dialogs/${dialogId}`;
+
+    let currentUri = `${baseUri}${getUrlSearch(
+      encodeArrayPathToDesignerPath(dialog.content, selected),
+      encodeArrayPathToDesignerPath(dialog.content, focused)
+    )}`;
+    if (promptTab && focused) {
       currentUri += `#${promptTab}`;
     }
     navigateTo(currentUri);
@@ -96,6 +127,11 @@ function mapTrackedAtomsOntoSnapshot(
       target = target.map(({ set }) => set(atom, next));
     }
   });
+
+  //add design page location to snapshot
+  target = target.map(({ set }) =>
+    set(designPageLocationState(projectId), nextAssets.get(designPageLocationState(projectId)))
+  );
   return target;
 }
 
@@ -112,13 +148,16 @@ interface UndoRootProps {
 export const UndoRoot = React.memo((props: UndoRootProps) => {
   const { projectId } = props;
   const undoHistory = useRecoilValue(undoHistoryState(projectId));
+  const rootBotProjectId = useRecoilValue(rootBotProjectIdSelector);
   const history: UndoHistory = useRef(undoHistory).current;
   const [initialStateLoaded, setInitialStateLoaded] = useState(false);
-
+  const setCanUndo = useSetRecoilState(canUndoState(projectId));
+  const setCanRedo = useSetRecoilState(canRedoState(projectId));
   const setUndoFunction = useSetRecoilState(undoFunctionState(projectId));
   const [, forceUpdate] = useState([]);
   const setVersion = useSetRecoilState(undoVersionState(projectId));
-
+  const rootBotId = useRef('');
+  rootBotId.current = rootBotProjectId || '';
   //use to record the first time change, this will help to get the init location
   //init location is used to undo navigate
   const assetsChanged = useRef(false);
@@ -156,7 +195,12 @@ export const UndoRoot = React.memo((props: UndoRootProps) => {
   ) => {
     target = mapTrackedAtomsOntoSnapshot(target, current, next, projectId);
     gotoSnapshot(target);
-    navigate(next, projectId);
+    navigate(next, projectId, rootBotId.current);
+  };
+
+  const updateUndoResult = () => {
+    setCanRedo(history.canRedo());
+    setCanUndo(history.canUndo());
   };
 
   const undo = useRecoilCallback(({ snapshot, gotoSnapshot }: CallbackInterface) => () => {
@@ -165,6 +209,7 @@ export const UndoRoot = React.memo((props: UndoRootProps) => {
       const next = history.undo();
       if (present) undoAssets(snapshot, present, next, gotoSnapshot, projectId);
       setVersion(uniqueId());
+      updateUndoResult();
     }
   });
 
@@ -174,24 +219,18 @@ export const UndoRoot = React.memo((props: UndoRootProps) => {
       const next = history.redo();
       if (present) undoAssets(snapshot, present, next, gotoSnapshot, projectId);
       setVersion(uniqueId());
+      updateUndoResult();
     }
   });
 
-  const canUndo = () => {
-    return history?.canUndo?.();
-  };
-
-  const canRedo = () => {
-    return history?.canRedo?.();
-  };
-
   const commit = useRecoilCallback(({ snapshot }) => () => {
     const currentAssets = getAtomAssetsMap(snapshot, projectId);
     const previousAssets = history.getPresentAssets();
     //filter some invalid changes
 
     if (previousAssets && checkAtomsChanged(currentAssets, previousAssets, trackedAtoms(projectId))) {
       history.add(getAtomAssetsMap(snapshot, projectId));
+      updateUndoResult();
     }
   });
 
@@ -208,7 +247,7 @@ export const UndoRoot = React.memo((props: UndoRootProps) => {
   });
 
   useEffect(() => {
-    setUndoFunction({ undo, redo, canRedo, canUndo, commitChanges, clearUndo });
+    setUndoFunction({ undo, redo, commitChanges, clearUndo });
   }, []);
 
   return null;

Composer/packages/ui-plugins/lg/src/LgField.tsx

@@ -78,6 +78,7 @@ const LgField: React.FC<FieldProps<string>> = (props) => {
       if (body) {
         updateLgTemplate(body);
         props.onChange(new LgTemplateRef(lgName).toString());
+        shellApi.commitChanges();
       } else {
         shellApi.removeLgTemplate(lgFileId, lgName);
         props.onChange();