From d558846afb7251a0288a3a47c022ec6620fc52c1 Mon Sep 17 00:00:00 2001 From: leileizhang Date: Thu, 5 Nov 2020 16:09:43 +0800 Subject: [PATCH 1/7] 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 --- .../cypress/integration/Breadcrumb.spec.ts | 4 +- .../hooks/useEditorEventApi.ts | 4 +- .../__tests__/pages/design/Design.test.tsx | 65 +++++ .../client/__tests__/utils/navigation.test.ts | 50 +--- .../components/ProjectTree/ProjectTree.tsx | 109 ++++---- .../client/src/pages/design/DesignPage.tsx | 243 +++++++++++------- .../src/pages/design/PropertyEditor.tsx | 2 +- .../client/src/recoilModel/atoms/botState.ts | 9 +- .../dispatchers/__tests__/navigation.test.tsx | 59 +---- .../src/recoilModel/dispatchers/navigation.ts | 50 ++-- .../src/recoilModel/dispatchers/publisher.ts | 5 +- .../packages/client/src/recoilModel/types.ts | 9 +- .../client/src/recoilModel/undo/history.ts | 12 +- .../packages/client/src/shell/useShell.ts | 6 +- .../utils/convertUtils/designerPathEncoder.ts | 7 +- .../packages/client/src/utils/navigation.ts | 36 +-- .../dialogUtils/validateDialogName.test.ts | 2 +- .../src/dialogUtils/validateDialogName.ts | 2 +- .../packages/server/src/locales/en-US.json | 9 +- .../models/bot/__tests__/botProject.test.ts | 2 +- 20 files changed, 337 insertions(+), 348 deletions(-) create mode 100644 Composer/packages/client/__tests__/pages/design/Design.test.tsx diff --git a/Composer/cypress/integration/Breadcrumb.spec.ts b/Composer/cypress/integration/Breadcrumb.spec.ts index 47016c8f3b..e0a0ff7eb4 100644 --- a/Composer/cypress/integration/Breadcrumb.spec.ts +++ b/Composer/cypress/integration/Breadcrumb.spec.ts @@ -35,12 +35,12 @@ context('breadcrumb', () => { hasBreadcrumbItems(cy, ['__TestTodoSample']); }); - it('can show event name in breadcrumb', () => { + it('can show dialog and trigger name in breadcrumb', () => { cy.findByTestId('ProjectTree').within(() => { cy.findByTestId('addtodo_Dialog started').click(); }); - hasBreadcrumbItems(cy, ['__TestTodoSample', 'Dialog started']); + hasBreadcrumbItems(cy, ['addtodo', 'Dialog started']); }); it('can show action name in breadcrumb', () => { diff --git a/Composer/packages/adaptive-flow/src/adaptive-flow-editor/hooks/useEditorEventApi.ts b/Composer/packages/adaptive-flow/src/adaptive-flow-editor/hooks/useEditorEventApi.ts index cb7575dbd9..71c6b4648d 100644 --- a/Composer/packages/adaptive-flow/src/adaptive-flow-editor/hooks/useEditorEventApi.ts +++ b/Composer/packages/adaptive-flow/src/adaptive-flow-editor/hooks/useEditorEventApi.ts @@ -46,7 +46,7 @@ export const useEditorEventApi = ( onFocusSteps, onFocusEvent, onCopy: onClipboardChange, - navTo: onOpen, + navTo, saveData: onChange, undo, redo, @@ -153,7 +153,7 @@ export const useEditorEventApi = ( break; case NodeEventTypes.OpenDialog: handler = ({ callee }) => { - onOpen(callee); + navTo(callee, '"beginDialog"'); announce(ScreenReaderMessage.DialogOpened); }; break; diff --git a/Composer/packages/client/__tests__/pages/design/Design.test.tsx b/Composer/packages/client/__tests__/pages/design/Design.test.tsx new file mode 100644 index 0000000000..a26cbeacc0 --- /dev/null +++ b/Composer/packages/client/__tests__/pages/design/Design.test.tsx @@ -0,0 +1,65 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +import React from 'react'; + +import { renderWithRecoil } from '../../testUtils'; +import { + botProjectIdsState, + currentProjectIdState, + dialogsSelectorFamily, + schemasState, + projectMetaDataState, + botProjectFileState, +} from '../../../src/recoilModel'; +import { undoFunctionState } from '../../../src/recoilModel/undo/history'; +import mockProjectResponse from '../../../src/recoilModel/dispatchers/__tests__/mocks/mockProjectResponse.json'; +import DesignPage from '../../../src/pages/design/DesignPage'; +import { SAMPLE_DIALOG, SAMPLE_DIALOG_2 } from '../../mocks/sampleDialog'; + +const projectId = '12345.6789'; +const skillId = '56789.1234'; +const dialogId = SAMPLE_DIALOG.id; + +const initRecoilState = ({ set }) => { + set(currentProjectIdState, projectId); + set(botProjectIdsState, [projectId]); + set(dialogsSelectorFamily(projectId), [SAMPLE_DIALOG]); + set(schemasState(projectId), mockProjectResponse.schemas); + set(projectMetaDataState(projectId), { isRootBot: true }); + set(botProjectFileState(projectId), { foo: 'bar' }); + set(undoFunctionState(projectId), { canUndo: () => false, canRedo: () => false }); +}; + +const initRecoilStateMulti = ({ set }) => { + set(currentProjectIdState, projectId); + set(botProjectIdsState, [projectId, skillId]); + set(dialogsSelectorFamily(projectId), [SAMPLE_DIALOG]); + set(dialogsSelectorFamily(skillId), [SAMPLE_DIALOG, SAMPLE_DIALOG_2]); + set(schemasState(projectId), mockProjectResponse.schemas); + set(schemasState(skillId), mockProjectResponse.schemas); + set(projectMetaDataState(projectId), { isRootBot: true }); + set(botProjectFileState(projectId), { foo: 'bar' }); + set(undoFunctionState(projectId), { canUndo: () => false, canRedo: () => false }); + set(undoFunctionState(skillId), { canUndo: () => false, canRedo: () => false }); +}; + +describe('publish page', () => { + it('should render the design page (no skill)', () => { + const { getAllByText, getByText } = renderWithRecoil( + , + initRecoilState + ); + getAllByText(SAMPLE_DIALOG.displayName); + getByText('Start Bot'); + }); + + it('should render the design page (with skill)', () => { + const { getAllByText, getByText } = renderWithRecoil( + , + initRecoilStateMulti + ); + getAllByText(SAMPLE_DIALOG.displayName); + getAllByText(SAMPLE_DIALOG_2.displayName); + getByText('Start Bot'); + }); +}); diff --git a/Composer/packages/client/__tests__/utils/navigation.test.ts b/Composer/packages/client/__tests__/utils/navigation.test.ts index 100e4c12d3..0d526988f7 100644 --- a/Composer/packages/client/__tests__/utils/navigation.test.ts +++ b/Composer/packages/client/__tests__/utils/navigation.test.ts @@ -3,15 +3,7 @@ import { PromptTab } from '@bfc/shared'; -import { - BreadcrumbUpdateType, - getUrlSearch, - checkUrl, - getFocusPath, - clearBreadcrumb, - updateBreadcrumb, - convertPathToUrl, -} from './../../src/utils/navigation'; +import { getUrlSearch, checkUrl, getFocusPath, convertPathToUrl } from './../../src/utils/navigation'; const projectId = '123a-sdf123'; const skillId = '98765.4321'; @@ -27,46 +19,6 @@ describe('getFocusPath', () => { }); }); -describe('Breadcrumb Util', () => { - it('return focus path', () => { - const breadcrumb = [ - { dialogId: `1`, selected: `1`, focused: `1` }, - { dialogId: `2`, selected: `2`, focused: `2` }, - { dialogId: `3`, selected: `3`, focused: `3` }, - ]; - const result1 = clearBreadcrumb(breadcrumb); - expect(result1).toEqual([]); - const result2 = clearBreadcrumb(breadcrumb, 0); - expect(result2).toEqual([]); - const result3 = clearBreadcrumb(breadcrumb, 1); - expect(result3.length).toEqual(1); - expect(result3[0].dialogId).toEqual('1'); - const result4 = clearBreadcrumb(breadcrumb, 4); - expect(result4.length).toEqual(3); - }); - - it('update breadcrumb', () => { - const result1 = updateBreadcrumb([], BreadcrumbUpdateType.Selected); - expect(result1).toEqual([]); - let breadcrumb = [ - { dialogId: `1`, selected: `1`, focused: `1` }, - { dialogId: `2`, selected: `2`, focused: `2` }, - { dialogId: `3`, selected: `3`, focused: `3` }, - ]; - const result2 = updateBreadcrumb(breadcrumb, BreadcrumbUpdateType.Selected); - expect(result2.length).toEqual(1); - expect(result2[0].dialogId).toEqual('1'); - breadcrumb = [ - { dialogId: `1`, selected: `1`, focused: `` }, - { dialogId: `2`, selected: `2`, focused: `` }, - { dialogId: `3`, selected: `3`, focused: `3` }, - ]; - const result3 = updateBreadcrumb(breadcrumb, BreadcrumbUpdateType.Focused); - expect(result3.length).toEqual(2); - expect(result3[1].dialogId).toEqual('2'); - }); -}); - describe('composer url util', () => { it('create url', () => { const result1 = getUrlSearch('triggers[0]', 'triggers[0].actions[0]'); diff --git a/Composer/packages/client/src/components/ProjectTree/ProjectTree.tsx b/Composer/packages/client/src/components/ProjectTree/ProjectTree.tsx index 38c30393ba..49ffc6cfb4 100644 --- a/Composer/packages/client/src/components/ProjectTree/ProjectTree.tsx +++ b/Composer/packages/client/src/components/ProjectTree/ProjectTree.tsx @@ -95,6 +95,7 @@ export type TreeLink = { skillId?: string; dialogId?: string; trigger?: number; + parentLink?: TreeLink; }; export type TreeMenuItem = { @@ -274,7 +275,7 @@ export const ProjectTree: React.FC = ({ .map((diag) => diag.message) .join(','); - const link: TreeLink = { + const dialogLink: TreeLink = { dialogId: dialog.id, displayName: dialog.displayName, isRoot: dialog.isRoot, @@ -287,53 +288,56 @@ export const ProjectTree: React.FC = ({ const isFormDialog = dialogIsFormDialog(dialog); const showEditSchema = formDialogSchemaExists(skillId, dialog); - return ( - - { - onDeleteDialog(link.dialogId ?? ''); + return { + summaryElement: ( + + { + onDeleteDialog(link.dialogId ?? ''); + }, }, - }, - ] - : []), - ...(showEditSchema - ? [ - { - label: formatMessage('Edit schema'), - icon: 'Edit', - onClick: (link) => - navigateToFormDialogSchema({ projectId: link.skillId, schemaId: link.dialogName }), - }, - ] - : []), - ]} - onSelect={handleOnSelect} - /> - - ); + ] + : []), + ...(showEditSchema + ? [ + { + label: formatMessage('Edit schema'), + icon: 'Edit', + onClick: (link) => + navigateToFormDialogSchema({ projectId: link.skillId, schemaId: link.dialogName }), + }, + ] + : []), + ]} + onSelect={handleOnSelect} + /> + + ), + dialogLink, + }; }; - const renderTrigger = (item: any, dialog: DialogInfo, projectId: string): React.ReactNode => { + const renderTrigger = (item: any, dialog: DialogInfo, projectId: string, dialogLink?: TreeLink): React.ReactNode => { const link: TreeLink = { projectId: rootProjectId, skillId: projectId === rootProjectId ? undefined : projectId, @@ -343,6 +347,7 @@ export const ProjectTree: React.FC = ({ warningContent: item.warningContent, errorContent: item.errorContent, isRoot: false, + parentLink: dialogLink, }; return ( @@ -377,7 +382,7 @@ export const ProjectTree: React.FC = ({ return scope.toLowerCase().includes(filter.toLowerCase()); }; - const renderTriggerList = (triggers: ITrigger[], dialog: DialogInfo, projectId: string) => { + const renderTriggerList = (triggers: ITrigger[], dialog: DialogInfo, projectId: string, dialogLink?: TreeLink) => { return triggers .filter((tr) => filterMatch(dialog.displayName) || filterMatch(getTriggerName(tr))) .map((tr) => { @@ -389,7 +394,8 @@ export const ProjectTree: React.FC = ({ return renderTrigger( { ...tr, index, displayName: getTriggerName(tr), warningContent, errorContent }, dialog, - projectId + projectId, + dialogLink ); }); }; @@ -451,10 +457,10 @@ export const ProjectTree: React.FC = ({ }); }; - const renderDialogTriggers = (dialog: DialogInfo, projectId: string, startDepth: number) => { + const renderDialogTriggers = (dialog: DialogInfo, projectId: string, startDepth: number, dialogLink?: TreeLink) => { return dialogIsFormDialog(dialog) ? renderDialogTriggersByProperty(dialog, projectId, startDepth) - : renderTriggerList(dialog.triggers, dialog, projectId); + : renderTriggerList(dialog.triggers, dialog, projectId, dialogLink); }; const createDetailsTree = (bot: BotInProject, startDepth: number) => { @@ -471,14 +477,15 @@ export const ProjectTree: React.FC = ({ if (showTriggers) { return filteredDialogs.map((dialog: DialogInfo) => { + const { summaryElement, dialogLink } = renderDialogHeader(projectId, dialog); return ( -
{renderDialogTriggers(dialog, projectId, startDepth + 1)}
+
{renderDialogTriggers(dialog, projectId, startDepth + 1, dialogLink)}
); }); diff --git a/Composer/packages/client/src/pages/design/DesignPage.tsx b/Composer/packages/client/src/pages/design/DesignPage.tsx index 4436850382..2753c8855c 100644 --- a/Composer/packages/client/src/pages/design/DesignPage.tsx +++ b/Composer/packages/client/src/pages/design/DesignPage.tsx @@ -18,20 +18,15 @@ import { LeftRightSplit } from '../../components/Split/LeftRightSplit'; import { LoadingSpinner } from '../../components/LoadingSpinner'; import { TestController } from '../../components/TestController/TestController'; import { DialogDeleting } from '../../constants'; -import { - createSelectedPath, - deleteTrigger, - getBreadcrumbLabel, - TriggerFormData, - getDialogData, -} from '../../utils/dialogUtil'; +import { createSelectedPath, deleteTrigger, TriggerFormData, getDialogData } from '../../utils/dialogUtil'; import { Conversation } from '../../components/Conversation'; import { dialogStyle } from '../../components/Modal/dialogStyle'; import { OpenConfirmModal } from '../../components/Modal/ConfirmDialog'; import { ProjectTree, TreeLink } from '../../components/ProjectTree/ProjectTree'; import { Toolbar, IToolbarItem } from '../../components/Toolbar'; -import { clearBreadcrumb, getFocusPath } from '../../utils/navigation'; +import { getFocusPath } from '../../utils/navigation'; import { navigateTo } from '../../utils/navigation'; +import { getFriendlyName } from '../../utils/dialogUtil'; import { useShell } from '../../shell'; import plugins, { mergePluginConfigs } from '../../plugins'; import { useElectronFeatures } from '../../hooks/useElectronFeatures'; @@ -42,7 +37,6 @@ import { schemasState, displaySkillManifestState, validateDialogsSelectorFamily, - breadcrumbState, focusPathState, showCreateDialogModalState, showAddSkillDialogModalState, @@ -69,6 +63,13 @@ import { import { VisualEditor } from './VisualEditor'; import { PropertyEditor } from './PropertyEditor'; +type BreadcrumbItem = { + key: string; + label: string; + link?: Partial; + onClick?: () => void; +}; + const CreateSkillModal = React.lazy(() => import('../../components/CreateSkillModal')); const CreateDialogModal = React.lazy(() => import('./createDialogModal')); const DisplayManifestModal = React.lazy(() => import('../../components/Modal/DisplayManifestModal')); @@ -85,10 +86,6 @@ function onRenderContent(subTitle, style) { ); } -function onRenderBreadcrumbItem(item, render) { - return {render(item)}; -} - function getAllRef(targetId, dialogs) { let refs: string[] = []; dialogs.forEach((dialog) => { @@ -109,6 +106,13 @@ const getTabFromFragment = () => { } }; +const parseTriggerId = (triggerId: string | undefined): number | undefined => { + if (triggerId == null) return undefined; + const indexString = triggerId.match(/\d+/)?.[0]; + if (indexString == null) return undefined; + return parseInt(indexString); +}; + const DesignPage: React.FC> = ( props ) => { @@ -119,7 +123,6 @@ const DesignPage: React.FC(dialogs[0]); + const [currentDialog, setCurrentDialog] = useState(dialogs[0] as DialogInfo); const [exportSkillModalVisible, setExportSkillModalVisible] = useState(false); const [warningIsVisible, setWarningIsVisible] = useState(true); + const [breadcrumbs, setBreadcrumbs] = useState>([]); + const shell = useShell('DesignPage', skillId ?? rootProjectId); const shellForFlowEditor = useShell('FlowEditor', skillId ?? rootProjectId); const shellForPropertyEditor = useShell('PropertyEditor', skillId ?? rootProjectId); @@ -168,7 +172,7 @@ const DesignPage: React.FC { - const currentDialog = dialogs.find(({ id }) => id === dialogId); + const currentDialog = dialogs.find(({ id }) => id === dialogId) as DialogInfo | undefined; if (currentDialog) { setCurrentDialog(currentDialog); } @@ -205,10 +209,49 @@ const DesignPage: React.FC = []; + + breadcrumbArray.push({ + key: 'dialog-' + props.dialogId, + label: dialogMap[props.dialogId]?.$designer?.name ?? dialogMap[props.dialogId]?.$designer?.$designer?.name, + link: { + projectId: props.projectId, + dialogId: props.dialogId, + }, + onClick: () => navTo(projectId, dialogId), + }); + if (triggerIndex != null && trigger != null) { + breadcrumbArray.push({ + key: 'trigger-' + triggerIndex, + label: trigger.$designer.name || getFriendlyName(trigger), + link: { + projectId: props.projectId, + dialogId: props.dialogId, + trigger: triggerIndex, + }, + onClick: () => navTo(projectId, dialogId, `${triggerIndex}`), + }); + } + + // getDialogData returns whatever's at the end of the path, which could be a trigger or an action + const possibleAction = getDialogData(dialogMap, dialogId, focusPath); - if (typeof data === 'undefined') { + if (params.get('focused') != null) { + // we've linked to an action, so put that in too + breadcrumbArray.push({ + key: 'action-' + focusPath, + label: getActionName(possibleAction), + }); + } + + if (typeof possibleAction === 'undefined') { const { id: foundId } = dialogs.find(({ id }) => id === dialogId) || dialogs.find(({ isRoot }) => isRoot) || {}; /** * It's improper to fallback to `dialogId` directly: @@ -226,9 +269,9 @@ const DesignPage: React.FC = []; + if (dialogId != null) { + breadcrumbArray.push({ + key: 'dialog-' + parentLink?.dialogId, + label: parentLink?.displayName ?? link.displayName, + link: { projectId, skillId, dialogId }, + onClick: () => navTo(skillId ?? projectId, dialogId), + }); + } + if (trigger != null) { + breadcrumbArray.push({ + key: 'trigger-' + parentLink?.trigger, + label: link.displayName, + link: { projectId, skillId, dialogId, trigger }, + onClick: () => selectTo(skillId ?? null, dialogId ?? null, `triggers[${trigger}]`), + }); + } + + setBreadcrumbs(breadcrumbArray); + + if (trigger != null) { + selectTo(skillId ?? null, dialogId ?? null, `triggers[${trigger}]`); + } else if (dialogId != null) { + navTo(skillId ?? projectId, dialogId); } else { // with no dialog or ID, we must be looking at a bot link - navTo(link.skillId ?? link.projectId, null, []); + navTo(skillId ?? projectId, null); } } const onCreateDialogComplete = (dialogId) => { if (dialogId) { - navTo(projectId, dialogId, [{ dialogId, selected: '', focused: '' }]); + navTo(projectId, dialogId); + } + }; + + const pluginConfig: PluginConfig = useMemo(() => { + const sdkUISchema = schemas?.ui?.content ?? {}; + const userUISchema = schemas?.uiOverrides?.content ?? {}; + return mergePluginConfigs({ uiSchema: sdkUISchema }, plugins, { uiSchema: userUISchema }); + }, [schemas?.ui?.content, schemas?.uiOverrides?.content]); + + const getActionName = (action) => { + const nameFromAction = action?.$designer?.name as string | undefined; + let detectedActionName: string; + + if (typeof nameFromAction === 'string') { + detectedActionName = nameFromAction; + } else { + const kind: string = action?.$kind as string; + const actionNameFromSchema = pluginConfig?.uiSchema?.[kind]?.form?.label as string | (() => string) | undefined; + if (typeof actionNameFromSchema === 'string') { + detectedActionName = actionNameFromSchema; + } else if (typeof actionNameFromSchema === 'function') { + detectedActionName = actionNameFromSchema(); + } else { + detectedActionName = formatMessage('Unknown'); + } } + return detectedActionName; }; const { actionSelected, showDisableBtn, showEnableBtn } = useMemo(() => { @@ -285,6 +373,14 @@ const DesignPage: React.FC get(currentDialog?.content, id)); const showDisableBtn = selectedActions.some((x) => get(x, 'disabled') !== true); const showEnableBtn = selectedActions.some((x) => get(x, 'disabled') === true); + + if (selectedActions.length === 1 && selectedActions[0] != null) { + const action = selectedActions[0] as any; + const actionName = getActionName(action); + + setBreadcrumbs((prev) => [...prev.slice(0, 2), { key: 'action-' + actionName, label: actionName }]); + } + return { actionSelected, showDisableBtn, showEnableBtn }; }, [visualEditorSelection, currentDialog?.content]); @@ -454,55 +550,37 @@ const DesignPage: React.FC IBreadcrumbItem = (breadcrumb: BreadcrumbItem) => { + return { + key: breadcrumb.key, + text: breadcrumb.label, + onClick: () => breadcrumb.onClick?.(), + }; + }; - const breadcrumbItems = useMemo(() => { - const items = - dialogs.length > 0 - ? breadcrumb.reduce((result, item, index) => { - const { dialogId, selected, focused } = item; - const text = getBreadcrumbLabel(dialogs, dialogId, selected, focused); - if (text) { - result.push({ - // @ts-ignore - index, - isRoot: !selected && !focused, - text, - ...item, - onClick: handleBreadcrumbItemClick, - }); - } - return result; - }, [] as IBreadcrumbItem[]) - : []; - return ( -
- undefined} - onRenderItem={onRenderBreadcrumbItem} - /> -
- { - setDialogJsonVisibility((current) => !current); - }} - > - {dialogJsonVisible ? formatMessage('Hide code') : formatMessage('Show code')} - -
+ const items = breadcrumbs.map(createBreadcrumbItem); + + const breadcrumbItems = ( +
+ undefined} + /> +
+ { + setDialogJsonVisibility((current) => !current); + }} + > + {dialogJsonVisible ? formatMessage('Hide code') : formatMessage('Show code')} +
- ); - }, [dialogs, breadcrumb, dialogJsonVisible]); +
+ ); async function handleCreateDialogSubmit(dialogName, dialogData) { await createDialog({ id: dialogName, content: dialogData, projectId }); @@ -550,7 +628,7 @@ const DesignPage: React.FC { - const sdkUISchema = schemas?.ui?.content ?? {}; - const userUISchema = schemas?.uiOverrides?.content ?? {}; - return mergePluginConfigs({ uiSchema: sdkUISchema }, plugins, { uiSchema: userUISchema }); - }, [schemas?.ui?.content, schemas?.uiOverrides?.content]); - if (!dialogId) { return ; } @@ -587,13 +659,6 @@ const DesignPage: React.FC t.id === selected); const withWarning = triggerNotSupported(currentDialog, selectedTrigger); - const parseTriggerId = (triggerId: string | undefined): number | undefined => { - if (triggerId == null) return undefined; - const indexString = triggerId.match(/\d+/)?.[0]; - if (indexString == null) return undefined; - return parseInt(indexString); - }; - return (
diff --git a/Composer/packages/client/src/pages/design/PropertyEditor.tsx b/Composer/packages/client/src/pages/design/PropertyEditor.tsx index 73c40582d1..b3c489d3da 100644 --- a/Composer/packages/client/src/pages/design/PropertyEditor.tsx +++ b/Composer/packages/client/src/pages/design/PropertyEditor.tsx @@ -104,7 +104,7 @@ const PropertyEditor: React.FC = () => { if (!isEqual(dialogData, localData)) { shellApi.saveData(localData, focusedSteps[0]); } else { - shellApi.commitChanges(); + shellApi.commitChanges?.(); } }, 300); diff --git a/Composer/packages/client/src/recoilModel/atoms/botState.ts b/Composer/packages/client/src/recoilModel/atoms/botState.ts index 09772c3cc2..fc6c316599 100644 --- a/Composer/packages/client/src/recoilModel/atoms/botState.ts +++ b/Composer/packages/client/src/recoilModel/atoms/botState.ts @@ -24,7 +24,7 @@ import { BotLoadError, DesignPageLocation } from '../../recoilModel/types'; import FilePersistence from '../persistence/FilePersistence'; import { BotStatus } from './../../constants'; -import { BreadcrumbItem, PublishType } from './../../recoilModel/types'; +import { PublishType } from './../../recoilModel/types'; const getFullyQualifiedKey = (value: string) => { return `Bot_${value}_State`; @@ -180,13 +180,6 @@ export const skillManifestsState = atomFamily({ }, }); -export const breadcrumbState = atomFamily({ - key: getFullyQualifiedKey('breadcrumb'), - default: (id) => { - return []; - }, -}); - export const showCreateDialogModalState = atomFamily({ key: getFullyQualifiedKey('showCreateDialogModal'), default: (id) => { diff --git a/Composer/packages/client/src/recoilModel/dispatchers/__tests__/navigation.test.tsx b/Composer/packages/client/src/recoilModel/dispatchers/__tests__/navigation.test.tsx index 9661ff113f..1ae9e0ca06 100644 --- a/Composer/packages/client/src/recoilModel/dispatchers/__tests__/navigation.test.tsx +++ b/Composer/packages/client/src/recoilModel/dispatchers/__tests__/navigation.test.tsx @@ -7,20 +7,12 @@ import { SDKKinds } from '@bfc/shared'; import { navigationDispatcher } from '../navigation'; import { renderRecoilHook } from '../../../../__tests__/testUtils'; -import { focusPathState, breadcrumbState, designPageLocationState } from '../../atoms/botState'; +import { focusPathState, designPageLocationState } from '../../atoms/botState'; import { dialogsSelectorFamily } from '../../selectors'; import { dispatcherState } from '../../../recoilModel/DispatcherWrapper'; import { Dispatcher } from '../../../recoilModel/dispatchers'; -import { - convertPathToUrl, - navigateTo, - checkUrl, - updateBreadcrumb, - getUrlSearch, - BreadcrumbUpdateType, -} from '../../../utils/navigation'; +import { convertPathToUrl, navigateTo, checkUrl, getUrlSearch } from '../../../utils/navigation'; import { createSelectedPath, getSelected } from '../../../utils/dialogUtil'; -import { BreadcrumbItem } from '../../../recoilModel/types'; import { currentProjectIdState, botProjectIdsState, botProjectFileState, projectMetaDataState } from '../../atoms'; jest.mock('../../../utils/navigation'); @@ -29,7 +21,6 @@ jest.mock('../../../utils/dialogUtil'); const mockCheckUrl = checkUrl as jest.Mock; const mockNavigateTo = navigateTo as jest.Mock; const mockGetSelected = getSelected as jest.Mock; -const mockUpdateBreadcrumb = updateBreadcrumb as jest.Mock; const mockGetUrlSearch = getUrlSearch as jest.Mock; const mockConvertPathToUrl = convertPathToUrl as jest.Mock; const mockCreateSelectedPath = createSelectedPath as jest.Mock; @@ -37,8 +28,8 @@ const mockCreateSelectedPath = createSelectedPath as jest.Mock; const projectId = '12345.678'; const skillId = '98765.4321'; -function expectNavTo(location: string, state: {} | null = null) { - expect(mockNavigateTo).toHaveBeenLastCalledWith(location, state == null ? expect.anything() : state); +function expectNavTo(location: string) { + expect(mockNavigateTo).toHaveBeenLastCalledWith(location); } describe('navigation dispatcher', () => { @@ -46,7 +37,6 @@ describe('navigation dispatcher', () => { beforeEach(() => { mockCheckUrl.mockClear(); mockNavigateTo.mockClear(); - mockUpdateBreadcrumb.mockReturnValue([]); mockConvertPathToUrl.mockClear(); mockCreateSelectedPath.mockClear(); @@ -54,7 +44,6 @@ describe('navigation dispatcher', () => { const useRecoilTestHook = () => { const focusPath = useRecoilValue(focusPathState(projectId)); - const breadcrumb = useRecoilValue(breadcrumbState(projectId)); const designPageLocation = useRecoilValue(designPageLocationState(projectId)); const dialogs = useRecoilValue(dialogsSelectorFamily(projectId)); const currentDispatcher = useRecoilValue(dispatcherState); @@ -62,7 +51,6 @@ describe('navigation dispatcher', () => { return { dialogs, focusPath, - breadcrumb, designPageLocation, projectId, currentDispatcher, @@ -72,7 +60,6 @@ describe('navigation dispatcher', () => { const { result } = renderRecoilHook(useRecoilTestHook, { states: [ { recoilState: focusPathState(projectId), initialValue: 'path' }, - { recoilState: breadcrumbState(projectId), initialValue: [{ dialogId: '100', selected: 'a', focused: 'b' }] }, { recoilState: designPageLocationState(projectId), initialValue: { @@ -124,17 +111,11 @@ describe('navigation dispatcher', () => { await act(async () => { await dispatcher.setDesignPageLocation(projectId, { dialogId: 'dialogId', - breadcrumb: [], promptTab: undefined, }); }); expect(renderedComponent.current.focusPath).toEqual('dialogId#'); - expect(renderedComponent.current.breadcrumb).toHaveLength(1); - expect(renderedComponent.current.breadcrumb[0]).toEqual({ - dialogId: 'dialogId', - focused: '', - selected: '', - }); + expect(renderedComponent.current.designPageLocation).toEqual({ dialogId: 'dialogId', promptTab: undefined, @@ -147,18 +128,12 @@ describe('navigation dispatcher', () => { await act(async () => { await dispatcher.setDesignPageLocation(projectId, { dialogId: 'dialogId', - breadcrumb: [], selected: 'select', promptTab: undefined, }); }); expect(renderedComponent.current.focusPath).toEqual('dialogId#.select'); - expect(renderedComponent.current.breadcrumb).toHaveLength(1); - expect(renderedComponent.current.breadcrumb[0]).toEqual({ - dialogId: 'dialogId', - focused: '', - selected: 'select', - }); + expect(renderedComponent.current.designPageLocation).toEqual({ dialogId: 'dialogId', promptTab: undefined, @@ -171,19 +146,13 @@ describe('navigation dispatcher', () => { await act(async () => { await dispatcher.setDesignPageLocation(projectId, { dialogId: 'dialogId', - breadcrumb: [], focused: 'focus', selected: 'select', promptTab: undefined, }); }); expect(renderedComponent.current.focusPath).toEqual('dialogId#.focus'); - expect(renderedComponent.current.breadcrumb).toHaveLength(1); - expect(renderedComponent.current.breadcrumb[0]).toEqual({ - dialogId: 'dialogId', - focused: 'focus', - selected: 'select', - }); + expect(renderedComponent.current.designPageLocation).toEqual({ dialogId: 'dialogId', promptTab: undefined, @@ -197,7 +166,7 @@ describe('navigation dispatcher', () => { it('navigates to a destination', async () => { mockConvertPathToUrl.mockReturnValue(`/bot/${projectId}/dialogs/dialogId`); await act(async () => { - await dispatcher.navTo(projectId, 'dialogId', []); + await dispatcher.navTo(projectId, 'dialogId'); }); expectNavTo(`/bot/${projectId}/dialogs/dialogId`); expect(mockConvertPathToUrl).toBeCalledWith(projectId, projectId, 'dialogId'); @@ -206,7 +175,7 @@ describe('navigation dispatcher', () => { it("doesn't navigate to a destination where we already are", async () => { mockCheckUrl.mockReturnValue(true); await act(async () => { - await dispatcher.navTo(projectId, 'dialogId', []); + await dispatcher.navTo(projectId, 'dialogId'); }); expect(mockNavigateTo).not.toBeCalled(); }); @@ -261,8 +230,6 @@ describe('navigation dispatcher', () => { await dispatcher.focusTo(projectId, null, 'focus', ''); }); expectNavTo(`/bot/${projectId}/dialogs/dialogId?selected=select&focused=focus`); - expect(mockUpdateBreadcrumb).toHaveBeenCalledWith(expect.anything(), BreadcrumbUpdateType.Selected); - expect(mockUpdateBreadcrumb).toHaveBeenCalledWith(expect.anything(), BreadcrumbUpdateType.Focused); }); it('goes to a focused page with skill', async () => { @@ -271,8 +238,6 @@ describe('navigation dispatcher', () => { await dispatcher.focusTo(projectId, skillId, 'focus', ''); }); expectNavTo(`/bot/${projectId}/skill/${skillId}/dialogs/dialogInSkillId?selected=select&focused=focus`); - expect(mockUpdateBreadcrumb).toHaveBeenCalledWith(expect.anything(), BreadcrumbUpdateType.Selected); - expect(mockUpdateBreadcrumb).toHaveBeenCalledWith(expect.anything(), BreadcrumbUpdateType.Focused); }); it('goes to a focused page with fragment', async () => { @@ -281,8 +246,6 @@ describe('navigation dispatcher', () => { await dispatcher.focusTo(projectId, null, 'focus', 'fragment'); }); expectNavTo(`/bot/${projectId}/dialogs/dialogId?selected=select&focused=focus#fragment`); - expect(mockUpdateBreadcrumb).toHaveBeenCalledWith(expect.anything(), BreadcrumbUpdateType.Selected); - expect(mockUpdateBreadcrumb).toHaveBeenCalledWith(expect.anything(), BreadcrumbUpdateType.Focused); }); it('goes to a focused page with skill and fragment', async () => { @@ -291,8 +254,6 @@ describe('navigation dispatcher', () => { await dispatcher.focusTo(projectId, skillId, 'focus', 'fragment'); }); expectNavTo(`/bot/${projectId}/skill/${skillId}/dialogs/dialogInSkillId?selected=select&focused=focus#fragment`); - expect(mockUpdateBreadcrumb).toHaveBeenCalledWith(expect.anything(), BreadcrumbUpdateType.Selected); - expect(mockUpdateBreadcrumb).toHaveBeenCalledWith(expect.anything(), BreadcrumbUpdateType.Focused); }); it('stays on the same page but updates breadcrumbs with a checked URL', async () => { @@ -302,8 +263,6 @@ describe('navigation dispatcher', () => { await dispatcher.focusTo(projectId, null, 'focus', 'fragment'); }); expect(mockNavigateTo).not.toBeCalled(); - expect(mockUpdateBreadcrumb).toHaveBeenCalledWith(expect.anything(), BreadcrumbUpdateType.Selected); - expect(mockUpdateBreadcrumb).toHaveBeenCalledWith(expect.anything(), BreadcrumbUpdateType.Focused); }); }); diff --git a/Composer/packages/client/src/recoilModel/dispatchers/navigation.ts b/Composer/packages/client/src/recoilModel/dispatchers/navigation.ts index e18df80e4d..c4440aa69b 100644 --- a/Composer/packages/client/src/recoilModel/dispatchers/navigation.ts +++ b/Composer/packages/client/src/recoilModel/dispatchers/navigation.ts @@ -11,22 +11,14 @@ import { encodeArrayPathToDesignerPath } from '../../utils/convertUtils/designer import { dialogsSelectorFamily, rootBotProjectIdSelector } from '../selectors'; import { getSelected } from './../../utils/dialogUtil'; -import { BreadcrumbItem } from './../../recoilModel/types'; -import { breadcrumbState, designPageLocationState, focusPathState } from './../atoms/botState'; -import { - BreadcrumbUpdateType, - checkUrl, - convertPathToUrl, - getUrlSearch, - navigateTo, - updateBreadcrumb, -} from './../../utils/navigation'; +import { designPageLocationState, focusPathState } from './../atoms/botState'; +import { checkUrl, convertPathToUrl, getUrlSearch, navigateTo } from './../../utils/navigation'; export const navigationDispatcher = () => { const setDesignPageLocation = useRecoilCallback( ({ set }: CallbackInterface) => async ( projectId: string, - { dialogId = '', selected = '', focused = '', breadcrumb = [], promptTab } + { dialogId = '', selected = '', focused = '', promptTab } ) => { let focusPath = dialogId + '#'; if (focused) { @@ -36,8 +28,6 @@ export const navigationDispatcher = () => { } set(currentProjectIdState, projectId); set(focusPathState(projectId), focusPath); - //add current path to the breadcrumb - set(breadcrumbState(projectId), [...breadcrumb, { dialogId, selected, focused }]); set(designPageLocationState(projectId), { dialogId, selected, @@ -51,7 +41,7 @@ export const navigationDispatcher = () => { ({ snapshot, set }: CallbackInterface) => async ( skillId: string | null, dialogId: string | null, - breadcrumb: BreadcrumbItem[] = [] + trigger?: string ) => { const rootBotProjectId = await snapshot.getPromise(rootBotProjectIdSelector); if (rootBotProjectId == null) return; @@ -61,10 +51,13 @@ export const navigationDispatcher = () => { const designPageLocation = await snapshot.getPromise(designPageLocationState(projectId)); set(currentProjectIdState, projectId); - const currentUri = convertPathToUrl(rootBotProjectId, projectId, dialogId); + const currentUri = + trigger == null + ? convertPathToUrl(rootBotProjectId, skillId, dialogId) + : convertPathToUrl(rootBotProjectId, skillId, dialogId, `selected=triggers[${trigger}]`); if (checkUrl(currentUri, rootBotProjectId, projectId, designPageLocation)) return; - navigateTo(currentUri, { state: { breadcrumb } }); + navigateTo(currentUri); } ); @@ -82,7 +75,6 @@ export const navigationDispatcher = () => { set(currentProjectIdState, projectId); const designPageLocation = await snapshot.getPromise(designPageLocationState(projectId)); - const breadcrumb = await snapshot.getPromise(breadcrumbState(projectId)); // target dialogId, projectId maybe empty string "" const dialogId = destinationDialogId ?? designPageLocation.dialogId ?? 'Main'; @@ -93,7 +85,7 @@ export const navigationDispatcher = () => { const currentUri = convertPathToUrl(rootBotProjectId, skillId, dialogId, encodedSelectPath); if (checkUrl(currentUri, rootBotProjectId, skillId, designPageLocation)) return; - navigateTo(currentUri, { state: { breadcrumb: updateBreadcrumb(breadcrumb, BreadcrumbUpdateType.Selected) } }); + navigateTo(currentUri); } ); @@ -106,12 +98,10 @@ export const navigationDispatcher = () => { ) => { set(currentProjectIdState, skillId ?? projectId); const designPageLocation = await snapshot.getPromise(designPageLocationState(skillId ?? projectId)); - const breadcrumb = await snapshot.getPromise(breadcrumbState(skillId ?? projectId)); - let updatedBreadcrumb = [...breadcrumb]; const { dialogId, selected } = designPageLocation; let currentUri = - skillId == null + skillId == null || skillId === projectId ? `/bot/${projectId}/dialogs/${dialogId}` : `/bot/${projectId}/skill/${skillId}/dialogs/${dialogId}`; @@ -121,22 +111,17 @@ export const navigationDispatcher = () => { const encodedFocusPath = encodeArrayPathToDesignerPath(currentDialog?.content, focusPath); const targetSelected = getSelected(encodedFocusPath); - if (targetSelected !== selected) { - updatedBreadcrumb = updateBreadcrumb(breadcrumb, BreadcrumbUpdateType.Selected); - updatedBreadcrumb.push({ dialogId, selected: targetSelected, focused: '' }); - } + currentUri = `${currentUri}?selected=${targetSelected}&focused=${encodedFocusPath}`; - updatedBreadcrumb = updateBreadcrumb(breadcrumb, BreadcrumbUpdateType.Focused); } else { currentUri = `${currentUri}?selected=${selected}`; - updatedBreadcrumb = updateBreadcrumb(breadcrumb, BreadcrumbUpdateType.Selected); } if (fragment && typeof fragment === 'string') { currentUri += `#${fragment}`; } if (checkUrl(currentUri, projectId, skillId, designPageLocation)) return; - navigateTo(currentUri, { state: { breadcrumb: updatedBreadcrumb } }); + navigateTo(currentUri); } ); @@ -146,8 +131,7 @@ export const navigationDispatcher = () => { skillId: string | null, dialogId: string, selectPath: string, - focusPath: string, - breadcrumb: BreadcrumbItem[] = [] + focusPath: string ) => { set(currentProjectIdState, projectId); @@ -159,14 +143,14 @@ export const navigationDispatcher = () => { const designPageLocation = await snapshot.getPromise(designPageLocationState(projectId)); if (search) { const currentUri = - skillId == null + skillId == null || skillId === projectId ? `/bot/${projectId}/dialogs/${dialogId}${search}` : `/bot/${projectId}/skill/${skillId}/dialogs/${dialogId}${search}`; if (checkUrl(currentUri, projectId, skillId, designPageLocation)) return; - navigateTo(currentUri, { state: { breadcrumb } }); + navigateTo(currentUri); } else { - navTo(skillId ?? projectId, dialogId, breadcrumb); + navTo(skillId ?? projectId, dialogId); } } ); diff --git a/Composer/packages/client/src/recoilModel/dispatchers/publisher.ts b/Composer/packages/client/src/recoilModel/dispatchers/publisher.ts index e4ec8f5a2e..96f342d7c7 100644 --- a/Composer/packages/client/src/recoilModel/dispatchers/publisher.ts +++ b/Composer/packages/client/src/recoilModel/dispatchers/publisher.ts @@ -68,6 +68,7 @@ export const publisherDispatcher = () => { }; const updatePublishStatus = ({ set }: CallbackInterface, projectId: string, target: any, data: any) => { + if (data == null) return; const { endpointURL, status, id } = data; // the action below only applies to when a bot is being started using the "start bot" button // a check should be added to this that ensures this ONLY applies to the "default" profile. @@ -173,9 +174,9 @@ export const publisherDispatcher = () => { (callbackHelpers: CallbackInterface) => async (projectId: string, target: any) => { try { const response = await httpClient.get(`/publish/${projectId}/status/${target.name}`); - updatePublishStatus(callbackHelpers, projectId, target, response.data); + updatePublishStatus(callbackHelpers, projectId, target, response?.data); } catch (err) { - updatePublishStatus(callbackHelpers, projectId, target, err.response.data); + updatePublishStatus(callbackHelpers, projectId, target, err.response?.data); } } ); diff --git a/Composer/packages/client/src/recoilModel/types.ts b/Composer/packages/client/src/recoilModel/types.ts index 453eca41b0..b22608fec3 100644 --- a/Composer/packages/client/src/recoilModel/types.ts +++ b/Composer/packages/client/src/recoilModel/types.ts @@ -76,13 +76,6 @@ export interface AppUpdateState { version?: string; } -export interface BreadcrumbItem { - skillId?: string; - dialogId: string; - selected: string; - focused: string; -} - export type dialogPayload = { id: string; content: any; @@ -94,7 +87,7 @@ export type DesignPageLocationPayload = { dialogId: string; selected: string; focused: string; - breadcrumb: BreadcrumbItem[]; + breadcrumb: string[]; promptTab?: string; }; diff --git a/Composer/packages/client/src/recoilModel/undo/history.ts b/Composer/packages/client/src/recoilModel/undo/history.ts index 1a431b8212..f34c92ae16 100644 --- a/Composer/packages/client/src/recoilModel/undo/history.ts +++ b/Composer/packages/client/src/recoilModel/undo/history.ts @@ -13,7 +13,6 @@ import isEmpty from 'lodash/isEmpty'; import { navigateTo, getUrlSearch } from '../../utils/navigation'; -import { breadcrumbState } from './../atoms/botState'; import { designPageLocationState } from './../atoms'; import { trackedAtoms, AtomAssetsMap } from './trackedAtoms'; import UndoHistory from './undoHistory'; @@ -55,7 +54,6 @@ const getAtomAssetsMap = (snap: Snapshot, projectId: string): AtomAssetsMap => { //should record the location state atomMap.set(designPageLocationState(projectId), snap.getLoadable(designPageLocationState(projectId)).contents); - atomMap.set(breadcrumbState(projectId), snap.getLoadable(breadcrumbState(projectId)).contents); return atomMap; }; @@ -75,15 +73,13 @@ const checkAtomsChanged = (current: AtomAssetsMap, previous: AtomAssetsMap, atom function navigate(next: AtomAssetsMap, projectId: string) { const location = next.get(designPageLocationState(projectId)); - const breadcrumb = [...next.get(breadcrumbState(projectId))]; if (location) { const { dialogId, selected, focused, promptTab } = location; let currentUri = `/bot/${projectId}/dialogs/${dialogId}${getUrlSearch(selected, focused)}`; if (promptTab) { currentUri += `#${promptTab}`; } - breadcrumb.pop(); - navigateTo(currentUri, { state: { breadcrumb } }); + navigateTo(currentUri); } } @@ -105,10 +101,8 @@ function mapTrackedAtomsOntoSnapshot( function setInitialLocation(snapshot: Snapshot, projectId: string, undoHistory: UndoHistory) { const location = snapshot.getLoadable(designPageLocationState(projectId)); - const breadcrumb = snapshot.getLoadable(breadcrumbState(projectId)); if (location.state === 'hasValue') { undoHistory.setInitialValue(designPageLocationState(projectId), location.contents); - undoHistory.setInitialValue(breadcrumbState(projectId), breadcrumb.contents); } } interface UndoRootProps { @@ -184,11 +178,11 @@ export const UndoRoot = React.memo((props: UndoRootProps) => { }); const canUndo = () => { - return history.canUndo(); + return history?.canUndo?.(); }; const canRedo = () => { - return history.canRedo(); + return history?.canRedo?.(); }; const commit = useRecoilCallback(({ snapshot }) => () => { diff --git a/Composer/packages/client/src/shell/useShell.ts b/Composer/packages/client/src/shell/useShell.ts index 2026a0414e..f7bab76ed6 100644 --- a/Composer/packages/client/src/shell/useShell.ts +++ b/Composer/packages/client/src/shell/useShell.ts @@ -16,7 +16,6 @@ import { clipboardActionsState, schemasState, validateDialogsSelectorFamily, - breadcrumbState, focusPathState, skillsState, localeState, @@ -66,7 +65,6 @@ export function useShell(source: EventSource, projectId: string): Shell { const schemas = useRecoilValue(schemasState(projectId)); const dialogs = useRecoilValue(validateDialogsSelectorFamily(projectId)); - const breadcrumb = useRecoilValue(breadcrumbState(projectId)); const focusPath = useRecoilValue(focusPathState(projectId)); const skills = useRecoilValue(skillsState(projectId)); const locale = useRecoilValue(localeState(projectId)); @@ -137,9 +135,9 @@ export function useShell(source: EventSource, projectId: string): Shell { updateDialog({ id, content: newDialog.content, projectId }); } - function navigationTo(path) { + function navigationTo(path, rest?) { if (rootBotProjectId == null) return; - navTo(projectId, path, breadcrumb); + navTo(projectId, path, rest); } function focusEvent(subPath) { diff --git a/Composer/packages/client/src/utils/convertUtils/designerPathEncoder.ts b/Composer/packages/client/src/utils/convertUtils/designerPathEncoder.ts index 657a050f33..402edc1874 100644 --- a/Composer/packages/client/src/utils/convertUtils/designerPathEncoder.ts +++ b/Composer/packages/client/src/utils/convertUtils/designerPathEncoder.ts @@ -101,7 +101,12 @@ export const decodeDesignerPathToArrayPath = (dialog, path: string): string => { let arrayIndex = -1; if (designerPathInfo) { const { designerId } = designerPathInfo; - arrayIndex = arrayData.findIndex((x) => get(x, '$designer.id') === designerId); + if (designerId === 'beginDialog') { + // special notation to route to this specific trigger + arrayIndex = arrayData.findIndex((x) => get(x, '$kind') === 'Microsoft.OnBeginDialog'); + } else { + arrayIndex = arrayData.findIndex((x) => get(x, '$designer.id') === designerId); + } } else if (arrayPathInfo) { arrayIndex = arrayPathInfo.index; } diff --git a/Composer/packages/client/src/utils/navigation.ts b/Composer/packages/client/src/utils/navigation.ts index dcc7b19fe4..7a8432d25e 100644 --- a/Composer/packages/client/src/utils/navigation.ts +++ b/Composer/packages/client/src/utils/navigation.ts @@ -1,20 +1,15 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import cloneDeep from 'lodash/cloneDeep'; import { navigate, NavigateOptions } from '@reach/router'; -import { BreadcrumbItem, DesignPageLocation } from '../recoilModel/types'; +import { DesignPageLocation } from '../recoilModel/types'; import { BASEPATH } from '../constants'; import { parsePathToFocused } from './convertUtils/parsePathToFocused'; import { parsePathToSelected } from './convertUtils/parsePathToSelected'; import { parseTypeToFragment } from './convertUtils/parseTypeToFragment'; import { resolveToBasePath } from './fileUtil'; -export const BreadcrumbUpdateType = { - Selected: 'selected', - Focused: 'focused', -}; export function getFocusPath(selected: string, focused: string): string { if (selected && focused) return focused; @@ -24,31 +19,6 @@ export function getFocusPath(selected: string, focused: string): string { return ''; } -export function clearBreadcrumb(breadcrumb: BreadcrumbItem[], fromIndex?: number): BreadcrumbItem[] { - let breadcrumbCopy = cloneDeep(breadcrumb); - if (fromIndex) { - breadcrumbCopy.splice(fromIndex, breadcrumbCopy.length - fromIndex); - } else { - breadcrumbCopy = []; - } - return breadcrumbCopy; -} - -export function updateBreadcrumb(breadcrumb: BreadcrumbItem[], type: string): BreadcrumbItem[] { - const breadcrumbCopy = cloneDeep(breadcrumb); - if (breadcrumbCopy.length === 0) { - return breadcrumbCopy; - } - - let lastIndex = breadcrumbCopy.length - 1; - while (lastIndex > 0 && breadcrumbCopy[lastIndex][type]) { - breadcrumbCopy.pop(); - lastIndex--; - } - - return breadcrumbCopy; -} - export function getUrlSearch(selected: string, focused: string): string { const search = new URLSearchParams(); if (selected) { @@ -83,7 +53,7 @@ export function checkUrl( } export interface NavigationState { - breadcrumb?: BreadcrumbItem[]; + breadcrumb?: string[]; qnaKbUrls?: string[]; } @@ -97,7 +67,7 @@ export function convertPathToUrl( //uri = id?selected=triggers[0]&focused=triggers[0].actions[0] let uri = `/bot/${projectId}`; - if (skillId != null) { + if (skillId != null && skillId !== projectId) { uri += `/skill/${skillId}`; } if (dialogId != null) { diff --git a/Composer/packages/lib/shared/__tests__/dialogUtils/validateDialogName.test.ts b/Composer/packages/lib/shared/__tests__/dialogUtils/validateDialogName.test.ts index 8edcbec0a2..0666c0dbc1 100644 --- a/Composer/packages/lib/shared/__tests__/dialogUtils/validateDialogName.test.ts +++ b/Composer/packages/lib/shared/__tests__/dialogUtils/validateDialogName.test.ts @@ -4,7 +4,7 @@ import { validateDialogName } from '../../src/dialogUtils/validateDialogName'; const error = new Error( - "Spaces and special characters are not allowed. Use letters, numbers, -, or _ and don't use number at the beginning." + 'Spaces and special characters are not allowed. Use letters, numbers, -, or _, and begin the name with a letter.' ); const emptyError = new Error('The file name can not be empty'); diff --git a/Composer/packages/lib/shared/src/dialogUtils/validateDialogName.ts b/Composer/packages/lib/shared/src/dialogUtils/validateDialogName.ts index cf6d2fad1c..6450599600 100644 --- a/Composer/packages/lib/shared/src/dialogUtils/validateDialogName.ts +++ b/Composer/packages/lib/shared/src/dialogUtils/validateDialogName.ts @@ -13,7 +13,7 @@ export const validateDialogName = (name: string) => { if (!nameRegex.test(name)) { throw new Error( formatMessage( - "Spaces and special characters are not allowed. Use letters, numbers, -, or _ and don't use number at the beginning." + 'Spaces and special characters are not allowed. Use letters, numbers, -, or _, and begin the name with a letter.' ) ); } diff --git a/Composer/packages/server/src/locales/en-US.json b/Composer/packages/server/src/locales/en-US.json index b02415fd27..9507708939 100644 --- a/Composer/packages/server/src/locales/en-US.json +++ b/Composer/packages/server/src/locales/en-US.json @@ -2330,12 +2330,12 @@ "spaces_and_special_characters_are_not_allowed_20d47684": { "message": "Spaces and special characters are not allowed." }, - "spaces_and_special_characters_are_not_allowed_use__2a61c454": { - "message": "Spaces and special characters are not allowed. Use letters, numbers, -, or _ and don''t use number at the beginning." - }, "spaces_and_special_characters_are_not_allowed_use__48acec3c": { "message": "Spaces and special characters are not allowed. Use letters, numbers, -, or _." }, + "spaces_and_special_characters_are_not_allowed_use__d24a8636": { + "message": "Spaces and special characters are not allowed. Use letters, numbers, -, or _, and begin the name with a letter." + }, "specify_a_name_and_description_for_your_new_dialog_86eb3130": { "message": "Specify a name and description for your new dialog." }, @@ -2630,6 +2630,9 @@ "uninstall_8730233": { "message": "Uninstall" }, + "unknown_47a3b725": { + "message": "Unknown" + }, "unknown_intent_44b962ba": { "message": "Unknown intent" }, diff --git a/Composer/packages/server/src/models/bot/__tests__/botProject.test.ts b/Composer/packages/server/src/models/bot/__tests__/botProject.test.ts index baeebe3c52..e7fcfa384a 100644 --- a/Composer/packages/server/src/models/bot/__tests__/botProject.test.ts +++ b/Composer/packages/server/src/models/bot/__tests__/botProject.test.ts @@ -356,7 +356,7 @@ describe('dialog schema operations', () => { describe('should validate the file name when create a new one', () => { const error = new Error( - "Spaces and special characters are not allowed. Use letters, numbers, -, or _ and don't use number at the beginning." + 'Spaces and special characters are not allowed. Use letters, numbers, -, or _, and begin the name with a letter.' ); const emptyError = new Error('The file name can not be empty'); From 197428bf82236cfaa306d9ef83435d84648df23b Mon Sep 17 00:00:00 2001 From: leilzh Date: Thu, 5 Nov 2020 18:02:58 +0800 Subject: [PATCH 2/7] fix: undo/redo --- .../client/src/pages/design/DesignPage.tsx | 12 ++--- .../src/pages/design/PropertyEditor.tsx | 5 +- .../client/src/recoilModel/undo/history.ts | 46 +++++++++++++------ .../src/recoilModel/undo/trackedAtoms.ts | 2 +- .../packages/ui-plugins/lg/src/LgField.tsx | 1 + 5 files changed, 43 insertions(+), 23 deletions(-) diff --git a/Composer/packages/client/src/pages/design/DesignPage.tsx b/Composer/packages/client/src/pages/design/DesignPage.tsx index 2753c8855c..7a0af7ea42 100644 --- a/Composer/packages/client/src/pages/design/DesignPage.tsx +++ b/Composer/packages/client/src/pages/design/DesignPage.tsx @@ -46,7 +46,7 @@ import { } from '../../recoilModel'; import { CreateQnAModal } from '../../components/QnA'; import { triggerNotSupported } from '../../utils/dialogValidator'; -import { undoFunctionState, undoVersionState } from '../../recoilModel/undo/history'; +import { undoFunctionState, undoStatusState, undoVersionState } from '../../recoilModel/undo/history'; import { decodeDesignerPathToArrayPath } from '../../utils/convertUtils/designerPathEncoder'; import { useTriggerApi } from '../../shell/triggerApi'; @@ -131,7 +131,8 @@ const DesignPage: React.FC { }, [currentDialog, focusedSteps[0]]); const [localData, setLocalData] = useState(dialogData as MicrosoftAdaptiveDialog); - + console.log(currentDialog); + console.log(focusedSteps[0]); const syncData = useRef( // eslint-disable-next-line @typescript-eslint/no-explicit-any debounce((shellData: any, localData: any) => { @@ -103,8 +104,6 @@ const PropertyEditor: React.FC = () => { const id = setTimeout(() => { if (!isEqual(dialogData, localData)) { shellApi.saveData(localData, focusedSteps[0]); - } else { - shellApi.commitChanges?.(); } }, 300); diff --git a/Composer/packages/client/src/recoilModel/undo/history.ts b/Composer/packages/client/src/recoilModel/undo/history.ts index f34c92ae16..6eaa44e612 100644 --- a/Composer/packages/client/src/recoilModel/undo/history.ts +++ b/Composer/packages/client/src/recoilModel/undo/history.ts @@ -12,6 +12,8 @@ import uniqueId from 'lodash/uniqueId'; import isEmpty from 'lodash/isEmpty'; import { navigateTo, getUrlSearch } from '../../utils/navigation'; +import { encodeArrayPathToDesignerPath } from '../../utils/convertUtils/designerPathEncoder'; +import { dialogsSelectorFamily } from '../selectors'; import { designPageLocationState } from './../atoms'; import { trackedAtoms, AtomAssetsMap } from './trackedAtoms'; @@ -20,8 +22,6 @@ import UndoHistory from './undoHistory'; type IUndoRedo = { undo: () => void; redo: () => void; - canUndo: () => boolean; - canRedo: () => boolean; commitChanges: () => void; clearUndo: () => void; }; @@ -32,6 +32,15 @@ export const undoFunctionState = atomFamily({ dangerouslyAllowMutability: true, }); +export const undoStatusState = atomFamily<{ canUndo: boolean; canRedo: boolean }, string>({ + key: 'undoStatus', + default: { + canRedo: false, + canUndo: false, + }, + dangerouslyAllowMutability: true, +}); + export const undoHistoryState = atomFamily({ key: 'undoHistory', default: {} as UndoHistory, @@ -75,7 +84,11 @@ function navigate(next: AtomAssetsMap, projectId: string) { const location = next.get(designPageLocationState(projectId)); if (location) { const { dialogId, selected, focused, promptTab } = location; - let currentUri = `/bot/${projectId}/dialogs/${dialogId}${getUrlSearch(selected, focused)}`; + const dialog = next.get(dialogsSelectorFamily(projectId)).find((dialog) => dialogId === dialog.id); + let currentUri = `/bot/${projectId}/dialogs/${dialogId}${getUrlSearch( + encodeArrayPathToDesignerPath(dialog.content, selected), + encodeArrayPathToDesignerPath(dialog.content, focused) + )}`; if (promptTab) { currentUri += `#${promptTab}`; } @@ -96,6 +109,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; } @@ -114,7 +132,7 @@ export const UndoRoot = React.memo((props: UndoRootProps) => { const undoHistory = useRecoilValue(undoHistoryState(projectId)); const history: UndoHistory = useRef(undoHistory).current; const [initialStateLoaded, setInitialStateLoaded] = useState(false); - + const setUndoStatus = useSetRecoilState(undoStatusState(projectId)); const setUndoFunction = useSetRecoilState(undoFunctionState(projectId)); const [, forceUpdate] = useState([]); const setVersion = useSetRecoilState(undoVersionState(projectId)); @@ -159,12 +177,20 @@ export const UndoRoot = React.memo((props: UndoRootProps) => { navigate(next, projectId); }; + const updateUndoResult = () => { + setUndoStatus({ + canUndo: history.canUndo(), + canRedo: history.canRedo(), + }); + }; + const undo = useRecoilCallback(({ snapshot, gotoSnapshot }: CallbackInterface) => () => { if (history.canUndo()) { const present = history.getPresentAssets(); const next = history.undo(); if (present) undoAssets(snapshot, present, next, gotoSnapshot, projectId); setVersion(uniqueId()); + updateUndoResult(); } }); @@ -174,17 +200,10 @@ 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(); @@ -192,6 +211,7 @@ export const UndoRoot = React.memo((props: UndoRootProps) => { if (previousAssets && checkAtomsChanged(currentAssets, previousAssets, trackedAtoms(projectId))) { history.add(getAtomAssetsMap(snapshot, projectId)); + updateUndoResult(); } }); @@ -208,7 +228,7 @@ export const UndoRoot = React.memo((props: UndoRootProps) => { }); useEffect(() => { - setUndoFunction({ undo, redo, canRedo, canUndo, commitChanges, clearUndo }); + setUndoFunction({ undo, redo, commitChanges, clearUndo }); }, []); return null; diff --git a/Composer/packages/client/src/recoilModel/undo/trackedAtoms.ts b/Composer/packages/client/src/recoilModel/undo/trackedAtoms.ts index 3ed1f116d0..ed5d403915 100644 --- a/Composer/packages/client/src/recoilModel/undo/trackedAtoms.ts +++ b/Composer/packages/client/src/recoilModel/undo/trackedAtoms.ts @@ -3,7 +3,7 @@ import { RecoilState } from 'recoil'; -import { luFilesState, lgFilesState } from '../atoms'; +import { luFilesState, lgFilesState, designPageLocationState } from '../atoms'; import { dialogsSelectorFamily } from '../selectors'; export type AtomAssetsMap = Map, any>; diff --git a/Composer/packages/ui-plugins/lg/src/LgField.tsx b/Composer/packages/ui-plugins/lg/src/LgField.tsx index 3c16dc0b21..683cc494a8 100644 --- a/Composer/packages/ui-plugins/lg/src/LgField.tsx +++ b/Composer/packages/ui-plugins/lg/src/LgField.tsx @@ -78,6 +78,7 @@ const LgField: React.FC> = (props) => { if (body) { updateLgTemplate(body); props.onChange(new LgTemplateRef(lgName).toString()); + shellApi.commitChanges(); } else { shellApi.removeLgTemplate(lgFileId, lgName); props.onChange(); From 2d2a4aadbd4a6d8f98c38d3c4076b68cef4cc29e Mon Sep 17 00:00:00 2001 From: leilzh Date: Thu, 5 Nov 2020 19:55:59 +0800 Subject: [PATCH 3/7] add location check and add fix unit tests --- .../src/pages/design/PropertyEditor.tsx | 2 - .../undo/__test__/history.test.tsx | 37 +++++++++------ .../client/src/recoilModel/undo/history.ts | 47 +++++++++++++++---- .../src/recoilModel/undo/trackedAtoms.ts | 2 +- 4 files changed, 62 insertions(+), 26 deletions(-) diff --git a/Composer/packages/client/src/pages/design/PropertyEditor.tsx b/Composer/packages/client/src/pages/design/PropertyEditor.tsx index 7a06d903e8..d523df8ca7 100644 --- a/Composer/packages/client/src/pages/design/PropertyEditor.tsx +++ b/Composer/packages/client/src/pages/design/PropertyEditor.tsx @@ -38,8 +38,6 @@ const PropertyEditor: React.FC = () => { }, [currentDialog, focusedSteps[0]]); const [localData, setLocalData] = useState(dialogData as MicrosoftAdaptiveDialog); - console.log(currentDialog); - console.log(focusedSteps[0]); const syncData = useRef( // eslint-disable-next-line @typescript-eslint/no-explicit-any debounce((shellData: any, localData: any) => { diff --git a/Composer/packages/client/src/recoilModel/undo/__test__/history.test.tsx b/Composer/packages/client/src/recoilModel/undo/__test__/history.test.tsx index 5d561b06bf..22613a926a 100644 --- a/Composer/packages/client/src/recoilModel/undo/__test__/history.test.tsx +++ b/Composer/packages/client/src/recoilModel/undo/__test__/history.test.tsx @@ -6,13 +6,14 @@ import { jsx } from '@emotion/core'; import { act } from 'react-test-renderer'; import { useRecoilValue, useSetRecoilState, useRecoilState } from 'recoil'; -import { UndoRoot, undoFunctionState, undoHistoryState } from '../history'; +import { UndoRoot, undoFunctionState, undoHistoryState, undoStatusState } from '../history'; import { lgFilesState, luFilesState, projectMetaDataState, currentProjectIdState, botProjectIdsState, + designPageLocationState, } from '../../atoms'; import { dialogsSelectorFamily } from '../../selectors'; import { renderRecoilHook } from '../../../../__tests__/testUtils/react-recoil-hooks-testing-library'; @@ -30,11 +31,12 @@ describe('', () => { 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 { canRedo, canUndo } = useRecoilValue(undoStatusState(projectId)); return { undo, redo, @@ -46,6 +48,7 @@ describe('', () => { setDialogs, dialogs, history, + setDesignPageLocation, }; }; @@ -60,18 +63,18 @@ describe('', () => { }, 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: undoStatusState(projectId), initialValue: { canUndo: false, canRedo: 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 +110,14 @@ describe('', () => { 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 +135,30 @@ describe('', () => { 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', () => { diff --git a/Composer/packages/client/src/recoilModel/undo/history.ts b/Composer/packages/client/src/recoilModel/undo/history.ts index 6eaa44e612..f189cce3e6 100644 --- a/Composer/packages/client/src/recoilModel/undo/history.ts +++ b/Composer/packages/client/src/recoilModel/undo/history.ts @@ -10,11 +10,14 @@ 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 { rootBotProjectIdSelector } from './../selectors/project'; import { designPageLocationState } from './../atoms'; import { trackedAtoms, AtomAssetsMap } from './trackedAtoms'; import UndoHistory from './undoHistory'; @@ -53,8 +56,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, any>(); + let atomMap = new Map, any>(); const atomsToBeTracked = trackedAtoms(projectId); atomsToBeTracked.forEach((atom) => { const loadable = snap.getLoadable(atom); @@ -63,6 +83,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; }; @@ -80,16 +101,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; - const dialog = next.get(dialogsSelectorFamily(projectId)).find((dialog) => dialogId === dialog.id); - let currentUri = `/bot/${projectId}/dialogs/${dialogId}${getUrlSearch( + 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) { + if (promptTab && focused) { currentUri += `#${promptTab}`; } navigateTo(currentUri); @@ -130,13 +157,15 @@ 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 setUndoStatus = useSetRecoilState(undoStatusState(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); @@ -174,7 +203,7 @@ 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 = () => { diff --git a/Composer/packages/client/src/recoilModel/undo/trackedAtoms.ts b/Composer/packages/client/src/recoilModel/undo/trackedAtoms.ts index ed5d403915..3ed1f116d0 100644 --- a/Composer/packages/client/src/recoilModel/undo/trackedAtoms.ts +++ b/Composer/packages/client/src/recoilModel/undo/trackedAtoms.ts @@ -3,7 +3,7 @@ import { RecoilState } from 'recoil'; -import { luFilesState, lgFilesState, designPageLocationState } from '../atoms'; +import { luFilesState, lgFilesState } from '../atoms'; import { dialogsSelectorFamily } from '../selectors'; export type AtomAssetsMap = Map, any>; From 4b43ad08604c50c80492d9c51227795fe069582a Mon Sep 17 00:00:00 2001 From: leilzh Date: Fri, 6 Nov 2020 08:41:13 +0800 Subject: [PATCH 4/7] Revert "Bot proj breadcrumbs (#4)" This reverts commit d558846afb7251a0288a3a47c022ec6620fc52c1. --- .../cypress/integration/Breadcrumb.spec.ts | 4 +- .../hooks/useEditorEventApi.ts | 4 +- .../__tests__/pages/design/Design.test.tsx | 65 ----- .../client/__tests__/utils/navigation.test.ts | 50 +++- .../components/ProjectTree/ProjectTree.tsx | 109 ++++---- .../client/src/pages/design/DesignPage.tsx | 243 +++++++----------- .../client/src/recoilModel/atoms/botState.ts | 9 +- .../dispatchers/__tests__/navigation.test.tsx | 59 ++++- .../src/recoilModel/dispatchers/navigation.ts | 50 ++-- .../src/recoilModel/dispatchers/publisher.ts | 5 +- .../packages/client/src/recoilModel/types.ts | 9 +- .../packages/client/src/shell/useShell.ts | 6 +- .../utils/convertUtils/designerPathEncoder.ts | 7 +- .../packages/client/src/utils/navigation.ts | 36 ++- .../dialogUtils/validateDialogName.test.ts | 2 +- .../src/dialogUtils/validateDialogName.ts | 2 +- .../packages/server/src/locales/en-US.json | 9 +- .../models/bot/__tests__/botProject.test.ts | 2 +- 18 files changed, 338 insertions(+), 333 deletions(-) delete mode 100644 Composer/packages/client/__tests__/pages/design/Design.test.tsx diff --git a/Composer/cypress/integration/Breadcrumb.spec.ts b/Composer/cypress/integration/Breadcrumb.spec.ts index e0a0ff7eb4..47016c8f3b 100644 --- a/Composer/cypress/integration/Breadcrumb.spec.ts +++ b/Composer/cypress/integration/Breadcrumb.spec.ts @@ -35,12 +35,12 @@ context('breadcrumb', () => { hasBreadcrumbItems(cy, ['__TestTodoSample']); }); - it('can show dialog and trigger name in breadcrumb', () => { + it('can show event name in breadcrumb', () => { cy.findByTestId('ProjectTree').within(() => { cy.findByTestId('addtodo_Dialog started').click(); }); - hasBreadcrumbItems(cy, ['addtodo', 'Dialog started']); + hasBreadcrumbItems(cy, ['__TestTodoSample', 'Dialog started']); }); it('can show action name in breadcrumb', () => { diff --git a/Composer/packages/adaptive-flow/src/adaptive-flow-editor/hooks/useEditorEventApi.ts b/Composer/packages/adaptive-flow/src/adaptive-flow-editor/hooks/useEditorEventApi.ts index 71c6b4648d..cb7575dbd9 100644 --- a/Composer/packages/adaptive-flow/src/adaptive-flow-editor/hooks/useEditorEventApi.ts +++ b/Composer/packages/adaptive-flow/src/adaptive-flow-editor/hooks/useEditorEventApi.ts @@ -46,7 +46,7 @@ export const useEditorEventApi = ( onFocusSteps, onFocusEvent, onCopy: onClipboardChange, - navTo, + navTo: onOpen, saveData: onChange, undo, redo, @@ -153,7 +153,7 @@ export const useEditorEventApi = ( break; case NodeEventTypes.OpenDialog: handler = ({ callee }) => { - navTo(callee, '"beginDialog"'); + onOpen(callee); announce(ScreenReaderMessage.DialogOpened); }; break; diff --git a/Composer/packages/client/__tests__/pages/design/Design.test.tsx b/Composer/packages/client/__tests__/pages/design/Design.test.tsx deleted file mode 100644 index a26cbeacc0..0000000000 --- a/Composer/packages/client/__tests__/pages/design/Design.test.tsx +++ /dev/null @@ -1,65 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. -import React from 'react'; - -import { renderWithRecoil } from '../../testUtils'; -import { - botProjectIdsState, - currentProjectIdState, - dialogsSelectorFamily, - schemasState, - projectMetaDataState, - botProjectFileState, -} from '../../../src/recoilModel'; -import { undoFunctionState } from '../../../src/recoilModel/undo/history'; -import mockProjectResponse from '../../../src/recoilModel/dispatchers/__tests__/mocks/mockProjectResponse.json'; -import DesignPage from '../../../src/pages/design/DesignPage'; -import { SAMPLE_DIALOG, SAMPLE_DIALOG_2 } from '../../mocks/sampleDialog'; - -const projectId = '12345.6789'; -const skillId = '56789.1234'; -const dialogId = SAMPLE_DIALOG.id; - -const initRecoilState = ({ set }) => { - set(currentProjectIdState, projectId); - set(botProjectIdsState, [projectId]); - set(dialogsSelectorFamily(projectId), [SAMPLE_DIALOG]); - set(schemasState(projectId), mockProjectResponse.schemas); - set(projectMetaDataState(projectId), { isRootBot: true }); - set(botProjectFileState(projectId), { foo: 'bar' }); - set(undoFunctionState(projectId), { canUndo: () => false, canRedo: () => false }); -}; - -const initRecoilStateMulti = ({ set }) => { - set(currentProjectIdState, projectId); - set(botProjectIdsState, [projectId, skillId]); - set(dialogsSelectorFamily(projectId), [SAMPLE_DIALOG]); - set(dialogsSelectorFamily(skillId), [SAMPLE_DIALOG, SAMPLE_DIALOG_2]); - set(schemasState(projectId), mockProjectResponse.schemas); - set(schemasState(skillId), mockProjectResponse.schemas); - set(projectMetaDataState(projectId), { isRootBot: true }); - set(botProjectFileState(projectId), { foo: 'bar' }); - set(undoFunctionState(projectId), { canUndo: () => false, canRedo: () => false }); - set(undoFunctionState(skillId), { canUndo: () => false, canRedo: () => false }); -}; - -describe('publish page', () => { - it('should render the design page (no skill)', () => { - const { getAllByText, getByText } = renderWithRecoil( - , - initRecoilState - ); - getAllByText(SAMPLE_DIALOG.displayName); - getByText('Start Bot'); - }); - - it('should render the design page (with skill)', () => { - const { getAllByText, getByText } = renderWithRecoil( - , - initRecoilStateMulti - ); - getAllByText(SAMPLE_DIALOG.displayName); - getAllByText(SAMPLE_DIALOG_2.displayName); - getByText('Start Bot'); - }); -}); diff --git a/Composer/packages/client/__tests__/utils/navigation.test.ts b/Composer/packages/client/__tests__/utils/navigation.test.ts index 0d526988f7..100e4c12d3 100644 --- a/Composer/packages/client/__tests__/utils/navigation.test.ts +++ b/Composer/packages/client/__tests__/utils/navigation.test.ts @@ -3,7 +3,15 @@ import { PromptTab } from '@bfc/shared'; -import { getUrlSearch, checkUrl, getFocusPath, convertPathToUrl } from './../../src/utils/navigation'; +import { + BreadcrumbUpdateType, + getUrlSearch, + checkUrl, + getFocusPath, + clearBreadcrumb, + updateBreadcrumb, + convertPathToUrl, +} from './../../src/utils/navigation'; const projectId = '123a-sdf123'; const skillId = '98765.4321'; @@ -19,6 +27,46 @@ describe('getFocusPath', () => { }); }); +describe('Breadcrumb Util', () => { + it('return focus path', () => { + const breadcrumb = [ + { dialogId: `1`, selected: `1`, focused: `1` }, + { dialogId: `2`, selected: `2`, focused: `2` }, + { dialogId: `3`, selected: `3`, focused: `3` }, + ]; + const result1 = clearBreadcrumb(breadcrumb); + expect(result1).toEqual([]); + const result2 = clearBreadcrumb(breadcrumb, 0); + expect(result2).toEqual([]); + const result3 = clearBreadcrumb(breadcrumb, 1); + expect(result3.length).toEqual(1); + expect(result3[0].dialogId).toEqual('1'); + const result4 = clearBreadcrumb(breadcrumb, 4); + expect(result4.length).toEqual(3); + }); + + it('update breadcrumb', () => { + const result1 = updateBreadcrumb([], BreadcrumbUpdateType.Selected); + expect(result1).toEqual([]); + let breadcrumb = [ + { dialogId: `1`, selected: `1`, focused: `1` }, + { dialogId: `2`, selected: `2`, focused: `2` }, + { dialogId: `3`, selected: `3`, focused: `3` }, + ]; + const result2 = updateBreadcrumb(breadcrumb, BreadcrumbUpdateType.Selected); + expect(result2.length).toEqual(1); + expect(result2[0].dialogId).toEqual('1'); + breadcrumb = [ + { dialogId: `1`, selected: `1`, focused: `` }, + { dialogId: `2`, selected: `2`, focused: `` }, + { dialogId: `3`, selected: `3`, focused: `3` }, + ]; + const result3 = updateBreadcrumb(breadcrumb, BreadcrumbUpdateType.Focused); + expect(result3.length).toEqual(2); + expect(result3[1].dialogId).toEqual('2'); + }); +}); + describe('composer url util', () => { it('create url', () => { const result1 = getUrlSearch('triggers[0]', 'triggers[0].actions[0]'); diff --git a/Composer/packages/client/src/components/ProjectTree/ProjectTree.tsx b/Composer/packages/client/src/components/ProjectTree/ProjectTree.tsx index 49ffc6cfb4..38c30393ba 100644 --- a/Composer/packages/client/src/components/ProjectTree/ProjectTree.tsx +++ b/Composer/packages/client/src/components/ProjectTree/ProjectTree.tsx @@ -95,7 +95,6 @@ export type TreeLink = { skillId?: string; dialogId?: string; trigger?: number; - parentLink?: TreeLink; }; export type TreeMenuItem = { @@ -275,7 +274,7 @@ export const ProjectTree: React.FC = ({ .map((diag) => diag.message) .join(','); - const dialogLink: TreeLink = { + const link: TreeLink = { dialogId: dialog.id, displayName: dialog.displayName, isRoot: dialog.isRoot, @@ -288,56 +287,53 @@ export const ProjectTree: React.FC = ({ const isFormDialog = dialogIsFormDialog(dialog); const showEditSchema = formDialogSchemaExists(skillId, dialog); - return { - summaryElement: ( - - { - onDeleteDialog(link.dialogId ?? ''); - }, - }, - ] - : []), - ...(showEditSchema - ? [ - { - label: formatMessage('Edit schema'), - icon: 'Edit', - onClick: (link) => - navigateToFormDialogSchema({ projectId: link.skillId, schemaId: link.dialogName }), + return ( + + { + onDeleteDialog(link.dialogId ?? ''); }, - ] - : []), - ]} - onSelect={handleOnSelect} - /> - - ), - dialogLink, - }; + }, + ] + : []), + ...(showEditSchema + ? [ + { + label: formatMessage('Edit schema'), + icon: 'Edit', + onClick: (link) => + navigateToFormDialogSchema({ projectId: link.skillId, schemaId: link.dialogName }), + }, + ] + : []), + ]} + onSelect={handleOnSelect} + /> + + ); }; - const renderTrigger = (item: any, dialog: DialogInfo, projectId: string, dialogLink?: TreeLink): React.ReactNode => { + const renderTrigger = (item: any, dialog: DialogInfo, projectId: string): React.ReactNode => { const link: TreeLink = { projectId: rootProjectId, skillId: projectId === rootProjectId ? undefined : projectId, @@ -347,7 +343,6 @@ export const ProjectTree: React.FC = ({ warningContent: item.warningContent, errorContent: item.errorContent, isRoot: false, - parentLink: dialogLink, }; return ( @@ -382,7 +377,7 @@ export const ProjectTree: React.FC = ({ return scope.toLowerCase().includes(filter.toLowerCase()); }; - const renderTriggerList = (triggers: ITrigger[], dialog: DialogInfo, projectId: string, dialogLink?: TreeLink) => { + const renderTriggerList = (triggers: ITrigger[], dialog: DialogInfo, projectId: string) => { return triggers .filter((tr) => filterMatch(dialog.displayName) || filterMatch(getTriggerName(tr))) .map((tr) => { @@ -394,8 +389,7 @@ export const ProjectTree: React.FC = ({ return renderTrigger( { ...tr, index, displayName: getTriggerName(tr), warningContent, errorContent }, dialog, - projectId, - dialogLink + projectId ); }); }; @@ -457,10 +451,10 @@ export const ProjectTree: React.FC = ({ }); }; - const renderDialogTriggers = (dialog: DialogInfo, projectId: string, startDepth: number, dialogLink?: TreeLink) => { + const renderDialogTriggers = (dialog: DialogInfo, projectId: string, startDepth: number) => { return dialogIsFormDialog(dialog) ? renderDialogTriggersByProperty(dialog, projectId, startDepth) - : renderTriggerList(dialog.triggers, dialog, projectId, dialogLink); + : renderTriggerList(dialog.triggers, dialog, projectId); }; const createDetailsTree = (bot: BotInProject, startDepth: number) => { @@ -477,15 +471,14 @@ export const ProjectTree: React.FC = ({ if (showTriggers) { return filteredDialogs.map((dialog: DialogInfo) => { - const { summaryElement, dialogLink } = renderDialogHeader(projectId, dialog); return ( -
{renderDialogTriggers(dialog, projectId, startDepth + 1, dialogLink)}
+
{renderDialogTriggers(dialog, projectId, startDepth + 1)}
); }); diff --git a/Composer/packages/client/src/pages/design/DesignPage.tsx b/Composer/packages/client/src/pages/design/DesignPage.tsx index 7a0af7ea42..a63017325d 100644 --- a/Composer/packages/client/src/pages/design/DesignPage.tsx +++ b/Composer/packages/client/src/pages/design/DesignPage.tsx @@ -18,15 +18,20 @@ import { LeftRightSplit } from '../../components/Split/LeftRightSplit'; import { LoadingSpinner } from '../../components/LoadingSpinner'; import { TestController } from '../../components/TestController/TestController'; import { DialogDeleting } from '../../constants'; -import { createSelectedPath, deleteTrigger, TriggerFormData, getDialogData } from '../../utils/dialogUtil'; +import { + createSelectedPath, + deleteTrigger, + getBreadcrumbLabel, + TriggerFormData, + getDialogData, +} from '../../utils/dialogUtil'; import { Conversation } from '../../components/Conversation'; import { dialogStyle } from '../../components/Modal/dialogStyle'; import { OpenConfirmModal } from '../../components/Modal/ConfirmDialog'; import { ProjectTree, TreeLink } from '../../components/ProjectTree/ProjectTree'; import { Toolbar, IToolbarItem } from '../../components/Toolbar'; -import { getFocusPath } from '../../utils/navigation'; +import { clearBreadcrumb, getFocusPath } from '../../utils/navigation'; import { navigateTo } from '../../utils/navigation'; -import { getFriendlyName } from '../../utils/dialogUtil'; import { useShell } from '../../shell'; import plugins, { mergePluginConfigs } from '../../plugins'; import { useElectronFeatures } from '../../hooks/useElectronFeatures'; @@ -37,6 +42,7 @@ import { schemasState, displaySkillManifestState, validateDialogsSelectorFamily, + breadcrumbState, focusPathState, showCreateDialogModalState, showAddSkillDialogModalState, @@ -63,13 +69,6 @@ import { import { VisualEditor } from './VisualEditor'; import { PropertyEditor } from './PropertyEditor'; -type BreadcrumbItem = { - key: string; - label: string; - link?: Partial; - onClick?: () => void; -}; - const CreateSkillModal = React.lazy(() => import('../../components/CreateSkillModal')); const CreateDialogModal = React.lazy(() => import('./createDialogModal')); const DisplayManifestModal = React.lazy(() => import('../../components/Modal/DisplayManifestModal')); @@ -86,6 +85,10 @@ function onRenderContent(subTitle, style) { ); } +function onRenderBreadcrumbItem(item, render) { + return {render(item)}; +} + function getAllRef(targetId, dialogs) { let refs: string[] = []; dialogs.forEach((dialog) => { @@ -106,13 +109,6 @@ const getTabFromFragment = () => { } }; -const parseTriggerId = (triggerId: string | undefined): number | undefined => { - if (triggerId == null) return undefined; - const indexString = triggerId.match(/\d+/)?.[0]; - if (indexString == null) return undefined; - return parseInt(indexString); -}; - const DesignPage: React.FC> = ( props ) => { @@ -123,6 +119,7 @@ const DesignPage: React.FC(dialogs[0] as DialogInfo); + const [currentDialog, setCurrentDialog] = useState(dialogs[0]); const [exportSkillModalVisible, setExportSkillModalVisible] = useState(false); const [warningIsVisible, setWarningIsVisible] = useState(true); - const [breadcrumbs, setBreadcrumbs] = useState>([]); - const shell = useShell('DesignPage', skillId ?? rootProjectId); const shellForFlowEditor = useShell('FlowEditor', skillId ?? rootProjectId); const shellForPropertyEditor = useShell('PropertyEditor', skillId ?? rootProjectId); @@ -173,7 +169,7 @@ const DesignPage: React.FC { - const currentDialog = dialogs.find(({ id }) => id === dialogId) as DialogInfo | undefined; + const currentDialog = dialogs.find(({ id }) => id === dialogId); if (currentDialog) { setCurrentDialog(currentDialog); } @@ -210,49 +206,10 @@ const DesignPage: React.FC = []; - - breadcrumbArray.push({ - key: 'dialog-' + props.dialogId, - label: dialogMap[props.dialogId]?.$designer?.name ?? dialogMap[props.dialogId]?.$designer?.$designer?.name, - link: { - projectId: props.projectId, - dialogId: props.dialogId, - }, - onClick: () => navTo(projectId, dialogId), - }); - if (triggerIndex != null && trigger != null) { - breadcrumbArray.push({ - key: 'trigger-' + triggerIndex, - label: trigger.$designer.name || getFriendlyName(trigger), - link: { - projectId: props.projectId, - dialogId: props.dialogId, - trigger: triggerIndex, - }, - onClick: () => navTo(projectId, dialogId, `${triggerIndex}`), - }); - } - - // getDialogData returns whatever's at the end of the path, which could be a trigger or an action - const possibleAction = getDialogData(dialogMap, dialogId, focusPath); + const data = getDialogData(dialogMap, dialogId, getFocusPath(selected, focused)); - if (params.get('focused') != null) { - // we've linked to an action, so put that in too - breadcrumbArray.push({ - key: 'action-' + focusPath, - label: getActionName(possibleAction), - }); - } - - if (typeof possibleAction === 'undefined') { + if (typeof data === 'undefined') { const { id: foundId } = dialogs.find(({ id }) => id === dialogId) || dialogs.find(({ isRoot }) => isRoot) || {}; /** * It's improper to fallback to `dialogId` directly: @@ -270,9 +227,9 @@ const DesignPage: React.FC = []; - if (dialogId != null) { - breadcrumbArray.push({ - key: 'dialog-' + parentLink?.dialogId, - label: parentLink?.displayName ?? link.displayName, - link: { projectId, skillId, dialogId }, - onClick: () => navTo(skillId ?? projectId, dialogId), - }); - } - if (trigger != null) { - breadcrumbArray.push({ - key: 'trigger-' + parentLink?.trigger, - label: link.displayName, - link: { projectId, skillId, dialogId, trigger }, - onClick: () => selectTo(skillId ?? null, dialogId ?? null, `triggers[${trigger}]`), - }); - } - - setBreadcrumbs(breadcrumbArray); - - if (trigger != null) { - selectTo(skillId ?? null, dialogId ?? null, `triggers[${trigger}]`); - } else if (dialogId != null) { - navTo(skillId ?? projectId, dialogId); + if (link.trigger != null) { + selectTo(link.skillId ?? null, link.dialogId ?? null, `triggers[${link.trigger}]`); + } else if (link.dialogId != null) { + navTo(link.skillId ?? link.projectId, link.dialogId, [ + { skillId: link.skillId, dialogId: link.dialogId, selected: '', focused: '' }, + ]); } else { // with no dialog or ID, we must be looking at a bot link - navTo(skillId ?? projectId, null); + navTo(link.skillId ?? link.projectId, null, []); } } const onCreateDialogComplete = (dialogId) => { if (dialogId) { - navTo(projectId, dialogId); - } - }; - - const pluginConfig: PluginConfig = useMemo(() => { - const sdkUISchema = schemas?.ui?.content ?? {}; - const userUISchema = schemas?.uiOverrides?.content ?? {}; - return mergePluginConfigs({ uiSchema: sdkUISchema }, plugins, { uiSchema: userUISchema }); - }, [schemas?.ui?.content, schemas?.uiOverrides?.content]); - - const getActionName = (action) => { - const nameFromAction = action?.$designer?.name as string | undefined; - let detectedActionName: string; - - if (typeof nameFromAction === 'string') { - detectedActionName = nameFromAction; - } else { - const kind: string = action?.$kind as string; - const actionNameFromSchema = pluginConfig?.uiSchema?.[kind]?.form?.label as string | (() => string) | undefined; - if (typeof actionNameFromSchema === 'string') { - detectedActionName = actionNameFromSchema; - } else if (typeof actionNameFromSchema === 'function') { - detectedActionName = actionNameFromSchema(); - } else { - detectedActionName = formatMessage('Unknown'); - } + navTo(projectId, dialogId, [{ dialogId, selected: '', focused: '' }]); } - return detectedActionName; }; const { actionSelected, showDisableBtn, showEnableBtn } = useMemo(() => { @@ -374,14 +286,6 @@ const DesignPage: React.FC get(currentDialog?.content, id)); const showDisableBtn = selectedActions.some((x) => get(x, 'disabled') !== true); const showEnableBtn = selectedActions.some((x) => get(x, 'disabled') === true); - - if (selectedActions.length === 1 && selectedActions[0] != null) { - const action = selectedActions[0] as any; - const actionName = getActionName(action); - - setBreadcrumbs((prev) => [...prev.slice(0, 2), { key: 'action-' + actionName, label: actionName }]); - } - return { actionSelected, showDisableBtn, showEnableBtn }; }, [visualEditorSelection, currentDialog?.content]); @@ -550,37 +454,55 @@ const DesignPage: React.FC IBreadcrumbItem = (breadcrumb: BreadcrumbItem) => { - return { - key: breadcrumb.key, - text: breadcrumb.label, - onClick: () => breadcrumb.onClick?.(), - }; - }; + function handleBreadcrumbItemClick(_event, item) { + if (item) { + const { dialogId, selected, focused, index } = item; + selectAndFocus(projectId, skillId, dialogId, selected, focused, clearBreadcrumb(breadcrumb, index)); + } + } - const items = breadcrumbs.map(createBreadcrumbItem); - - const breadcrumbItems = ( -
- undefined} - /> -
- { - setDialogJsonVisibility((current) => !current); - }} - > - {dialogJsonVisible ? formatMessage('Hide code') : formatMessage('Show code')} - + const breadcrumbItems = useMemo(() => { + const items = + dialogs.length > 0 + ? breadcrumb.reduce((result, item, index) => { + const { dialogId, selected, focused } = item; + const text = getBreadcrumbLabel(dialogs, dialogId, selected, focused); + if (text) { + result.push({ + // @ts-ignore + index, + isRoot: !selected && !focused, + text, + ...item, + onClick: handleBreadcrumbItemClick, + }); + } + return result; + }, [] as IBreadcrumbItem[]) + : []; + return ( +
+ undefined} + onRenderItem={onRenderBreadcrumbItem} + /> +
+ { + setDialogJsonVisibility((current) => !current); + }} + > + {dialogJsonVisible ? formatMessage('Hide code') : formatMessage('Show code')} + +
-
- ); + ); + }, [dialogs, breadcrumb, dialogJsonVisible]); async function handleCreateDialogSubmit(dialogName, dialogData) { await createDialog({ id: dialogName, content: dialogData, projectId }); @@ -628,7 +550,7 @@ const DesignPage: React.FC { + const sdkUISchema = schemas?.ui?.content ?? {}; + const userUISchema = schemas?.uiOverrides?.content ?? {}; + return mergePluginConfigs({ uiSchema: sdkUISchema }, plugins, { uiSchema: userUISchema }); + }, [schemas?.ui?.content, schemas?.uiOverrides?.content]); + if (!dialogId) { return ; } @@ -659,6 +587,13 @@ const DesignPage: React.FC t.id === selected); const withWarning = triggerNotSupported(currentDialog, selectedTrigger); + const parseTriggerId = (triggerId: string | undefined): number | undefined => { + if (triggerId == null) return undefined; + const indexString = triggerId.match(/\d+/)?.[0]; + if (indexString == null) return undefined; + return parseInt(indexString); + }; + return (
diff --git a/Composer/packages/client/src/recoilModel/atoms/botState.ts b/Composer/packages/client/src/recoilModel/atoms/botState.ts index fc6c316599..09772c3cc2 100644 --- a/Composer/packages/client/src/recoilModel/atoms/botState.ts +++ b/Composer/packages/client/src/recoilModel/atoms/botState.ts @@ -24,7 +24,7 @@ import { BotLoadError, DesignPageLocation } from '../../recoilModel/types'; import FilePersistence from '../persistence/FilePersistence'; import { BotStatus } from './../../constants'; -import { PublishType } from './../../recoilModel/types'; +import { BreadcrumbItem, PublishType } from './../../recoilModel/types'; const getFullyQualifiedKey = (value: string) => { return `Bot_${value}_State`; @@ -180,6 +180,13 @@ export const skillManifestsState = atomFamily({ }, }); +export const breadcrumbState = atomFamily({ + key: getFullyQualifiedKey('breadcrumb'), + default: (id) => { + return []; + }, +}); + export const showCreateDialogModalState = atomFamily({ key: getFullyQualifiedKey('showCreateDialogModal'), default: (id) => { diff --git a/Composer/packages/client/src/recoilModel/dispatchers/__tests__/navigation.test.tsx b/Composer/packages/client/src/recoilModel/dispatchers/__tests__/navigation.test.tsx index 1ae9e0ca06..9661ff113f 100644 --- a/Composer/packages/client/src/recoilModel/dispatchers/__tests__/navigation.test.tsx +++ b/Composer/packages/client/src/recoilModel/dispatchers/__tests__/navigation.test.tsx @@ -7,12 +7,20 @@ import { SDKKinds } from '@bfc/shared'; import { navigationDispatcher } from '../navigation'; import { renderRecoilHook } from '../../../../__tests__/testUtils'; -import { focusPathState, designPageLocationState } from '../../atoms/botState'; +import { focusPathState, breadcrumbState, designPageLocationState } from '../../atoms/botState'; import { dialogsSelectorFamily } from '../../selectors'; import { dispatcherState } from '../../../recoilModel/DispatcherWrapper'; import { Dispatcher } from '../../../recoilModel/dispatchers'; -import { convertPathToUrl, navigateTo, checkUrl, getUrlSearch } from '../../../utils/navigation'; +import { + convertPathToUrl, + navigateTo, + checkUrl, + updateBreadcrumb, + getUrlSearch, + BreadcrumbUpdateType, +} from '../../../utils/navigation'; import { createSelectedPath, getSelected } from '../../../utils/dialogUtil'; +import { BreadcrumbItem } from '../../../recoilModel/types'; import { currentProjectIdState, botProjectIdsState, botProjectFileState, projectMetaDataState } from '../../atoms'; jest.mock('../../../utils/navigation'); @@ -21,6 +29,7 @@ jest.mock('../../../utils/dialogUtil'); const mockCheckUrl = checkUrl as jest.Mock; const mockNavigateTo = navigateTo as jest.Mock; const mockGetSelected = getSelected as jest.Mock; +const mockUpdateBreadcrumb = updateBreadcrumb as jest.Mock; const mockGetUrlSearch = getUrlSearch as jest.Mock; const mockConvertPathToUrl = convertPathToUrl as jest.Mock; const mockCreateSelectedPath = createSelectedPath as jest.Mock; @@ -28,8 +37,8 @@ const mockCreateSelectedPath = createSelectedPath as jest.Mock; const projectId = '12345.678'; const skillId = '98765.4321'; -function expectNavTo(location: string) { - expect(mockNavigateTo).toHaveBeenLastCalledWith(location); +function expectNavTo(location: string, state: {} | null = null) { + expect(mockNavigateTo).toHaveBeenLastCalledWith(location, state == null ? expect.anything() : state); } describe('navigation dispatcher', () => { @@ -37,6 +46,7 @@ describe('navigation dispatcher', () => { beforeEach(() => { mockCheckUrl.mockClear(); mockNavigateTo.mockClear(); + mockUpdateBreadcrumb.mockReturnValue([]); mockConvertPathToUrl.mockClear(); mockCreateSelectedPath.mockClear(); @@ -44,6 +54,7 @@ describe('navigation dispatcher', () => { const useRecoilTestHook = () => { const focusPath = useRecoilValue(focusPathState(projectId)); + const breadcrumb = useRecoilValue(breadcrumbState(projectId)); const designPageLocation = useRecoilValue(designPageLocationState(projectId)); const dialogs = useRecoilValue(dialogsSelectorFamily(projectId)); const currentDispatcher = useRecoilValue(dispatcherState); @@ -51,6 +62,7 @@ describe('navigation dispatcher', () => { return { dialogs, focusPath, + breadcrumb, designPageLocation, projectId, currentDispatcher, @@ -60,6 +72,7 @@ describe('navigation dispatcher', () => { const { result } = renderRecoilHook(useRecoilTestHook, { states: [ { recoilState: focusPathState(projectId), initialValue: 'path' }, + { recoilState: breadcrumbState(projectId), initialValue: [{ dialogId: '100', selected: 'a', focused: 'b' }] }, { recoilState: designPageLocationState(projectId), initialValue: { @@ -111,11 +124,17 @@ describe('navigation dispatcher', () => { await act(async () => { await dispatcher.setDesignPageLocation(projectId, { dialogId: 'dialogId', + breadcrumb: [], promptTab: undefined, }); }); expect(renderedComponent.current.focusPath).toEqual('dialogId#'); - + expect(renderedComponent.current.breadcrumb).toHaveLength(1); + expect(renderedComponent.current.breadcrumb[0]).toEqual({ + dialogId: 'dialogId', + focused: '', + selected: '', + }); expect(renderedComponent.current.designPageLocation).toEqual({ dialogId: 'dialogId', promptTab: undefined, @@ -128,12 +147,18 @@ describe('navigation dispatcher', () => { await act(async () => { await dispatcher.setDesignPageLocation(projectId, { dialogId: 'dialogId', + breadcrumb: [], selected: 'select', promptTab: undefined, }); }); expect(renderedComponent.current.focusPath).toEqual('dialogId#.select'); - + expect(renderedComponent.current.breadcrumb).toHaveLength(1); + expect(renderedComponent.current.breadcrumb[0]).toEqual({ + dialogId: 'dialogId', + focused: '', + selected: 'select', + }); expect(renderedComponent.current.designPageLocation).toEqual({ dialogId: 'dialogId', promptTab: undefined, @@ -146,13 +171,19 @@ describe('navigation dispatcher', () => { await act(async () => { await dispatcher.setDesignPageLocation(projectId, { dialogId: 'dialogId', + breadcrumb: [], focused: 'focus', selected: 'select', promptTab: undefined, }); }); expect(renderedComponent.current.focusPath).toEqual('dialogId#.focus'); - + expect(renderedComponent.current.breadcrumb).toHaveLength(1); + expect(renderedComponent.current.breadcrumb[0]).toEqual({ + dialogId: 'dialogId', + focused: 'focus', + selected: 'select', + }); expect(renderedComponent.current.designPageLocation).toEqual({ dialogId: 'dialogId', promptTab: undefined, @@ -166,7 +197,7 @@ describe('navigation dispatcher', () => { it('navigates to a destination', async () => { mockConvertPathToUrl.mockReturnValue(`/bot/${projectId}/dialogs/dialogId`); await act(async () => { - await dispatcher.navTo(projectId, 'dialogId'); + await dispatcher.navTo(projectId, 'dialogId', []); }); expectNavTo(`/bot/${projectId}/dialogs/dialogId`); expect(mockConvertPathToUrl).toBeCalledWith(projectId, projectId, 'dialogId'); @@ -175,7 +206,7 @@ describe('navigation dispatcher', () => { it("doesn't navigate to a destination where we already are", async () => { mockCheckUrl.mockReturnValue(true); await act(async () => { - await dispatcher.navTo(projectId, 'dialogId'); + await dispatcher.navTo(projectId, 'dialogId', []); }); expect(mockNavigateTo).not.toBeCalled(); }); @@ -230,6 +261,8 @@ describe('navigation dispatcher', () => { await dispatcher.focusTo(projectId, null, 'focus', ''); }); expectNavTo(`/bot/${projectId}/dialogs/dialogId?selected=select&focused=focus`); + expect(mockUpdateBreadcrumb).toHaveBeenCalledWith(expect.anything(), BreadcrumbUpdateType.Selected); + expect(mockUpdateBreadcrumb).toHaveBeenCalledWith(expect.anything(), BreadcrumbUpdateType.Focused); }); it('goes to a focused page with skill', async () => { @@ -238,6 +271,8 @@ describe('navigation dispatcher', () => { await dispatcher.focusTo(projectId, skillId, 'focus', ''); }); expectNavTo(`/bot/${projectId}/skill/${skillId}/dialogs/dialogInSkillId?selected=select&focused=focus`); + expect(mockUpdateBreadcrumb).toHaveBeenCalledWith(expect.anything(), BreadcrumbUpdateType.Selected); + expect(mockUpdateBreadcrumb).toHaveBeenCalledWith(expect.anything(), BreadcrumbUpdateType.Focused); }); it('goes to a focused page with fragment', async () => { @@ -246,6 +281,8 @@ describe('navigation dispatcher', () => { await dispatcher.focusTo(projectId, null, 'focus', 'fragment'); }); expectNavTo(`/bot/${projectId}/dialogs/dialogId?selected=select&focused=focus#fragment`); + expect(mockUpdateBreadcrumb).toHaveBeenCalledWith(expect.anything(), BreadcrumbUpdateType.Selected); + expect(mockUpdateBreadcrumb).toHaveBeenCalledWith(expect.anything(), BreadcrumbUpdateType.Focused); }); it('goes to a focused page with skill and fragment', async () => { @@ -254,6 +291,8 @@ describe('navigation dispatcher', () => { await dispatcher.focusTo(projectId, skillId, 'focus', 'fragment'); }); expectNavTo(`/bot/${projectId}/skill/${skillId}/dialogs/dialogInSkillId?selected=select&focused=focus#fragment`); + expect(mockUpdateBreadcrumb).toHaveBeenCalledWith(expect.anything(), BreadcrumbUpdateType.Selected); + expect(mockUpdateBreadcrumb).toHaveBeenCalledWith(expect.anything(), BreadcrumbUpdateType.Focused); }); it('stays on the same page but updates breadcrumbs with a checked URL', async () => { @@ -263,6 +302,8 @@ describe('navigation dispatcher', () => { await dispatcher.focusTo(projectId, null, 'focus', 'fragment'); }); expect(mockNavigateTo).not.toBeCalled(); + expect(mockUpdateBreadcrumb).toHaveBeenCalledWith(expect.anything(), BreadcrumbUpdateType.Selected); + expect(mockUpdateBreadcrumb).toHaveBeenCalledWith(expect.anything(), BreadcrumbUpdateType.Focused); }); }); diff --git a/Composer/packages/client/src/recoilModel/dispatchers/navigation.ts b/Composer/packages/client/src/recoilModel/dispatchers/navigation.ts index c4440aa69b..e18df80e4d 100644 --- a/Composer/packages/client/src/recoilModel/dispatchers/navigation.ts +++ b/Composer/packages/client/src/recoilModel/dispatchers/navigation.ts @@ -11,14 +11,22 @@ import { encodeArrayPathToDesignerPath } from '../../utils/convertUtils/designer import { dialogsSelectorFamily, rootBotProjectIdSelector } from '../selectors'; import { getSelected } from './../../utils/dialogUtil'; -import { designPageLocationState, focusPathState } from './../atoms/botState'; -import { checkUrl, convertPathToUrl, getUrlSearch, navigateTo } from './../../utils/navigation'; +import { BreadcrumbItem } from './../../recoilModel/types'; +import { breadcrumbState, designPageLocationState, focusPathState } from './../atoms/botState'; +import { + BreadcrumbUpdateType, + checkUrl, + convertPathToUrl, + getUrlSearch, + navigateTo, + updateBreadcrumb, +} from './../../utils/navigation'; export const navigationDispatcher = () => { const setDesignPageLocation = useRecoilCallback( ({ set }: CallbackInterface) => async ( projectId: string, - { dialogId = '', selected = '', focused = '', promptTab } + { dialogId = '', selected = '', focused = '', breadcrumb = [], promptTab } ) => { let focusPath = dialogId + '#'; if (focused) { @@ -28,6 +36,8 @@ export const navigationDispatcher = () => { } set(currentProjectIdState, projectId); set(focusPathState(projectId), focusPath); + //add current path to the breadcrumb + set(breadcrumbState(projectId), [...breadcrumb, { dialogId, selected, focused }]); set(designPageLocationState(projectId), { dialogId, selected, @@ -41,7 +51,7 @@ export const navigationDispatcher = () => { ({ snapshot, set }: CallbackInterface) => async ( skillId: string | null, dialogId: string | null, - trigger?: string + breadcrumb: BreadcrumbItem[] = [] ) => { const rootBotProjectId = await snapshot.getPromise(rootBotProjectIdSelector); if (rootBotProjectId == null) return; @@ -51,13 +61,10 @@ export const navigationDispatcher = () => { const designPageLocation = await snapshot.getPromise(designPageLocationState(projectId)); set(currentProjectIdState, projectId); - const currentUri = - trigger == null - ? convertPathToUrl(rootBotProjectId, skillId, dialogId) - : convertPathToUrl(rootBotProjectId, skillId, dialogId, `selected=triggers[${trigger}]`); + const currentUri = convertPathToUrl(rootBotProjectId, projectId, dialogId); if (checkUrl(currentUri, rootBotProjectId, projectId, designPageLocation)) return; - navigateTo(currentUri); + navigateTo(currentUri, { state: { breadcrumb } }); } ); @@ -75,6 +82,7 @@ export const navigationDispatcher = () => { set(currentProjectIdState, projectId); const designPageLocation = await snapshot.getPromise(designPageLocationState(projectId)); + const breadcrumb = await snapshot.getPromise(breadcrumbState(projectId)); // target dialogId, projectId maybe empty string "" const dialogId = destinationDialogId ?? designPageLocation.dialogId ?? 'Main'; @@ -85,7 +93,7 @@ export const navigationDispatcher = () => { const currentUri = convertPathToUrl(rootBotProjectId, skillId, dialogId, encodedSelectPath); if (checkUrl(currentUri, rootBotProjectId, skillId, designPageLocation)) return; - navigateTo(currentUri); + navigateTo(currentUri, { state: { breadcrumb: updateBreadcrumb(breadcrumb, BreadcrumbUpdateType.Selected) } }); } ); @@ -98,10 +106,12 @@ export const navigationDispatcher = () => { ) => { set(currentProjectIdState, skillId ?? projectId); const designPageLocation = await snapshot.getPromise(designPageLocationState(skillId ?? projectId)); + const breadcrumb = await snapshot.getPromise(breadcrumbState(skillId ?? projectId)); + let updatedBreadcrumb = [...breadcrumb]; const { dialogId, selected } = designPageLocation; let currentUri = - skillId == null || skillId === projectId + skillId == null ? `/bot/${projectId}/dialogs/${dialogId}` : `/bot/${projectId}/skill/${skillId}/dialogs/${dialogId}`; @@ -111,17 +121,22 @@ export const navigationDispatcher = () => { const encodedFocusPath = encodeArrayPathToDesignerPath(currentDialog?.content, focusPath); const targetSelected = getSelected(encodedFocusPath); - + if (targetSelected !== selected) { + updatedBreadcrumb = updateBreadcrumb(breadcrumb, BreadcrumbUpdateType.Selected); + updatedBreadcrumb.push({ dialogId, selected: targetSelected, focused: '' }); + } currentUri = `${currentUri}?selected=${targetSelected}&focused=${encodedFocusPath}`; + updatedBreadcrumb = updateBreadcrumb(breadcrumb, BreadcrumbUpdateType.Focused); } else { currentUri = `${currentUri}?selected=${selected}`; + updatedBreadcrumb = updateBreadcrumb(breadcrumb, BreadcrumbUpdateType.Selected); } if (fragment && typeof fragment === 'string') { currentUri += `#${fragment}`; } if (checkUrl(currentUri, projectId, skillId, designPageLocation)) return; - navigateTo(currentUri); + navigateTo(currentUri, { state: { breadcrumb: updatedBreadcrumb } }); } ); @@ -131,7 +146,8 @@ export const navigationDispatcher = () => { skillId: string | null, dialogId: string, selectPath: string, - focusPath: string + focusPath: string, + breadcrumb: BreadcrumbItem[] = [] ) => { set(currentProjectIdState, projectId); @@ -143,14 +159,14 @@ export const navigationDispatcher = () => { const designPageLocation = await snapshot.getPromise(designPageLocationState(projectId)); if (search) { const currentUri = - skillId == null || skillId === projectId + skillId == null ? `/bot/${projectId}/dialogs/${dialogId}${search}` : `/bot/${projectId}/skill/${skillId}/dialogs/${dialogId}${search}`; if (checkUrl(currentUri, projectId, skillId, designPageLocation)) return; - navigateTo(currentUri); + navigateTo(currentUri, { state: { breadcrumb } }); } else { - navTo(skillId ?? projectId, dialogId); + navTo(skillId ?? projectId, dialogId, breadcrumb); } } ); diff --git a/Composer/packages/client/src/recoilModel/dispatchers/publisher.ts b/Composer/packages/client/src/recoilModel/dispatchers/publisher.ts index 96f342d7c7..e4ec8f5a2e 100644 --- a/Composer/packages/client/src/recoilModel/dispatchers/publisher.ts +++ b/Composer/packages/client/src/recoilModel/dispatchers/publisher.ts @@ -68,7 +68,6 @@ export const publisherDispatcher = () => { }; const updatePublishStatus = ({ set }: CallbackInterface, projectId: string, target: any, data: any) => { - if (data == null) return; const { endpointURL, status, id } = data; // the action below only applies to when a bot is being started using the "start bot" button // a check should be added to this that ensures this ONLY applies to the "default" profile. @@ -174,9 +173,9 @@ export const publisherDispatcher = () => { (callbackHelpers: CallbackInterface) => async (projectId: string, target: any) => { try { const response = await httpClient.get(`/publish/${projectId}/status/${target.name}`); - updatePublishStatus(callbackHelpers, projectId, target, response?.data); + updatePublishStatus(callbackHelpers, projectId, target, response.data); } catch (err) { - updatePublishStatus(callbackHelpers, projectId, target, err.response?.data); + updatePublishStatus(callbackHelpers, projectId, target, err.response.data); } } ); diff --git a/Composer/packages/client/src/recoilModel/types.ts b/Composer/packages/client/src/recoilModel/types.ts index b22608fec3..453eca41b0 100644 --- a/Composer/packages/client/src/recoilModel/types.ts +++ b/Composer/packages/client/src/recoilModel/types.ts @@ -76,6 +76,13 @@ export interface AppUpdateState { version?: string; } +export interface BreadcrumbItem { + skillId?: string; + dialogId: string; + selected: string; + focused: string; +} + export type dialogPayload = { id: string; content: any; @@ -87,7 +94,7 @@ export type DesignPageLocationPayload = { dialogId: string; selected: string; focused: string; - breadcrumb: string[]; + breadcrumb: BreadcrumbItem[]; promptTab?: string; }; diff --git a/Composer/packages/client/src/shell/useShell.ts b/Composer/packages/client/src/shell/useShell.ts index f7bab76ed6..2026a0414e 100644 --- a/Composer/packages/client/src/shell/useShell.ts +++ b/Composer/packages/client/src/shell/useShell.ts @@ -16,6 +16,7 @@ import { clipboardActionsState, schemasState, validateDialogsSelectorFamily, + breadcrumbState, focusPathState, skillsState, localeState, @@ -65,6 +66,7 @@ export function useShell(source: EventSource, projectId: string): Shell { const schemas = useRecoilValue(schemasState(projectId)); const dialogs = useRecoilValue(validateDialogsSelectorFamily(projectId)); + const breadcrumb = useRecoilValue(breadcrumbState(projectId)); const focusPath = useRecoilValue(focusPathState(projectId)); const skills = useRecoilValue(skillsState(projectId)); const locale = useRecoilValue(localeState(projectId)); @@ -135,9 +137,9 @@ export function useShell(source: EventSource, projectId: string): Shell { updateDialog({ id, content: newDialog.content, projectId }); } - function navigationTo(path, rest?) { + function navigationTo(path) { if (rootBotProjectId == null) return; - navTo(projectId, path, rest); + navTo(projectId, path, breadcrumb); } function focusEvent(subPath) { diff --git a/Composer/packages/client/src/utils/convertUtils/designerPathEncoder.ts b/Composer/packages/client/src/utils/convertUtils/designerPathEncoder.ts index 402edc1874..657a050f33 100644 --- a/Composer/packages/client/src/utils/convertUtils/designerPathEncoder.ts +++ b/Composer/packages/client/src/utils/convertUtils/designerPathEncoder.ts @@ -101,12 +101,7 @@ export const decodeDesignerPathToArrayPath = (dialog, path: string): string => { let arrayIndex = -1; if (designerPathInfo) { const { designerId } = designerPathInfo; - if (designerId === 'beginDialog') { - // special notation to route to this specific trigger - arrayIndex = arrayData.findIndex((x) => get(x, '$kind') === 'Microsoft.OnBeginDialog'); - } else { - arrayIndex = arrayData.findIndex((x) => get(x, '$designer.id') === designerId); - } + arrayIndex = arrayData.findIndex((x) => get(x, '$designer.id') === designerId); } else if (arrayPathInfo) { arrayIndex = arrayPathInfo.index; } diff --git a/Composer/packages/client/src/utils/navigation.ts b/Composer/packages/client/src/utils/navigation.ts index 7a8432d25e..dcc7b19fe4 100644 --- a/Composer/packages/client/src/utils/navigation.ts +++ b/Composer/packages/client/src/utils/navigation.ts @@ -1,15 +1,20 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +import cloneDeep from 'lodash/cloneDeep'; import { navigate, NavigateOptions } from '@reach/router'; -import { DesignPageLocation } from '../recoilModel/types'; +import { BreadcrumbItem, DesignPageLocation } from '../recoilModel/types'; import { BASEPATH } from '../constants'; import { parsePathToFocused } from './convertUtils/parsePathToFocused'; import { parsePathToSelected } from './convertUtils/parsePathToSelected'; import { parseTypeToFragment } from './convertUtils/parseTypeToFragment'; import { resolveToBasePath } from './fileUtil'; +export const BreadcrumbUpdateType = { + Selected: 'selected', + Focused: 'focused', +}; export function getFocusPath(selected: string, focused: string): string { if (selected && focused) return focused; @@ -19,6 +24,31 @@ export function getFocusPath(selected: string, focused: string): string { return ''; } +export function clearBreadcrumb(breadcrumb: BreadcrumbItem[], fromIndex?: number): BreadcrumbItem[] { + let breadcrumbCopy = cloneDeep(breadcrumb); + if (fromIndex) { + breadcrumbCopy.splice(fromIndex, breadcrumbCopy.length - fromIndex); + } else { + breadcrumbCopy = []; + } + return breadcrumbCopy; +} + +export function updateBreadcrumb(breadcrumb: BreadcrumbItem[], type: string): BreadcrumbItem[] { + const breadcrumbCopy = cloneDeep(breadcrumb); + if (breadcrumbCopy.length === 0) { + return breadcrumbCopy; + } + + let lastIndex = breadcrumbCopy.length - 1; + while (lastIndex > 0 && breadcrumbCopy[lastIndex][type]) { + breadcrumbCopy.pop(); + lastIndex--; + } + + return breadcrumbCopy; +} + export function getUrlSearch(selected: string, focused: string): string { const search = new URLSearchParams(); if (selected) { @@ -53,7 +83,7 @@ export function checkUrl( } export interface NavigationState { - breadcrumb?: string[]; + breadcrumb?: BreadcrumbItem[]; qnaKbUrls?: string[]; } @@ -67,7 +97,7 @@ export function convertPathToUrl( //uri = id?selected=triggers[0]&focused=triggers[0].actions[0] let uri = `/bot/${projectId}`; - if (skillId != null && skillId !== projectId) { + if (skillId != null) { uri += `/skill/${skillId}`; } if (dialogId != null) { diff --git a/Composer/packages/lib/shared/__tests__/dialogUtils/validateDialogName.test.ts b/Composer/packages/lib/shared/__tests__/dialogUtils/validateDialogName.test.ts index 0666c0dbc1..8edcbec0a2 100644 --- a/Composer/packages/lib/shared/__tests__/dialogUtils/validateDialogName.test.ts +++ b/Composer/packages/lib/shared/__tests__/dialogUtils/validateDialogName.test.ts @@ -4,7 +4,7 @@ import { validateDialogName } from '../../src/dialogUtils/validateDialogName'; const error = new Error( - 'Spaces and special characters are not allowed. Use letters, numbers, -, or _, and begin the name with a letter.' + "Spaces and special characters are not allowed. Use letters, numbers, -, or _ and don't use number at the beginning." ); const emptyError = new Error('The file name can not be empty'); diff --git a/Composer/packages/lib/shared/src/dialogUtils/validateDialogName.ts b/Composer/packages/lib/shared/src/dialogUtils/validateDialogName.ts index 6450599600..cf6d2fad1c 100644 --- a/Composer/packages/lib/shared/src/dialogUtils/validateDialogName.ts +++ b/Composer/packages/lib/shared/src/dialogUtils/validateDialogName.ts @@ -13,7 +13,7 @@ export const validateDialogName = (name: string) => { if (!nameRegex.test(name)) { throw new Error( formatMessage( - 'Spaces and special characters are not allowed. Use letters, numbers, -, or _, and begin the name with a letter.' + "Spaces and special characters are not allowed. Use letters, numbers, -, or _ and don't use number at the beginning." ) ); } diff --git a/Composer/packages/server/src/locales/en-US.json b/Composer/packages/server/src/locales/en-US.json index 9507708939..b02415fd27 100644 --- a/Composer/packages/server/src/locales/en-US.json +++ b/Composer/packages/server/src/locales/en-US.json @@ -2330,12 +2330,12 @@ "spaces_and_special_characters_are_not_allowed_20d47684": { "message": "Spaces and special characters are not allowed." }, + "spaces_and_special_characters_are_not_allowed_use__2a61c454": { + "message": "Spaces and special characters are not allowed. Use letters, numbers, -, or _ and don''t use number at the beginning." + }, "spaces_and_special_characters_are_not_allowed_use__48acec3c": { "message": "Spaces and special characters are not allowed. Use letters, numbers, -, or _." }, - "spaces_and_special_characters_are_not_allowed_use__d24a8636": { - "message": "Spaces and special characters are not allowed. Use letters, numbers, -, or _, and begin the name with a letter." - }, "specify_a_name_and_description_for_your_new_dialog_86eb3130": { "message": "Specify a name and description for your new dialog." }, @@ -2630,9 +2630,6 @@ "uninstall_8730233": { "message": "Uninstall" }, - "unknown_47a3b725": { - "message": "Unknown" - }, "unknown_intent_44b962ba": { "message": "Unknown intent" }, diff --git a/Composer/packages/server/src/models/bot/__tests__/botProject.test.ts b/Composer/packages/server/src/models/bot/__tests__/botProject.test.ts index e7fcfa384a..baeebe3c52 100644 --- a/Composer/packages/server/src/models/bot/__tests__/botProject.test.ts +++ b/Composer/packages/server/src/models/bot/__tests__/botProject.test.ts @@ -356,7 +356,7 @@ describe('dialog schema operations', () => { describe('should validate the file name when create a new one', () => { const error = new Error( - 'Spaces and special characters are not allowed. Use letters, numbers, -, or _, and begin the name with a letter.' + "Spaces and special characters are not allowed. Use letters, numbers, -, or _ and don't use number at the beginning." ); const emptyError = new Error('The file name can not be empty'); From 4e32c66633959fd5943f29467665d142b275eb7f Mon Sep 17 00:00:00 2001 From: leilzh Date: Fri, 6 Nov 2020 09:06:49 +0800 Subject: [PATCH 5/7] add selector for undo --- .../client/src/pages/design/DesignPage.tsx | 5 ++-- .../client/src/recoilModel/selectors/undo.ts | 14 +++++++++++ .../client/src/recoilModel/undo/history.ts | 24 ++++++++++--------- 3 files changed, 30 insertions(+), 13 deletions(-) create mode 100644 Composer/packages/client/src/recoilModel/selectors/undo.ts diff --git a/Composer/packages/client/src/pages/design/DesignPage.tsx b/Composer/packages/client/src/pages/design/DesignPage.tsx index 7a0af7ea42..5bb9b395bb 100644 --- a/Composer/packages/client/src/pages/design/DesignPage.tsx +++ b/Composer/packages/client/src/pages/design/DesignPage.tsx @@ -46,9 +46,10 @@ import { } from '../../recoilModel'; import { CreateQnAModal } from '../../components/QnA'; import { triggerNotSupported } from '../../utils/dialogValidator'; -import { undoFunctionState, undoStatusState, undoVersionState } from '../../recoilModel/undo/history'; +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 { @@ -132,7 +133,7 @@ const DesignPage: React.FC({ + key: 'undoStatus', + get: (projectId: string) => ({ get }) => { + const canUndo = get(canUndoState(projectId)); + const canRedo = get(canRedoState(projectId)); + return [canUndo, canRedo]; + }, +}); diff --git a/Composer/packages/client/src/recoilModel/undo/history.ts b/Composer/packages/client/src/recoilModel/undo/history.ts index f189cce3e6..a5a661e755 100644 --- a/Composer/packages/client/src/recoilModel/undo/history.ts +++ b/Composer/packages/client/src/recoilModel/undo/history.ts @@ -35,12 +35,15 @@ export const undoFunctionState = atomFamily({ dangerouslyAllowMutability: true, }); -export const undoStatusState = atomFamily<{ canUndo: boolean; canRedo: boolean }, string>({ - key: 'undoStatus', - default: { - canRedo: false, - canUndo: false, - }, +export const canUndoState = atomFamily({ + key: 'canUndoState', + default: false, + dangerouslyAllowMutability: true, +}); + +export const canRedoState = atomFamily({ + key: 'canRedoState', + default: false, dangerouslyAllowMutability: true, }); @@ -160,7 +163,8 @@ export const UndoRoot = React.memo((props: UndoRootProps) => { const rootBotProjectId = useRecoilValue(rootBotProjectIdSelector); const history: UndoHistory = useRef(undoHistory).current; const [initialStateLoaded, setInitialStateLoaded] = useState(false); - const setUndoStatus = useSetRecoilState(undoStatusState(projectId)); + const setCanUndo = useSetRecoilState(canUndoState(projectId)); + const setCanRedo = useSetRecoilState(canRedoState(projectId)); const setUndoFunction = useSetRecoilState(undoFunctionState(projectId)); const [, forceUpdate] = useState([]); const setVersion = useSetRecoilState(undoVersionState(projectId)); @@ -207,10 +211,8 @@ export const UndoRoot = React.memo((props: UndoRootProps) => { }; const updateUndoResult = () => { - setUndoStatus({ - canUndo: history.canUndo(), - canRedo: history.canRedo(), - }); + setCanRedo(history.canRedo()); + setCanUndo(history.canUndo()); }; const undo = useRecoilCallback(({ snapshot, gotoSnapshot }: CallbackInterface) => () => { From ec0c498383133adcd149fa0622abc9487ffd0297 Mon Sep 17 00:00:00 2001 From: leilzh Date: Fri, 6 Nov 2020 09:13:02 +0800 Subject: [PATCH 6/7] move status --- .../client/src/recoilModel/atoms/botState.ts | 10 ++++++++++ .../client/src/recoilModel/selectors/undo.ts | 2 +- .../client/src/recoilModel/undo/history.ts | 14 +------------- 3 files changed, 12 insertions(+), 14 deletions(-) diff --git a/Composer/packages/client/src/recoilModel/atoms/botState.ts b/Composer/packages/client/src/recoilModel/atoms/botState.ts index fc6c316599..0df22daf4d 100644 --- a/Composer/packages/client/src/recoilModel/atoms/botState.ts +++ b/Composer/packages/client/src/recoilModel/atoms/botState.ts @@ -354,3 +354,13 @@ export const botNameIdentifierState = atomFamily({ key: getFullyQualifiedKey('botNameIdentifier'), default: '', }); + +export const canUndoState = atomFamily({ + key: getFullyQualifiedKey('canUndoState'), + default: false, +}); + +export const canRedoState = atomFamily({ + key: getFullyQualifiedKey('canRedoState'), + default: false, +}); diff --git a/Composer/packages/client/src/recoilModel/selectors/undo.ts b/Composer/packages/client/src/recoilModel/selectors/undo.ts index 01856c5f02..f031ca1c3e 100644 --- a/Composer/packages/client/src/recoilModel/selectors/undo.ts +++ b/Composer/packages/client/src/recoilModel/selectors/undo.ts @@ -2,7 +2,7 @@ // Licensed under the MIT License. import { selectorFamily } from 'recoil'; -import { canRedoState, canUndoState } from './../undo/history'; +import { canRedoState, canUndoState } from '../atoms/botState'; export const undoStatusSelectorFamily = selectorFamily<[boolean, boolean], string>({ key: 'undoStatus', diff --git a/Composer/packages/client/src/recoilModel/undo/history.ts b/Composer/packages/client/src/recoilModel/undo/history.ts index a5a661e755..96b06fc88c 100644 --- a/Composer/packages/client/src/recoilModel/undo/history.ts +++ b/Composer/packages/client/src/recoilModel/undo/history.ts @@ -18,7 +18,7 @@ import { encodeArrayPathToDesignerPath } from '../../utils/convertUtils/designer import { dialogsSelectorFamily } from '../selectors'; import { rootBotProjectIdSelector } from './../selectors/project'; -import { designPageLocationState } from './../atoms'; +import { canRedoState, canUndoState, designPageLocationState } from './../atoms'; import { trackedAtoms, AtomAssetsMap } from './trackedAtoms'; import UndoHistory from './undoHistory'; @@ -35,18 +35,6 @@ export const undoFunctionState = atomFamily({ dangerouslyAllowMutability: true, }); -export const canUndoState = atomFamily({ - key: 'canUndoState', - default: false, - dangerouslyAllowMutability: true, -}); - -export const canRedoState = atomFamily({ - key: 'canRedoState', - default: false, - dangerouslyAllowMutability: true, -}); - export const undoHistoryState = atomFamily({ key: 'undoHistory', default: {} as UndoHistory, From 4c893cb472b5b6eea595bbb93fd6a6389fccae42 Mon Sep 17 00:00:00 2001 From: leilzh Date: Fri, 6 Nov 2020 09:49:01 +0800 Subject: [PATCH 7/7] fix unit test --- .../src/recoilModel/undo/__test__/history.test.tsx | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/Composer/packages/client/src/recoilModel/undo/__test__/history.test.tsx b/Composer/packages/client/src/recoilModel/undo/__test__/history.test.tsx index 22613a926a..aee175dc76 100644 --- a/Composer/packages/client/src/recoilModel/undo/__test__/history.test.tsx +++ b/Composer/packages/client/src/recoilModel/undo/__test__/history.test.tsx @@ -6,7 +6,7 @@ import { jsx } from '@emotion/core'; import { act } from 'react-test-renderer'; import { useRecoilValue, useSetRecoilState, useRecoilState } from 'recoil'; -import { UndoRoot, undoFunctionState, undoHistoryState, undoStatusState } from '../history'; +import { UndoRoot, undoFunctionState, undoHistoryState } from '../history'; import { lgFilesState, luFilesState, @@ -14,10 +14,13 @@ import { 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 = () => { @@ -36,7 +39,7 @@ describe('', () => { const setProjectIdState = useSetRecoilState(currentProjectIdState); const setDesignPageLocation = useSetRecoilState(designPageLocationState(projectId)); const history = useRecoilValue(undoHistoryState(projectId)); - const { canRedo, canUndo } = useRecoilValue(undoStatusState(projectId)); + const [canUndo, canRedo] = useRecoilValue(undoStatusSelectorFamily(projectId)); return { undo, redo, @@ -68,7 +71,8 @@ describe('', () => { { recoilState: luFilesState(projectId), initialValue: [{ id: '1.lu' }, { id: '2' }] }, { recoilState: currentProjectIdState, initialValue: projectId }, { recoilState: undoHistoryState(projectId), initialValue: new UndoHistory(projectId) }, - { recoilState: undoStatusState(projectId), initialValue: { canUndo: false, canRedo: false } }, + { recoilState: canUndoState(projectId), initialValue: false }, + { recoilState: canRedoState(projectId), initialValue: false }, { recoilState: designPageLocationState(projectId), initialValue: { dialogId: '1', focused: '', selected: '' } }, { recoilState: undoFunctionState(projectId),