Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fix Visual Editor nested flow flickering issue by improving layout cache management #2176

Merged
merged 10 commits into from
Mar 24, 2020
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

import { insert, deleteNode, queryNode } from '../../src/utils/jsonTracker';
import { insert, deleteNode, queryNode, getParentPaths } from '../../src/utils/jsonTracker';

describe('queryNode', () => {
describe('can query correct result', () => {
Expand Down Expand Up @@ -161,3 +161,13 @@ describe('delete node flow', () => {
});
});
});

describe('getParentPaths', () => {
it('can generate correct parent paths.', () => {
expect(getParentPaths('a')).toEqual([]);
expect(getParentPaths('a.b')).toEqual(['a']);
expect(getParentPaths('a.b.c')).toEqual(['a', 'a.b']);
expect(getParentPaths('a.b.c.d')).toEqual(['a', 'a.b', 'a.b.c']);
expect(getParentPaths('triggers[0].actions[0].actions')).toEqual(['triggers[0]', 'triggers[0].actions[0]']);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { jsx } from '@emotion/core';
import { useContext, FC, useEffect, useState, useRef } from 'react';
import { MarqueeSelection, Selection } from 'office-ui-fabric-react/lib/MarqueeSelection';
import {
SDKTypes,
deleteAction,
deleteActions,
LgTemplateRef,
Expand All @@ -14,7 +15,7 @@ import {
ExternalResourceHandlerAsync,
walkLgResourcesInActionList,
} from '@bfc/shared';
import { SDKTypes } from '@bfc/shared';
import get from 'lodash/get';

import { NodeEventTypes } from '../constants/NodeEventTypes';
import { KeyboardCommandTypes, KeyboardPrimaryTypes } from '../constants/KeyboardCommandTypes';
Expand All @@ -29,13 +30,15 @@ import {
appendNodesAfter,
pasteNodes,
deleteNodes,
getParentPaths,
insertAction,
} from '../utils/jsonTracker';
import { moveCursor, querySelectableElements, SelectorElement } from '../utils/cursorTracker';
import { NodeIndexGenerator } from '../utils/NodeIndexGetter';
import { normalizeSelection } from '../utils/normalizeSelection';
import { KeyboardZone } from '../components/lib/KeyboardZone';
import { scrollNodeIntoView } from '../utils/nodeOperation';
import { designerCache } from '../store/DesignerCache';

import { AdaptiveDialogEditor } from './AdaptiveDialogEditor';

Expand Down Expand Up @@ -102,6 +105,19 @@ export const ObiEditor: FC<ObiEditorProps> = ({
return removeLgTemplates(lgFileId, normalizedLgTemplates);
};

const trackActionChange = (actionPath: string) => {
const affectedPaths = getParentPaths(actionPath);
for (const path of affectedPaths) {
const json = get(data, path);
designerCache.uncacheBoundary(json);
}
};

const trackActionListChange = (actionPaths: string[]) => {
if (!Array.isArray(actionPaths)) return;
actionPaths.forEach(x => trackActionChange(x));
};

const deleteLuIntents = (luIntents: string[]) => {
return Promise.all(luIntents.map(intent => removeLuIntent(path, intent)));
};
Expand All @@ -126,12 +142,14 @@ export const ObiEditor: FC<ObiEditorProps> = ({
handler = ({ caller, callee }) => onOpen(callee, caller);
break;
case NodeEventTypes.Delete:
trackActionChange(eventData.id);
handler = e => {
onChange(deleteNode(data, e.id, node => deleteAction(node, deleteLgTemplates, deleteLuIntents)));
onFocusSteps([]);
};
break;
case NodeEventTypes.Insert:
trackActionChange(eventData.id);
if (eventData.$type === 'PASTE') {
handler = e => {
pasteNodes(data, e.id, e.position, clipboardActions, buildLgReference).then(dialog => {
Expand Down Expand Up @@ -159,6 +177,7 @@ export const ObiEditor: FC<ObiEditorProps> = ({
};
break;
case NodeEventTypes.CutSelection:
trackActionListChange(eventData.actionIds);
handler = e => {
cutNodes(data, e.actionIds, dereferenceLg, nodes =>
deleteActions(nodes, deleteLgTemplates, deleteLuIntents)
Expand Down Expand Up @@ -221,6 +240,7 @@ export const ObiEditor: FC<ObiEditorProps> = ({
};
break;
case NodeEventTypes.DeleteSelection:
trackActionListChange(eventData.actionIds);
handler = e => {
const dialog = deleteNodes(data, e.actionIds, nodes =>
deleteActions(nodes, deleteLgTemplates, deleteLuIntents)
Expand All @@ -230,6 +250,7 @@ export const ObiEditor: FC<ObiEditorProps> = ({
};
break;
case NodeEventTypes.AppendSelection:
trackActionListChange(eventData.target);
handler = e => {
// forbid paste to root level.
if (!e.target || e.target === focusedEvent) return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,13 @@ export class DesignerCache {
return true;
}

uncacheBoundary(actionData: BaseSchema): boolean {
const key = this.getActionDataHash(actionData);
if (!key) return false;

return delete this.boundaryCache[key];
}

loadBounary(actionData: BaseSchema): Boundary | undefined {
const key = this.getActionDataHash(actionData);
if (key) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -235,3 +235,19 @@ export async function pasteNodes(
const newNodes = await deepCopyActions(clipboardNodes, handleLgField);
return insertNodes(inputDialog, arrayPath, arrayIndex, newNodes);
}

export const getParentPaths = (actionPath: string): string[] => {
if (typeof actionPath !== 'string') return [];
const selectors = actionPath.split('.');
// exclude the path of current action
selectors.pop();
if (!selectors.length) return [];

let path = selectors[0];
const results = [path];
for (let i = 1; i < selectors.length; i++) {
path = `${path}.${selectors[i]}`;
results.push(path);
}
return results;
};