diff --git a/Composer/cypress/integration/LGPage.spec.ts b/Composer/cypress/integration/LGPage.spec.ts index a10f2c7c82..b9813945c6 100644 --- a/Composer/cypress/integration/LGPage.spec.ts +++ b/Composer/cypress/integration/LGPage.spec.ts @@ -30,9 +30,5 @@ context('LG Page', () => { // nav to Main dialog cy.get('.dialogNavTree a[title="__TestTodoSample.Main"]').click(); - // cy.wait(300); - - // dialog filter, edit mode button is disabled. - cy.get('@switchButton').should('be.disabled'); }); }); diff --git a/Composer/cypress/integration/NotificationPage.spec.ts b/Composer/cypress/integration/NotificationPage.spec.ts index ff21475ef0..b45e200249 100644 --- a/Composer/cypress/integration/NotificationPage.spec.ts +++ b/Composer/cypress/integration/NotificationPage.spec.ts @@ -25,7 +25,6 @@ context('Notification Page', () => { }); cy.findAllByText('Bot Responses').should('exist'); - cy.get('@switchButton').should('be.disabled'); }); it('can show lu syntax error ', () => { diff --git a/Composer/packages/client/src/pages/language-generation/code-editor.tsx b/Composer/packages/client/src/pages/language-generation/code-editor.tsx index 7dab5c8265..f0d58d7e52 100644 --- a/Composer/packages/client/src/pages/language-generation/code-editor.tsx +++ b/Composer/packages/client/src/pages/language-generation/code-editor.tsx @@ -3,67 +3,101 @@ /* eslint-disable react/display-name */ import React, { useState, useEffect, useMemo, useContext, useCallback } from 'react'; -import { LgEditor, LGOption } from '@bfc/code-editor'; +import { LgEditor } from '@bfc/code-editor'; import get from 'lodash/get'; import debounce from 'lodash/debounce'; import isEmpty from 'lodash/isEmpty'; -import { LgFile } from '@bfc/indexers'; import { editor } from '@bfcomposer/monaco-editor/esm/vs/editor/editor.api'; -import { lgIndexer, Diagnostic, combineMessage, isValid } from '@bfc/indexers'; +import { lgIndexer, combineMessage, isValid } from '@bfc/indexers'; +import { RouteComponentProps } from '@reach/router'; +import querystring from 'query-string'; import { StoreContext } from '../../store'; import * as lgUtil from '../../utils/lgUtil'; const { check } = lgIndexer; -interface CodeEditorProps { - file: LgFile; - template: lgUtil.Template | null; - line: number; -} - const lspServerPath = '/lg-language-server'; -export default function CodeEditor(props: CodeEditorProps) { - const { actions } = useContext(StoreContext); - const { file, template, line } = props; +interface CodeEditorProps extends RouteComponentProps<{}> { + fileId: string; +} + +const CodeEditor: React.FC = props => { + const { actions, state } = useContext(StoreContext); + const { lgFiles } = state; + const { fileId } = props; + const file = lgFiles?.find(({ id }) => id === 'common'); const [diagnostics, setDiagnostics] = useState(get(file, 'diagnostics', [])); const [content, setContent] = useState(''); const [errorMsg, setErrorMsg] = useState(''); const [lgEditor, setLgEditor] = useState(null); - const fileId = file && file.id; + const search = props.location?.search ?? ''; + const searchTemplateName = querystring.parse(search).t; + const templateId = Array.isArray(searchTemplateName) + ? searchTemplateName[0] + : typeof searchTemplateName === 'string' + ? searchTemplateName + : undefined; + const template = templateId && file ? file.templates.find(({ name }) => name === templateId) : undefined; + + const hash = props.location?.hash ?? ''; + const hashLine = querystring.parse(hash).L; + const line = Array.isArray(hashLine) ? +hashLine[0] : typeof hashLine === 'string' ? +hashLine : undefined; + const inlineMode = !!template; + useEffect(() => { // reset content with file.content's initial state - if (isEmpty(file)) return; - const value = template ? get(template, 'body', '') : get(file, 'content', ''); + if (!file || isEmpty(file)) return; + const value = template ? template.body : file.content; setContent(value); - }, [fileId, template]); + }, [fileId, templateId]); useEffect(() => { - const isInvalid = !isValid(diagnostics); - const text = isInvalid ? combineMessage(diagnostics) : ''; + const currentDiagnostics = + inlineMode && template + ? diagnostics.filter(d => { + return ( + d.range && + template.range && + d.range.start.line >= template.range.startLineNumber && + d.range.end.line <= template.range.endLineNumber + ); + }) + : diagnostics; + + const isInvalid = !isValid(currentDiagnostics); + const text = isInvalid ? combineMessage(currentDiagnostics) : ''; setErrorMsg(text); }, [diagnostics]); + const editorDidMount = (lgEditor: editor.IStandaloneCodeEditor) => { + setLgEditor(lgEditor); + }; + useEffect(() => { - if (lgEditor) { - lgEditor.revealLine(line); + if (lgEditor && line !== undefined) { + window.requestAnimationFrame(() => { + lgEditor.revealLine(line); + lgEditor.focus(); + lgEditor.setPosition({ lineNumber: line, column: 1 }); + }); } - }, [lgEditor]); + }, [line, lgEditor]); const updateLgTemplate = useMemo( () => debounce((body: string) => { - const templateName = get(template, 'name'); - if (!templateName) return; + if (!file || !template) return; + const { name, parameters } = template; const payload = { file, - templateName, + templateName: name, template: { - name: templateName, - parameters: get(template, 'parameters'), + name, + parameters, body, }, }; @@ -75,8 +109,10 @@ export default function CodeEditor(props: CodeEditorProps) { const updateLgFile = useMemo( () => debounce((content: string) => { + if (!file) return; + const { id } = file; const payload = { - id: file.id, + id, content, }; actions.updateLgFile(payload); @@ -87,36 +123,38 @@ export default function CodeEditor(props: CodeEditorProps) { const _onChange = useCallback( value => { setContent(value); - - let diagnostics: Diagnostic[] = []; + if (!file) return; + const { id } = file; if (inlineMode) { - const content = get(file, 'content', ''); - const templateName = get(template, 'name', ''); + if (!template) return; + const { name, parameters } = template; + const { content } = file; try { - const newContent = lgUtil.updateTemplate(content, templateName, { - name: templateName, - parameters: get(template, 'parameters'), + const newContent = lgUtil.updateTemplate(content, name, { + name, + parameters, body: value, }); - diagnostics = check(newContent, fileId); + setDiagnostics(check(newContent, id)); updateLgTemplate(value); } catch (error) { setErrorMsg(error.message); } } else { - diagnostics = check(value, fileId); + setDiagnostics(check(value, id)); updateLgFile(value); } - setDiagnostics(diagnostics); }, [file, template] ); - const lgOption: LGOption = { - inline: inlineMode, - content: get(file, 'content', ''), - template: template ? template : undefined, - }; + const lgOption = template + ? { + inline: inlineMode, + content: file?.content ?? '', + template, + } + : undefined; return ( ); -} +}; + +export default CodeEditor; diff --git a/Composer/packages/client/src/pages/language-generation/index.tsx b/Composer/packages/client/src/pages/language-generation/index.tsx index b994463ae2..9b1ea5020a 100644 --- a/Composer/packages/client/src/pages/language-generation/index.tsx +++ b/Composer/packages/client/src/pages/language-generation/index.tsx @@ -3,14 +3,11 @@ /** @jsx jsx */ import { jsx } from '@emotion/core'; -import React, { useContext, Fragment, useEffect, useState, useMemo, Suspense, useCallback } from 'react'; +import React, { useContext, Fragment, useMemo, useCallback, Suspense } from 'react'; import formatMessage from 'format-message'; import { Toggle } from 'office-ui-fabric-react/lib/Toggle'; import { Nav, INavLinkGroup, INavLink } from 'office-ui-fabric-react/lib/Nav'; -import { LGTemplate } from 'botbuilder-lg'; -import { RouteComponentProps } from '@reach/router'; -import get from 'lodash/get'; -import { isValid } from '@bfc/indexers'; +import { RouteComponentProps, Router } from '@reach/router'; import { LoadingSpinner } from '../../components/LoadingSpinner'; import { StoreContext } from '../../store'; @@ -23,7 +20,6 @@ import { } from '../language-understanding/styles'; import { projectContainer, projectTree, projectWrapper } from '../design/styles'; import { navigateTo } from '../../utils'; -import * as lgUtil from '../../utils/lgUtil'; import { Tree } from './../../components/Tree'; import TableView from './table-view'; @@ -32,29 +28,17 @@ import { TestController } from './../../TestController'; const CodeEditor = React.lazy(() => import('./code-editor')); -const LGPage: React.FC = props => { +interface LGPageProps extends RouteComponentProps<{}> { + fileId?: string; +} + +const LGPage: React.FC = props => { const { state } = useContext(StoreContext); const { lgFiles, dialogs } = state; - const [editMode, setEditMode] = useState(lgFiles.filter(file => !isValid(file.diagnostics)).length > 0); - const [fileValid, setFileValid] = useState(true); - const [inlineTemplate, setInlineTemplate] = useState(null); - const [line, setLine] = useState(0); - - const hash = props.location ? props.location.hash : ''; - const subPath = props['*']; - const isRoot = subPath === ''; - const activeDialog = dialogs.find(item => item.id === subPath); - - useEffect(() => { - if (hash) { - const match = /line=(\d+)/g.exec(hash); - if (match) setLine(+match[1]); - } - }, [hash]); - - // for now, one bot only have one lg file by default. all dialog share one lg - // file. - const lgFile = lgFiles.length ? lgFiles[0] : null; + const path = props.location?.pathname ?? ''; + const { fileId = 'common' } = props; + const edit = /edit(\/)*$/.test(path); + const file = lgFiles.find(({ id }) => id === 'common'); const navLinks = useMemo(() => { const subLinks = dialogs.reduce((result, file) => { @@ -82,8 +66,8 @@ const LGPage: React.FC = props => { { links: [ { - id: '_all', - key: '_all', + id: 'common', + key: 'common', name: 'All', url: '', isExpanded: true, @@ -94,51 +78,23 @@ const LGPage: React.FC = props => { ]; }, [dialogs]); - useEffect(() => { - // dialog lg templates is part of commong.lg. By restricting edit in root view, user would aware that the changes they made may affect other dialogs. - if (!isRoot && fileValid) { - setEditMode(false); - } - - // fall back to the all-up page if we don't have an active dialog - if (!isRoot && !activeDialog && dialogs.length) { - navigateTo('/language-generation'); - } - }, [subPath, dialogs]); - - useEffect(() => { - const errorFiles = lgFiles.filter(file => { - return !isValid(file.diagnostics); - }); - const hasError = errorFiles.length !== 0; - setFileValid(hasError === false); - if (hasError) { - setEditMode(true); - } - }, [lgFiles]); - - const onSelect = useCallback(id => { - if (id === '_all') { - navigateTo('/language-generation'); - } else { - navigateTo(`language-generation/${id}`); - } - }, []); - - const onToggleEditMode = useCallback(() => { - setEditMode(!editMode); - setInlineTemplate(null); - }, [editMode]); + const onSelect = useCallback( + id => { + let url = `/language-generation/${id}`; + if (edit) url += `/edit`; + navigateTo(url); + }, + [edit] + ); - const onTableViewClickEdit = useCallback((template: LGTemplate) => { - setInlineTemplate({ - name: get(template, 'name', ''), - parameters: get(template, 'parameters'), - body: get(template, 'body', ''), - }); - navigateTo(`/language-generation`); - setEditMode(true); - }, []); + const onToggleEditMode = useCallback( + (_e, checked) => { + let url = `/language-generation/${fileId}`; + if (checked) url += `/edit`; + navigateTo(url); + }, + [fileId] + ); const toolbarItems = [ { @@ -160,8 +116,7 @@ const LGPage: React.FC = props => { onText={formatMessage('Edit mode')} offText={formatMessage('Edit mode')} defaultChecked={false} - checked={editMode} - disabled={(!isRoot && editMode === false) || (fileValid === false && editMode === true)} + checked={!!edit} onChange={onToggleEditMode} /> @@ -190,7 +145,7 @@ const LGPage: React.FC = props => { backgroundColor: 'transparent', }, }} - selectedKey={isRoot ? '_all' : subPath} + selectedKey={fileId} groups={navLinks} className={'dialogNavTree'} data-testid={'dialogNavTree'} @@ -198,15 +153,14 @@ const LGPage: React.FC = props => { - {lgFile && ( + {file && (
- {editMode ? ( - }> - - - ) : ( - - )} + }> + + + + +
)} diff --git a/Composer/packages/client/src/pages/language-generation/table-view.tsx b/Composer/packages/client/src/pages/language-generation/table-view.tsx index 9cc26ee993..483b69d9a5 100644 --- a/Composer/packages/client/src/pages/language-generation/table-view.tsx +++ b/Composer/packages/client/src/pages/language-generation/table-view.tsx @@ -15,43 +15,39 @@ import { ScrollablePane, ScrollbarVisibility } from 'office-ui-fabric-react/lib/ import { Sticky, StickyPositionType } from 'office-ui-fabric-react/lib/Sticky'; import formatMessage from 'format-message'; import { NeutralColors, FontSizes } from '@uifabric/fluent-theme'; -import { DialogInfo, LgFile } from '@bfc/indexers'; -import { LGTemplate, LGParser } from 'botbuilder-lg'; -import { isValid } from '@bfc/indexers'; -import get from 'lodash/get'; +import { RouteComponentProps } from '@reach/router'; +import { LgTemplate } from '@bfc/indexers'; import { StoreContext } from '../../store'; -import * as lgUtil from '../../utils/lgUtil'; +import { increaseNameUtilNotExist } from '../../utils/lgUtil'; import { navigateTo } from '../../utils'; import { actionButton, formCell } from '../language-understanding/styles'; -interface TableViewProps { - file: LgFile; - activeDialog?: DialogInfo; - onClickEdit: (template: LGTemplate) => void; +interface TableViewProps extends RouteComponentProps<{}> { + fileId: string; } const TableView: React.FC = props => { const { state, actions } = useContext(StoreContext); - const { dialogs } = state; - const { file: lgFile, activeDialog, onClickEdit } = props; + const { dialogs, lgFiles } = state; + const { fileId } = props; + const file = lgFiles.find(({ id }) => id === 'common'); const createLgTemplate = useRef(debounce(actions.createLgTemplate, 500)).current; const copyLgTemplate = useRef(debounce(actions.copyLgTemplate, 500)).current; const removeLgTemplate = useRef(debounce(actions.removeLgTemplate, 500)).current; - const [templates, setTemplates] = useState([]); + const [templates, setTemplates] = useState([]); const listRef = useRef(null); + const activeDialog = dialogs.find(({ id }) => id === fileId); + useEffect(() => { - if (isEmpty(lgFile)) return; - let allTemplates: LGTemplate[] = []; - if (isValid(lgFile.diagnostics)) { - const resource = LGParser.parse(lgFile.content, ''); - allTemplates = get(resource, 'templates', []); - } + if (!file || isEmpty(file)) return; + const allTemplates = file.templates; + if (!activeDialog) { setTemplates(allTemplates); } else { - const dialogsTemplates: LGTemplate[] = []; + const dialogsTemplates: LgTemplate[] = []; activeDialog.lgTemplates.forEach(item => { const template = allTemplates.find(t => t.name === item); if (template) { @@ -60,44 +56,52 @@ const TableView: React.FC = props => { }); setTemplates(dialogsTemplates); } - }, [lgFile, activeDialog]); + }, [file, activeDialog]); + + const onClickEdit = useCallback( + (template: LgTemplate) => { + const { name } = template; + navigateTo(`/language-generation/${fileId}/edit?t=${encodeURIComponent(name)}`); + }, + [fileId] + ); const onCreateNewTemplate = useCallback(() => { - const newName = lgUtil.increaseNameUtilNotExist(templates, 'TemplateName'); + const newName = increaseNameUtilNotExist(templates, 'TemplateName'); const payload = { - file: lgFile, + file, template: { name: newName, body: '-TemplateValue', }, }; createLgTemplate(payload); - }, [templates, lgFile]); + }, [templates, file]); const onRemoveTemplate = useCallback( index => { const payload = { - file: lgFile, + file, templateName: templates[index].name, }; removeLgTemplate(payload); }, - [templates, lgFile] + [templates, file] ); const onCopyTemplate = useCallback( index => { const name = templates[index].name; - const resolvedName = lgUtil.increaseNameUtilNotExist(templates, `${name}_Copy`); + const resolvedName = increaseNameUtilNotExist(templates, `${name}_Copy`); const payload = { - file: lgFile, + file, fromTemplateName: name, toTemplateName: resolvedName, }; copyLgTemplate(payload); }, - [templates, lgFile] + [templates, file] ); const getTemplatesMoreButtons = useCallback( diff --git a/Composer/packages/client/src/pages/notifications/index.tsx b/Composer/packages/client/src/pages/notifications/index.tsx index 6cb913dbd3..74d298a30b 100644 --- a/Composer/packages/client/src/pages/notifications/index.tsx +++ b/Composer/packages/client/src/pages/notifications/index.tsx @@ -17,7 +17,7 @@ import { convertDialogDiagnosticToUrl } from './../../utils/navigation'; const navigations = { lg: (item: INotification) => { - navigateTo(`/language-generation/#line=${item.diagnostic.range?.start.line || 0}`); + navigateTo(`/language-generation/${item.id}/edit#L=${item.diagnostic.range?.start.line || 0}`); }, lu: (item: INotification) => { navigateTo(`/dialogs/${item.id}`); diff --git a/Composer/packages/client/src/router.tsx b/Composer/packages/client/src/router.tsx index c38972c914..e8271517fb 100644 --- a/Composer/packages/client/src/router.tsx +++ b/Composer/packages/client/src/router.tsx @@ -49,7 +49,8 @@ const Routes = props => { - + + diff --git a/Composer/packages/client/src/utils/lgUtil.ts b/Composer/packages/client/src/utils/lgUtil.ts index 4378df04f5..21307d6d40 100644 --- a/Composer/packages/client/src/utils/lgUtil.ts +++ b/Composer/packages/client/src/utils/lgUtil.ts @@ -7,8 +7,8 @@ * */ -import { LGParser, LGTemplate } from 'botbuilder-lg'; -import { lgIndexer, combineMessage, isValid } from '@bfc/indexers'; +import { LGParser } from 'botbuilder-lg'; +import { lgIndexer, combineMessage, isValid, LgTemplate } from '@bfc/indexers'; const { check, parse } = lgIndexer; export interface Template { @@ -26,7 +26,7 @@ export function checkLgContent(content: string, id: string) { } } -export function increaseNameUtilNotExist(templates: LGTemplate[], name: string): string { +export function increaseNameUtilNotExist(templates: LgTemplate[], name: string): string { // if duplicate, increse name with Copy1 Copy2 ... let repeatIndex = 0; @@ -42,7 +42,7 @@ export function increaseNameUtilNotExist(templates: LGTemplate[], name: string): export function updateTemplate( content: string, templateName: string, - { name, parameters = [], body }: Template + { name, parameters = [], body }: LgTemplate ): string { const resource = LGParser.parse(content); // add if not exist @@ -54,7 +54,7 @@ export function updateTemplate( } // if name exist, throw error. -export function addTemplate(content: string, { name, parameters = [], body }: Template): string { +export function addTemplate(content: string, { name, parameters = [], body }: LgTemplate): string { const resource = LGParser.parse(content); return resource.addTemplate(name, parameters, body).toString(); } @@ -62,7 +62,7 @@ export function addTemplate(content: string, { name, parameters = [], body }: Te // if name exist, add it anyway, with name like `${name}1` `${name}2` export function addTemplateAnyway( content: string, - { name = 'TemplateName', parameters = [], body = '-TemplateBody' }: Template + { name = 'TemplateName', parameters = [], body = '-TemplateBody' }: LgTemplate ): string { const resource = LGParser.parse(content); const newName = increaseNameUtilNotExist(resource.templates, name); @@ -98,11 +98,6 @@ export function copyTemplateAnyway(content: string, fromTemplateName: string, to return resource.addTemplate(newName, parameters, body).toString(); } -export function getTemplate(content: string, templateName: string): LGTemplate | undefined { - const resource = LGParser.parse(content); - return resource.templates.find(t => t.name === templateName); -} - export function removeTemplate(content: string, templateName: string): string { const resource = LGParser.parse(content); return resource.deleteTemplate(templateName).toString(); @@ -116,17 +111,7 @@ export function removeTemplates(content: string, templateNames: string[]): strin return resource.toString(); } -export function textFromTemplates(templates: Template[]): string { - const textBuilder: string[] = []; - - templates.forEach(template => { - textBuilder.push(`${textFromTemplate(template)}\n`); - }); - - return textBuilder.join(''); -} - -export function textFromTemplate(template: Template): string { +export function textFromTemplate(template: LgTemplate): string { const { name, parameters = [], body } = template; const textBuilder: string[] = []; if (name && body !== null && body !== undefined) { @@ -139,7 +124,17 @@ export function textFromTemplate(template: Template): string { return textBuilder.join(''); } -export function checkSingleLgTemplate(template: Template) { +export function textFromTemplates(templates: LgTemplate[]): string { + const textBuilder: string[] = []; + + templates.forEach(template => { + textBuilder.push(`${textFromTemplate(template)}\n`); + }); + + return textBuilder.join(''); +} + +export function checkSingleLgTemplate(template: LgTemplate) { const content = textFromTemplates([template]); if (parse(content).length !== 1) { diff --git a/Composer/packages/extensions/obiformeditor/src/Form/widgets/LgEditorWidget.tsx b/Composer/packages/extensions/obiformeditor/src/Form/widgets/LgEditorWidget.tsx index b898618943..7b624e35f0 100644 --- a/Composer/packages/extensions/obiformeditor/src/Form/widgets/LgEditorWidget.tsx +++ b/Composer/packages/extensions/obiformeditor/src/Form/widgets/LgEditorWidget.tsx @@ -74,6 +74,7 @@ export const LgEditorWidget: React.FC = props => { lgFile.diagnostics.find(d => { return ( d.range && + template.range && d.range.start.line >= template.range.startLineNumber && d.range.end.line <= template.range.endLineNumber ); diff --git a/Composer/packages/lib/indexers/src/type.ts b/Composer/packages/lib/indexers/src/type.ts index b935d644d1..86c4883e97 100644 --- a/Composer/packages/lib/indexers/src/type.ts +++ b/Composer/packages/lib/indexers/src/type.ts @@ -67,7 +67,7 @@ export interface LgTemplate { name: string; body: string; parameters: string[]; - range: CodeRange; + range?: CodeRange; } export interface LgFile {