From 833f786278d1c2bb8e163406f601f82c7e07e813 Mon Sep 17 00:00:00 2001 From: Soroush Date: Mon, 15 Mar 2021 09:41:48 -0700 Subject: [PATCH] fix: Allow multiline variations for LG text and speech modalities (#6394) * fix: Allow multiline variations for LG text and speech modalities * fix tests * pr comment Co-authored-by: Soroush --- .../lg/__tests__/TextModalityEditors.test.tsx | 2 ++ .../src/lg/hooks/useStringArray.ts | 35 ++++++++++++++++--- .../lg/modalityEditors/StringArrayEditor.tsx | 23 ++++++++---- .../lg/modalityEditors/StringArrayItem.tsx | 15 ++------ 4 files changed, 51 insertions(+), 24 deletions(-) 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( ( response: T, lgTemplates?: readonly LgTemplate[], @@ -16,10 +18,30 @@ 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 + // 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 ? [''] : []); }; +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 +67,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 10c886de61..f57da6c9f1 100644 --- a/Composer/packages/lib/code-editor/src/lg/modalityEditors/StringArrayItem.tsx +++ b/Composer/packages/lib/code-editor/src/lg/modalityEditors/StringArrayItem.tsx @@ -158,7 +158,7 @@ const TextViewItem = React.memo( onFocus={focus} > - {onRenderDisplayText?.() ?? value} + {onRenderDisplayText?.() ?? value.replace(/\r?\n/g, '↵')} @@ -197,18 +197,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 (