From c9d16ef9776f85721f97307a5751e2793c7671dc Mon Sep 17 00:00:00 2001 From: Soroush Date: Thu, 11 Mar 2021 23:57:05 -0800 Subject: [PATCH 1/3] fix: Allow multiline variations for LG text and speech modalities --- .../src/lg/hooks/useStringArray.ts | 34 ++++++++++++++++--- .../lg/modalityEditors/StringArrayEditor.tsx | 23 +++++++++---- .../lg/modalityEditors/StringArrayItem.tsx | 16 ++------- 3 files changed, 49 insertions(+), 24 deletions(-) diff --git a/Composer/packages/lib/code-editor/src/lg/hooks/useStringArray.ts b/Composer/packages/lib/code-editor/src/lg/hooks/useStringArray.ts index d46d63df6c..e30b025b9f 100644 --- a/Composer/packages/lib/code-editor/src/lg/hooks/useStringArray.ts +++ b/Composer/packages/lib/code-editor/src/lg/hooks/useStringArray.ts @@ -8,6 +8,8 @@ import { ArrayBasedStructuredResponseItem, PartialStructuredResponse } from '../ import { getTemplateId } from '../../utils/structuredResponse'; import { LGOption } from '../../utils/types'; +const multiLineBlockSymbol = '```'; + const getInitialItems = ( response: T, lgTemplates?: readonly LgTemplate[], @@ -16,10 +18,29 @@ const getInitialItems = ( const templateId = getTemplateId(response); const template = lgTemplates?.find(({ name }) => name === templateId); return response?.value && template?.body - ? template?.body?.replace(/- /g, '').split(/\r?\n/g) || [] + ? template?.body + // Look behind (negative) and split by non-escaped - + // eslint-disable-next-line security/detect-unsafe-regex + ?.split(/(? s !== '' && s !== '\n') + // Remove LG template multiline block symbol + ?.map((s) => s.replace(/```/g, '')) || [] : response?.value || (focusOnMount ? [''] : []); }; +const fixMultilineItems = (items: string[]) => { + return items.map((item) => { + if (/\r?\n/g.test(item)) { + // Escape all un-escaped - + // eslint-disable-next-line security/detect-unsafe-regex + return `${multiLineBlockSymbol}${item.replace(/(?( kind: 'Text' | 'Speak', structuredResponse: T, @@ -45,18 +66,21 @@ export const useStringArray = ( const onChange = React.useCallback( (newItems: string[]) => { setItems(newItems); + // Fix variations that are multiline + // If only one item but it's multiline, still use helper LG template + const fixedNewItems = fixMultilineItems(newItems); const id = templateId || `${lgOption?.templateId}_${newTemplateNameSuffix}`; - if (!newItems.length) { + if (!fixedNewItems.length) { setTemplateId(id); onUpdateResponseTemplate({ [kind]: { kind, value: [], valueType: 'direct' } }); onRemoveTemplate(id); - } else if (newItems.length === 1 && lgOption?.templateId) { - onUpdateResponseTemplate({ [kind]: { kind, value: newItems, valueType: 'direct' } }); + } else if (fixedNewItems.length === 1 && !/\r?\n/g.test(fixedNewItems[0]) && lgOption?.templateId) { + onUpdateResponseTemplate({ [kind]: { kind, value: fixedNewItems, valueType: 'direct' } }); onTemplateChange(id, ''); } else { setTemplateId(id); onUpdateResponseTemplate({ [kind]: { kind, value: [`\${${id}()}`], valueType: 'template' } }); - onTemplateChange(id, newItems.map((item) => `- ${item}`).join('\n')); + onTemplateChange(id, fixedNewItems.map((item) => `- ${item}`).join('\n')); } }, [kind, newTemplateNameSuffix, lgOption, templateId, onRemoveTemplate, onTemplateChange, onUpdateResponseTemplate] diff --git a/Composer/packages/lib/code-editor/src/lg/modalityEditors/StringArrayEditor.tsx b/Composer/packages/lib/code-editor/src/lg/modalityEditors/StringArrayEditor.tsx index 698d67e69d..7841aee920 100644 --- a/Composer/packages/lib/code-editor/src/lg/modalityEditors/StringArrayEditor.tsx +++ b/Composer/packages/lib/code-editor/src/lg/modalityEditors/StringArrayEditor.tsx @@ -119,16 +119,27 @@ export const StringArrayEditor = React.memo( useEffect(() => { const keydownHandler = (e: KeyboardEvent) => { if (submitKeys.includes(e.key)) { - setCalloutTargetElement(null); - - const filteredItems = items.filter(Boolean); + // Allow multiline via shift+Enter + if (e.key === 'Enter' && e.shiftKey) { + return; + } + setCalloutTargetElement(null); + // Filter our empty or newline strings + const filteredItems = items.filter((s) => s !== '' && s !== '\n'); if (e.key === 'Enter' && containerRef.current?.contains(e.target as Node)) { - onChange([...filteredItems, '']); - setCurrentIndex(filteredItems.length); + // If the value is not filtered, go to the next entry + // Otherwise cancel editing + if (items.length === filteredItems.length) { + e.preventDefault(); + onChange([...filteredItems, '']); + setCurrentIndex(filteredItems.length); + } else { + onChange(filteredItems); + setCurrentIndex(null); + } } else { setCurrentIndex(null); - // Remove empty variations only if necessary if (items.length !== filteredItems.length) { onChange(filteredItems); diff --git a/Composer/packages/lib/code-editor/src/lg/modalityEditors/StringArrayItem.tsx b/Composer/packages/lib/code-editor/src/lg/modalityEditors/StringArrayItem.tsx index 3134e252e5..736c4252ac 100644 --- a/Composer/packages/lib/code-editor/src/lg/modalityEditors/StringArrayItem.tsx +++ b/Composer/packages/lib/code-editor/src/lg/modalityEditors/StringArrayItem.tsx @@ -139,11 +139,12 @@ const TextViewItem = React.memo(({ value, onRemove, onFocus, onRenderDisplayText const RemoveIcon = React.useMemo(() => withTooltip({ content: formatMessage('Remove variation') }, IconButton), []); + // Shows newline symbol when in view mode return ( - {onRenderDisplayText?.() ?? value} + {onRenderDisplayText?.() ?? value.replace(/\r?\n/g, '↵')} @@ -181,18 +182,6 @@ const TextFieldItem = React.memo(({ value, onShowCallout, onChange }: TextFieldI [onShowCallout] ); - React.useEffect(() => { - if (inputRef.current && inputRef.current.value !== value) { - const nativeInputValueSetter = Object.getOwnPropertyDescriptor(window.HTMLTextAreaElement.prototype, 'value') - ?.set; - if (nativeInputValueSetter) { - nativeInputValueSetter.call(inputRef.current, value); - const inputEvent = new Event('input', { bubbles: true }); - inputRef.current.dispatchEvent(inputEvent); - } - } - }, [value]); - return (
Date: Fri, 12 Mar 2021 00:46:16 -0800 Subject: [PATCH 2/3] fix tests --- .../code-editor/src/lg/__tests__/TextModalityEditors.test.tsx | 2 ++ .../packages/lib/code-editor/src/lg/hooks/useStringArray.ts | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/Composer/packages/lib/code-editor/src/lg/__tests__/TextModalityEditors.test.tsx b/Composer/packages/lib/code-editor/src/lg/__tests__/TextModalityEditors.test.tsx index 317d164c84..94e0343ea1 100644 --- a/Composer/packages/lib/code-editor/src/lg/__tests__/TextModalityEditors.test.tsx +++ b/Composer/packages/lib/code-editor/src/lg/__tests__/TextModalityEditors.test.tsx @@ -13,6 +13,7 @@ describe('TextModalityEditor', () => { it('should render the value if it is not a template reference', async () => { const { findByText } = render( { it('should render items from template if the value is a template reference', async () => { const { findByText, queryByText } = render( ( const template = lgTemplates?.find(({ name }) => name === templateId); return response?.value && template?.body ? template?.body - // Look behind (negative) and split by non-escaped - + // Split by non-escaped - // eslint-disable-next-line security/detect-unsafe-regex ?.split(/(? s !== '' && s !== '\n') + ?.map((s) => s.replace(/\r?\n$/g, '')) // Remove LG template multiline block symbol ?.map((s) => s.replace(/```/g, '')) || [] : response?.value || (focusOnMount ? [''] : []); From 39a58a2cc3deef076ce5aa3f916274e85606300d Mon Sep 17 00:00:00 2001 From: Soroush Date: Fri, 12 Mar 2021 11:21:24 -0800 Subject: [PATCH 3/3] pr comment --- .../packages/lib/code-editor/src/lg/hooks/useStringArray.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Composer/packages/lib/code-editor/src/lg/hooks/useStringArray.ts b/Composer/packages/lib/code-editor/src/lg/hooks/useStringArray.ts index 3b22473592..f7c3e2f063 100644 --- a/Composer/packages/lib/code-editor/src/lg/hooks/useStringArray.ts +++ b/Composer/packages/lib/code-editor/src/lg/hooks/useStringArray.ts @@ -23,10 +23,10 @@ const getInitialItems = ( // eslint-disable-next-line security/detect-unsafe-regex ?.split(/(? s !== '' && s !== '\n') - ?.map((s) => s.replace(/\r?\n$/g, '')) + .filter((s) => s !== '' && s !== '\n') + .map((s) => s.replace(/\r?\n$/g, '')) // Remove LG template multiline block symbol - ?.map((s) => s.replace(/```/g, '')) || [] + .map((s) => s.replace(/```/g, '')) || [] : response?.value || (focusOnMount ? [''] : []); };