diff --git a/CHANGELOG.md b/CHANGELOG.md index d1c1cecf8f..df0ec62747 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Fixes [#2822](https://github.com/microsoft/BotFramework-WebChat/issues/2822). Fixed `credentials` should return `authorizationToken` and `subscriptionKey` as string and allow empty LUIS reference grammar ID, by [@compulim](https://github.com/compulim) in PR [#2824](https://github.com/microsoft/BotFramework-WebChat/pull/2824) - Fixes [#2751](https://github.com/microsoft/BotFramework-WebChat/issues/2751). Move documentation to docs folder, by [@corinagum](https://github.com/corinagum) in PR [#2832](https://github.com/microsoft/BotFramework-WebChat/pull/2832) - Fixes [#2838](https://github.com/microsoft/BotFramework-WebChat/issues/2838). Fixed `concatMiddleware` should allow any middleware to call its downstream middleware twice, by [@compulim](https://github.com/compulim) in PR [#2839](https://github.com/microsoft/BotFramework-WebChat/pull/2839) +- Fixes [#2864](https://github.com/microsoft/BotFramework-WebChat/issues/2864). Replaced `DownloadAttachment` and `UploadAttachment` with `FileAttachment`, which shows the download link and icon if the attachment contains the `contentUrl`, by [@compulim](https://github.com/compulim) in PR [#2868](https://github.com/microsoft/BotFramework-WebChat/pull/2868) ### Changed diff --git a/__tests__/__image_snapshots__/chrome-docker/file-attachment-js-show-zip-files-with-content-url-1-snap.png b/__tests__/__image_snapshots__/chrome-docker/file-attachment-js-show-zip-files-with-content-url-1-snap.png new file mode 100644 index 0000000000..c0a0e79f6b Binary files /dev/null and b/__tests__/__image_snapshots__/chrome-docker/file-attachment-js-show-zip-files-with-content-url-1-snap.png differ diff --git a/__tests__/__image_snapshots__/chrome-docker/file-attachment-js-show-zip-files-without-content-url-1-snap.png b/__tests__/__image_snapshots__/chrome-docker/file-attachment-js-show-zip-files-without-content-url-1-snap.png new file mode 100644 index 0000000000..4061132696 Binary files /dev/null and b/__tests__/__image_snapshots__/chrome-docker/file-attachment-js-show-zip-files-without-content-url-1-snap.png differ diff --git a/__tests__/attachmentMiddleware.js b/__tests__/attachmentMiddleware.js index ba08ca39dd..1821f2b20f 100644 --- a/__tests__/attachmentMiddleware.js +++ b/__tests__/attachmentMiddleware.js @@ -45,7 +45,9 @@ test('file upload should show thumbnail and file name', async () => { return next({ activity, attachment }); } - } + }, + // TODO: [P3] Offline bot did not reply with a downloadable attachment, we need to use production bot + useProductionBot: true }); await driver.wait(uiConnected(), timeouts.directLine); diff --git a/__tests__/fileAttachment.js b/__tests__/fileAttachment.js new file mode 100644 index 0000000000..feb080ced9 --- /dev/null +++ b/__tests__/fileAttachment.js @@ -0,0 +1,114 @@ +import { imageSnapshotOptions, timeouts } from './constants.json'; + +import allImagesLoaded from './setup/conditions/allImagesLoaded'; +import minNumActivitiesShown from './setup/conditions/minNumActivitiesShown'; +import uiConnected from './setup/conditions/uiConnected'; + +// selenium-webdriver API doc: +// https://seleniumhq.github.io/selenium/docs/api/javascript/module/selenium-webdriver/index_exports_WebDriver.html + +jest.setTimeout(timeouts.test); + +test('show ZIP files with contentUrl', async () => { + const { driver, pageObjects } = await setupWebDriver({ + createDirectLine: options => { + const directLine = window.WebChat.createDirectLine(options); + const patchedDirectLine = { + activity$: new Observable(observer => { + const subscription = directLine.activity$.subscribe({ + next(activity) { + observer.next( + Object.assign({}, activity, { + attachments: (activity.attachments || []).map(attachment => + Object.assign({}, attachment, { + contentUrl: 'https://example.org/' + }) + ) + }) + ); + } + }); + + return () => subscription.unsubscribe(); + }), + + connectionStatus$: directLine.connectionStatus$, + postActivity: directLine.postActivity.bind(directLine), + token: directLine.token + }; + + return patchedDirectLine; + } + }); + + await driver.wait(uiConnected(), timeouts.directLine); + + await pageObjects.sendFile('empty.zip'); + await driver.wait(minNumActivitiesShown(2), timeouts.directLine); + await driver.wait(allImagesLoaded(), timeouts.fetchImage); + + const base64PNG = await driver.takeScreenshot(); + + expect(base64PNG).toMatchImageSnapshot(imageSnapshotOptions); + + await expect( + driver.executeScript(() => + document.querySelector('[role="listitem"]:nth-child(1) a[target="_blank"]').getAttribute('href') + ) + ).resolves.toEqual('https://example.org/'); + await expect( + driver.executeScript(() => + document.querySelector('[role="listitem"]:nth-child(2) a[target="_blank"]').getAttribute('href') + ) + ).resolves.toEqual('https://example.org/'); +}); + +test('show ZIP files without contentUrl', async () => { + const { driver, pageObjects } = await setupWebDriver({ + createDirectLine: options => { + const directLine = window.WebChat.createDirectLine(options); + const patchedDirectLine = { + activity$: new Observable(observer => { + const subscription = directLine.activity$.subscribe({ + next(activity) { + observer.next( + Object.assign({}, activity, { + attachments: (activity.attachments || []).map(attachment => + Object.assign({}, attachment, { + contentUrl: undefined + }) + ) + }) + ); + } + }); + + return () => subscription.unsubscribe(); + }), + + connectionStatus$: directLine.connectionStatus$, + postActivity: directLine.postActivity.bind(directLine), + token: directLine.token + }; + + return patchedDirectLine; + } + }); + + await driver.wait(uiConnected(), timeouts.directLine); + + await pageObjects.sendFile('empty.zip'); + await driver.wait(minNumActivitiesShown(2), timeouts.directLine); + await driver.wait(allImagesLoaded(), timeouts.fetchImage); + + const base64PNG = await driver.takeScreenshot(); + + expect(base64PNG).toMatchImageSnapshot(imageSnapshotOptions); + + await expect( + driver.executeScript(() => !!document.querySelector('[role="listitem"]:nth-child(1) a')) + ).resolves.toBeFalsy(); + await expect( + driver.executeScript(() => !!document.querySelector('[role="listitem"]:nth-child(2) a')) + ).resolves.toBeFalsy(); +}); diff --git a/__tests__/hooks/useSendFiles.js b/__tests__/hooks/useSendFiles.js index 8784140635..52863f1c68 100644 --- a/__tests__/hooks/useSendFiles.js +++ b/__tests__/hooks/useSendFiles.js @@ -1,5 +1,6 @@ import { imageSnapshotOptions, timeouts } from '../constants.json'; +import allOutgoingActivitiesSent from '../setup/conditions/allOutgoingActivitiesSent'; import minNumActivitiesShown from '../setup/conditions/minNumActivitiesShown'; import uiConnected from '../setup/conditions/uiConnected'; @@ -9,7 +10,10 @@ import uiConnected from '../setup/conditions/uiConnected'; jest.setTimeout(timeouts.test); test('calling sendFile should send files', async () => { - const { driver, pageObjects } = await setupWebDriver(); + const { driver, pageObjects } = await setupWebDriver({ + // TODO: [P3] Offline bot did not reply with a downloadable attachment, we need to use production bot + useProductionBot: true + }); await driver.wait(uiConnected(), timeouts.directLine); @@ -24,6 +28,7 @@ test('calling sendFile should send files', async () => { }); await driver.wait(minNumActivitiesShown(2), timeouts.directLine); + await driver.wait(allOutgoingActivitiesSent(), timeouts.directLine); const base64PNG = await driver.takeScreenshot(); diff --git a/__tests__/hooks/useSendPostBack.js b/__tests__/hooks/useSendPostBack.js index 68908b0fb9..fee6eedc42 100644 --- a/__tests__/hooks/useSendPostBack.js +++ b/__tests__/hooks/useSendPostBack.js @@ -15,7 +15,7 @@ test('calling sendPostBack should send a post back activity', async () => { await pageObjects.runHook('useSendPostBack', [], sendPostBack => sendPostBack({ hello: 'World!' })); - await driver.wait(minNumActivitiesShown(2), timeouts.directLine); + await driver.wait(minNumActivitiesShown(1), timeouts.directLine); const base64PNG = await driver.takeScreenshot(); diff --git a/__tests__/language.js b/__tests__/language.js index 63504410ed..de223f9f67 100644 --- a/__tests__/language.js +++ b/__tests__/language.js @@ -14,7 +14,7 @@ test('setting language to empty string should not crash', async () => { await driver.wait(uiConnected(), timeouts.directLine); await pageObjects.sendMessageViaSendBox('echo Hello', { waitForSend: true }); - await driver.wait(minNumActivitiesShown(2), timeouts.directLine); + await driver.wait(minNumActivitiesShown(3), timeouts.directLine); const base64PNG = await driver.takeScreenshot(); diff --git a/__tests__/markdown.js b/__tests__/markdown.js index 4a22c5277a..95fa1ddd31 100644 --- a/__tests__/markdown.js +++ b/__tests__/markdown.js @@ -29,7 +29,7 @@ test('null renderMarkdown function', async () => { await pageObjects.sendMessageViaSendBox('echo **This text should be plain text**', { waitForSend: true }); await driver.wait(allImagesLoaded(), timeouts.fetchImage); - await driver.wait(minNumActivitiesShown(2), timeouts.directLine); + await driver.wait(minNumActivitiesShown(3), timeouts.directLine); const base64PNG = await driver.takeScreenshot(); diff --git a/__tests__/sendBox.js b/__tests__/sendBox.js index d8117558a8..b3485ec201 100644 --- a/__tests__/sendBox.js +++ b/__tests__/sendBox.js @@ -26,7 +26,7 @@ test('should focus send box when message is being sent', async () => { await driver.wait(uiConnected(), timeouts.directLine); await pageObjects.sendMessageViaSendBox('Hello, World!', { waitForSend: true }); - await driver.wait(minNumActivitiesShown(1), timeouts.directLine); + await driver.wait(minNumActivitiesShown(2), timeouts.directLine); const base64PNG = await driver.takeScreenshot(); diff --git a/__tests__/sendTypingIndicator.js b/__tests__/sendTypingIndicator.js index a7b5c43305..2870fe8b96 100644 --- a/__tests__/sendTypingIndicator.js +++ b/__tests__/sendTypingIndicator.js @@ -67,7 +67,7 @@ test('typing indicator should not display after second activity', async () => { }); await pageObjects.sendMessageViaSendBox('typing', { waitForSend: true }); - await driver.wait(minNumActivitiesShown(2), timeouts.directLine); + await driver.wait(minNumActivitiesShown(3), timeouts.directLine); const base64PNG = await driver.takeScreenshot(); expect(base64PNG).toMatchImageSnapshot(imageSnapshotOptions); diff --git a/__tests__/setup/conditions/minNumActivitiesShown.js b/__tests__/setup/conditions/minNumActivitiesShown.js index f708cfb7e5..bd26f3f15b 100644 --- a/__tests__/setup/conditions/minNumActivitiesShown.js +++ b/__tests__/setup/conditions/minNumActivitiesShown.js @@ -1,5 +1,19 @@ -import { By, until } from 'selenium-webdriver'; +import { Condition } from 'selenium-webdriver'; export default function minNumActivitiesShown(numActivities) { - return until.elementLocated(By.css(`[role="listitem"]:nth-child(${numActivities})`)); + return new Condition(`${numActivities} activities is shown`, async driver => { + // To run hooks (WebChatTest.runHook), the code internally create an activity. + // Inside the activity renderer, it call hooks, but return empty visually. + // This activity is invisible and should not count towards "minNumActivitiesShown". + + const numActivitiesShown = await driver.executeScript(() => + [].reduce.call( + document.querySelectorAll('[role="listitem"]'), + (numActivitiesShown, child) => numActivitiesShown + (child.children.length ? 1 : 0), + 0 + ) + ); + + return numActivitiesShown >= numActivities; + }); } diff --git a/__tests__/speech.selectVoice.js b/__tests__/speech.selectVoice.js index 92b0ac2d40..31e7e5a1f2 100644 --- a/__tests__/speech.selectVoice.js +++ b/__tests__/speech.selectVoice.js @@ -20,7 +20,7 @@ describe('selecting voice based on language', () => { await pageObjects.sendMessageViaMicrophone('echo 123'); - await driver.wait(minNumActivitiesShown(2), timeouts.directLine); + await driver.wait(minNumActivitiesShown(3), timeouts.directLine); await driver.wait(speechSynthesisUtterancePended(), timeouts.ui); await expect(pageObjects.startSpeechSynthesize()).resolves.toHaveProperty('voice', { @@ -42,7 +42,7 @@ describe('selecting voice based on language', () => { await pageObjects.sendMessageViaMicrophone('echo 123'); - await driver.wait(minNumActivitiesShown(2), timeouts.directLine); + await driver.wait(minNumActivitiesShown(3), timeouts.directLine); await driver.wait(speechSynthesisUtterancePended(), timeouts.ui); await expect(pageObjects.startSpeechSynthesize()).resolves.toHaveProperty('voice', { @@ -66,7 +66,7 @@ describe('selecting voice based on language', () => { await pageObjects.sendMessageViaMicrophone('echo 123'); - await driver.wait(minNumActivitiesShown(2), timeouts.directLine); + await driver.wait(minNumActivitiesShown(3), timeouts.directLine); await driver.wait(speechSynthesisUtterancePended(), timeouts.ui); await expect(pageObjects.startSpeechSynthesize()).resolves.toHaveProperty('voice', { diff --git a/__tests__/speech.synthesis.js b/__tests__/speech.synthesis.js index 52fbf582a3..40cbf82776 100644 --- a/__tests__/speech.synthesis.js +++ b/__tests__/speech.synthesis.js @@ -110,7 +110,7 @@ describe('speech synthesis', () => { await pageObjects.sendMessageViaMicrophone('echo Hello, World!'); await expect(speechRecognitionStartCalled().fn(driver)).resolves.toBeFalsy(); - await driver.wait(minNumActivitiesShown(2), timeouts.directLine); + await driver.wait(minNumActivitiesShown(3), timeouts.directLine); await expect(pageObjects.startSpeechSynthesize()).resolves.toHaveProperty( 'text', @@ -140,7 +140,7 @@ describe('speech synthesis', () => { } }); - await pageObjects.sendMessageViaMicrophone('input hint expected'); + await pageObjects.sendMessageViaMicrophone('hint expected'); await driver.wait(minNumActivitiesShown(2), timeouts.directLine); diff --git a/__tests__/styleOptions.js b/__tests__/styleOptions.js index 74ef198523..cd3c985a0a 100644 --- a/__tests__/styleOptions.js +++ b/__tests__/styleOptions.js @@ -35,7 +35,7 @@ describe('style options', () => { waitForSend: true }); - await driver.wait(minNumActivitiesShown(2), timeouts.directLine); + await driver.wait(minNumActivitiesShown(3), timeouts.directLine); expect(await driver.takeScreenshot()).toMatchImageSnapshot(imageSnapshotOptions); }); diff --git a/__tests__/suggestedActions.js b/__tests__/suggestedActions.js index 1c04c9a7b7..6fd20b402a 100644 --- a/__tests__/suggestedActions.js +++ b/__tests__/suggestedActions.js @@ -57,7 +57,7 @@ describe('suggested-actions command', () => { const imBackButton = buttons[1]; await imBackButton.click(); - await driver.wait(minNumActivitiesShown(3), timeouts.directLine); + await driver.wait(minNumActivitiesShown(4), timeouts.directLine); await driver.wait(allOutgoingActivitiesSent(), timeouts.directLine); const base64PNG = await driver.takeScreenshot(); @@ -120,7 +120,7 @@ describe('suggested-actions command', () => { const postBackStringButton = buttons[4]; await postBackStringButton.click(); - await driver.wait(minNumActivitiesShown(3), timeouts.directLine); + await driver.wait(minNumActivitiesShown(4), timeouts.directLine); await driver.wait(allOutgoingActivitiesSent(), timeouts.directLine); const base64PNG = await driver.takeScreenshot(); diff --git a/__tests__/timestamp.js b/__tests__/timestamp.js index 5bbb0ccc64..802a0e3ee1 100644 --- a/__tests__/timestamp.js +++ b/__tests__/timestamp.js @@ -24,7 +24,7 @@ test('update timestamp language on-the-fly', async () => { await driver.wait(uiConnected(), timeouts.directLine); await pageObjects.sendMessageViaSendBox('echo Hello, World!', { waitForSend: true }); - await driver.wait(minNumActivitiesShown(2), timeouts.directLine); + await driver.wait(minNumActivitiesShown(3), timeouts.directLine); expect(await driver.takeScreenshot()).toMatchImageSnapshot(imageSnapshotOptions); @@ -83,7 +83,7 @@ test('prepend text', async () => { await driver.wait(uiConnected(), timeouts.directLine); await pageObjects.sendMessageViaSendBox('echo Hello, World!', { waitForSend: true }); - await driver.wait(minNumActivitiesShown(2), timeouts.directLine); + await driver.wait(minNumActivitiesShown(3), timeouts.directLine); expect(await driver.takeScreenshot()).toMatchImageSnapshot(imageSnapshotOptions); }); @@ -94,7 +94,7 @@ test('change timestamp grouping on-the-fly', async () => { await driver.wait(uiConnected(), timeouts.directLine); await pageObjects.sendMessageViaSendBox('echo Hello, World!', { waitForSend: true }); - await driver.wait(minNumActivitiesShown(2), timeouts.directLine); + await driver.wait(minNumActivitiesShown(3), timeouts.directLine); expect(await driver.takeScreenshot()).toMatchImageSnapshot(imageSnapshotOptions); diff --git a/__tests__/updateActivity.js b/__tests__/updateActivity.js index c0c511eac2..78ca851aec 100644 --- a/__tests__/updateActivity.js +++ b/__tests__/updateActivity.js @@ -78,12 +78,12 @@ test('should not replace activity without activity ID', async () => { await pageObjects.sendMessageViaSendBox('echo This message will not be replaced by a carousel.', { waitForSend: true }); - await driver.wait(minNumActivitiesShown(2), timeouts.directLine); + await driver.wait(minNumActivitiesShown(3), timeouts.directLine); await expect(driver.takeScreenshot()).resolves.toMatchImageSnapshot(imageSnapshotOptions); await pageObjects.sendMessageViaSendBox('carousel', { waitForSend: true }); - await driver.wait(minNumActivitiesShown(3), timeouts.directLine); + await driver.wait(minNumActivitiesShown(5), timeouts.directLine); await driver.wait(allImagesLoaded(), timeouts.fetchImage); await driver.wait(scrollToBottomCompleted(), timeouts.scrollToBottom); diff --git a/__tests__/upload.js b/__tests__/upload.js index 7c7a589c34..30f4c6942a 100644 --- a/__tests__/upload.js +++ b/__tests__/upload.js @@ -11,7 +11,10 @@ jest.setTimeout(timeouts.test); describe('upload a picture', () => { test('', async () => { - const { driver, pageObjects } = await setupWebDriver(); + const { driver, pageObjects } = await setupWebDriver({ + // TODO: [P3] Offline bot did not reply with a downloadable attachment, we need to use production bot + useProductionBot: true + }); await driver.wait(uiConnected(), timeouts.directLine); @@ -32,7 +35,9 @@ describe('upload a picture', () => { uploadThumbnailHeight: 60, uploadThumbnailWidth: 120 } - } + }, + // TODO: [P3] Offline bot did not reply with a downloadable attachment, we need to use production bot + useProductionBot: true }); await driver.wait(uiConnected(), timeouts.directLine); @@ -52,7 +57,9 @@ describe('upload a picture', () => { styleOptions: { uploadThumbnailQuality: 0.1 } - } + }, + // TODO: [P3] Offline bot did not reply with a downloadable attachment, we need to use production bot + useProductionBot: true }); await driver.wait(uiConnected(), timeouts.directLine); @@ -72,7 +79,9 @@ describe('upload a picture', () => { styleOptions: { enableUploadThumbnail: false } - } + }, + // TODO: [P3] Offline bot did not reply with a downloadable attachment, we need to use production bot + useProductionBot: true }); await driver.wait(uiConnected(), timeouts.directLine); @@ -88,7 +97,10 @@ describe('upload a picture', () => { describe('without Web Worker', () => { test('', async () => { - const { driver, pageObjects } = await setupWebDriver(); + const { driver, pageObjects } = await setupWebDriver({ + // TODO: [P3] Offline bot did not reply with a downloadable attachment, we need to use production bot + useProductionBot: true + }); await driver.executeScript(() => { window.Worker = undefined; @@ -112,7 +124,9 @@ describe('upload a picture', () => { uploadThumbnailHeight: 60, uploadThumbnailWidth: 120 } - } + }, + // TODO: [P3] Offline bot did not reply with a downloadable attachment, we need to use production bot + useProductionBot: true }); await driver.executeScript(() => { @@ -135,7 +149,9 @@ describe('upload a picture', () => { styleOptions: { uploadThumbnailQuality: 0.1 } - } + }, + // TODO: [P3] Offline bot did not reply with a downloadable attachment, we need to use production bot + useProductionBot: true }); await driver.executeScript(() => { @@ -155,7 +171,10 @@ describe('upload a picture', () => { }); test('upload a ZIP file', async () => { - const { driver, pageObjects } = await setupWebDriver(); + const { driver, pageObjects } = await setupWebDriver({ + // TODO: [P3] Offline bot did not reply with a downloadable attachment, we need to use production bot + useProductionBot: true + }); await driver.wait(uiConnected(), timeouts.directLine); diff --git a/packages/component/src/Attachment/DownloadAttachment.js b/packages/component/src/Attachment/DownloadAttachment.js deleted file mode 100644 index 737f8a9547..0000000000 --- a/packages/component/src/Attachment/DownloadAttachment.js +++ /dev/null @@ -1,58 +0,0 @@ -import { format } from 'bytes'; -import PropTypes from 'prop-types'; -import React from 'react'; - -import DownloadIcon from './Assets/DownloadIcon'; -import ScreenReaderText from '../ScreenReaderText'; -import useLocalize from '../hooks/useLocalize'; -import useStyleSet from '../hooks/useStyleSet'; - -const DownloadAttachment = ({ - activity: { attachments = [], channelData: { attachmentSizes = [] } = {} } = {}, - attachment -}) => { - const [{ downloadAttachment: downloadAttachmentStyleSet }] = useStyleSet(); - const downloadLabel = useLocalize('Download file'); - - const attachmentIndex = attachments.indexOf(attachment); - const size = attachmentSizes[attachmentIndex]; - const formattedSize = typeof size === 'number' && format(size); - - const downloadFileWithFileSizeLabel = useLocalize( - 'DownloadFileWithFileSize', - downloadLabel, - attachment.name, - formattedSize - ); - - return ( - - -
- - {/* Although nested, Chrome v75 does not respect the above aria-hidden and makes the below aria-hidden necessary */} -
-
{attachment.name}
-
{formattedSize}
-
- -
-
-
- ); -}; - -DownloadAttachment.propTypes = { - activity: PropTypes.shape({ - attachment: PropTypes.array, - channelData: PropTypes.shape({ - attachmentSizes: PropTypes.arrayOf(PropTypes.number) - }) - }).isRequired, - attachment: PropTypes.shape({ - contentUrl: PropTypes.string.isRequired, - name: PropTypes.string.isRequired - }).isRequired -}; - -export default DownloadAttachment; diff --git a/packages/component/src/Attachment/FileAttachment.js b/packages/component/src/Attachment/FileAttachment.js new file mode 100644 index 0000000000..7fa45cadda --- /dev/null +++ b/packages/component/src/Attachment/FileAttachment.js @@ -0,0 +1,29 @@ +import PropTypes from 'prop-types'; +import React from 'react'; + +import FileContent from './FileContent'; + +const FileAttachment = ({ + activity: { attachments = [], channelData: { attachmentSizes = [] } = {} } = {}, + attachment +}) => { + const attachmentIndex = attachments.indexOf(attachment); + const size = attachmentSizes[attachmentIndex]; + + return ; +}; + +FileAttachment.propTypes = { + activity: PropTypes.shape({ + attachment: PropTypes.array, + channelData: PropTypes.shape({ + attachmentSizes: PropTypes.arrayOf(PropTypes.number) + }) + }).isRequired, + attachment: PropTypes.shape({ + contentUrl: PropTypes.string, + name: PropTypes.string.isRequired + }).isRequired +}; + +export default FileAttachment; diff --git a/packages/component/src/Attachment/FileContent.js b/packages/component/src/Attachment/FileContent.js new file mode 100644 index 0000000000..bfcaae7f3e --- /dev/null +++ b/packages/component/src/Attachment/FileContent.js @@ -0,0 +1,101 @@ +import { css } from 'glamor'; +import { format } from 'bytes'; +import classNames from 'classnames'; +import PropTypes from 'prop-types'; +import React from 'react'; + +import DownloadIcon from './Assets/DownloadIcon'; +import ScreenReaderText from '../ScreenReaderText'; +import useLocalize from '../hooks/useLocalize'; +import useStyleSet from '../hooks/useStyleSet'; + +const ROOT_CSS = css({ + display: 'flex', + + '& .webchat__fileContent__buttonLink': { + display: 'flex', + flex: 1 + }, + + '& .webchat__fileContent__badge': { + display: 'flex', + flex: 1, + flexDirection: 'column' + } +}); + +const FileContentBadge = ({ downloadIcon, fileName, size }) => { + const formattedSize = typeof size === 'number' && format(size); + + return ( + +
+
{fileName}
+ {!!formattedSize &&
{formattedSize}
} +
+ {downloadIcon && } +
+ ); +}; + +FileContentBadge.defaultProps = { + downloadIcon: false, + size: undefined +}; + +FileContentBadge.propTypes = { + downloadIcon: PropTypes.bool, + fileName: PropTypes.string.isRequired, + size: PropTypes.number +}; + +const FileContent = ({ className, href, fileName, size }) => { + const formattedSize = format(size); + + const [{ fileContent: fileContentStyleSet }] = useStyleSet(); + + const downloadLabel = useLocalize('Download file'); + const uploadLabel = useLocalize('Upload file'); + const downloadFileWithFileSizeLabel = useLocalize('DownloadFileWithFileSize', downloadLabel, fileName, formattedSize); + const uploadFileWithFileSizeLabel = useLocalize('UploadFileWithFileSize', uploadLabel, fileName, formattedSize); + + const alt = href ? downloadFileWithFileSizeLabel : uploadFileWithFileSizeLabel; + + return ( +
+ + {href ? ( + + {/* Although nested, Chrome v75 does not respect the above aria-hidden and makes the below aria-hidden in FileContentBadge necessary */} + + + ) : ( + + )} +
+ ); +}; + +FileContent.defaultProps = { + className: '', + href: undefined, + size: undefined +}; + +FileContent.propTypes = { + className: PropTypes.string, + fileName: PropTypes.string.isRequired, + href: PropTypes.string, + size: PropTypes.number +}; + +export default FileContent; diff --git a/packages/component/src/Attachment/ImageAttachment.js b/packages/component/src/Attachment/ImageAttachment.js index eadd2fc708..99598f666f 100644 --- a/packages/component/src/Attachment/ImageAttachment.js +++ b/packages/component/src/Attachment/ImageAttachment.js @@ -11,11 +11,20 @@ ImageAttachment.propTypes = { activity: PropTypes.shape({ attachments: PropTypes.array.isRequired }).isRequired, - attachment: PropTypes.shape({ - contentUrl: PropTypes.string.isRequired, - name: PropTypes.string, - thumbnailUrl: PropTypes.string - }).isRequired + + // Either attachment.contentUrl or attachment.thumbnailUrl must be specified. + attachment: PropTypes.oneOfType([ + PropTypes.shape({ + contentUrl: PropTypes.string.isRequired, + name: PropTypes.string, + thumbnailUrl: PropTypes.string + }), + PropTypes.shape({ + contentUrl: PropTypes.string, + name: PropTypes.string, + thumbnailUrl: PropTypes.string.isRequired + }) + ]).isRequired }; export default ImageAttachment; diff --git a/packages/component/src/Attachment/UploadAttachment.js b/packages/component/src/Attachment/UploadAttachment.js deleted file mode 100644 index 850e8f38b6..0000000000 --- a/packages/component/src/Attachment/UploadAttachment.js +++ /dev/null @@ -1,56 +0,0 @@ -import { css } from 'glamor'; -import { format } from 'bytes'; -import classNames from 'classnames'; -import PropTypes from 'prop-types'; -import React from 'react'; - -import ScreenReaderText from '../ScreenReaderText'; -import useLocalize from '../hooks/useLocalize'; -import useStyleSet from '../hooks/useStyleSet'; - -const ROOT_CSS = css({ - display: 'flex', - flexDirection: 'column' -}); - -const UploadAttachment = ({ - activity: { attachments = [], channelData: { attachmentSizes = [] } = {} } = {}, - attachment -}) => { - const [{ uploadAttachment: uploadAttachmentStyleSet }] = useStyleSet(); - - const attachmentIndex = attachments.indexOf(attachment); - const uploadLabel = useLocalize('Upload file'); - const size = attachmentSizes[attachmentIndex]; - const formattedSize = typeof size === 'number' && format(size); - const uploadFileWithFileSizeLabel = useLocalize( - 'UploadFileWithFileSize', - uploadLabel, - attachment.name, - formattedSize - ); - - return ( - - -
-
{attachment.name}
-
{formattedSize}
-
-
- ); -}; - -UploadAttachment.propTypes = { - activity: PropTypes.shape({ - attachment: PropTypes.array, - channelData: PropTypes.shape({ - attachmentSizes: PropTypes.arrayOf(PropTypes.number) - }) - }).isRequired, - attachment: PropTypes.shape({ - name: PropTypes.string.isRequired - }).isRequired -}; - -export default UploadAttachment; diff --git a/packages/component/src/Localization/en-US.js b/packages/component/src/Localization/en-US.js index 5c0ddf883a..325668ab8e 100644 --- a/packages/component/src/Localization/en-US.js +++ b/packages/component/src/Localization/en-US.js @@ -92,7 +92,7 @@ export default { Total: 'Total', 'Type your message': 'Type your message', TypingIndicator: 'Showing typing indicator', - 'Upload file': 'Upload file', + 'Upload file': 'File', UploadFileWithFileSize: uploadFileWithFileSize, UserSent: 'User sent: ', VAT: 'VAT' diff --git a/packages/component/src/Middleware/Attachment/createCoreMiddleware.js b/packages/component/src/Middleware/Attachment/createCoreMiddleware.js index bdca8a7fe2..29bb401ee1 100644 --- a/packages/component/src/Middleware/Attachment/createCoreMiddleware.js +++ b/packages/component/src/Middleware/Attachment/createCoreMiddleware.js @@ -2,9 +2,8 @@ import PropTypes from 'prop-types'; import React from 'react'; import AudioAttachment from '../../Attachment/AudioAttachment'; -import DownloadAttachment from '../../Attachment/DownloadAttachment'; +import FileAttachment from '../../Attachment/FileAttachment'; import ImageAttachment from '../../Attachment/ImageAttachment'; -import UploadAttachment from '../../Attachment/UploadAttachment'; import TextAttachment from '../../Attachment/TextAttachment'; import VideoAttachment from '../../Attachment/VideoAttachment'; @@ -18,7 +17,7 @@ export default function createCoreMiddleware() { attachment: { contentType, contentUrl, thumbnailUrl } = {} }) => role === 'user' && !/^text\//u.test(contentType) && !thumbnailUrl ? ( - + ) : /^audio\//u.test(contentType) ? ( ) : /^image\//u.test(contentType) ? ( @@ -26,7 +25,7 @@ export default function createCoreMiddleware() { ) : /^video\//u.test(contentType) ? ( ) : contentUrl || contentType === 'application/octet-stream' ? ( - + ) : /^text\//u.test(contentType) ? ( ) : ( @@ -37,7 +36,8 @@ export default function createCoreMiddleware() { activity: PropTypes.any.isRequired, attachment: PropTypes.shape({ contentType: PropTypes.string.isRequired, - contentUrl: PropTypes.string.isRequired + contentUrl: PropTypes.string, + thumbnailUrl: PropTypes.string }).isRequired }; diff --git a/packages/component/src/Styles/StyleSet/DownloadAttachment.js b/packages/component/src/Styles/StyleSet/DownloadAttachment.js deleted file mode 100644 index bda90fb2a9..0000000000 --- a/packages/component/src/Styles/StyleSet/DownloadAttachment.js +++ /dev/null @@ -1,32 +0,0 @@ -export default function createDownloadAttachmentStyle({ accent, bubbleTextColor, paddingRegular, primaryFont }) { - return { - fontFamily: primaryFont, - - '& > a': { - alignItems: 'center', - color: bubbleTextColor, - // TODO: [P2] We should not set "display" in styleSet, this will allow the user to break the layout for no good reasons. - display: 'flex', - padding: paddingRegular, - textDecoration: 'none', - - '&:focus': { - backgroundColor: 'rgba(0, 0, 0, .1)' - }, - - '& > .icon': { - fill: accent, - marginLeft: paddingRegular, - padding: paddingRegular - }, - - '& > .details': { - flex: 1, - - '& > .name': { - color: accent - } - } - } - }; -} diff --git a/packages/component/src/Styles/StyleSet/FileContent.js b/packages/component/src/Styles/StyleSet/FileContent.js new file mode 100644 index 0000000000..fc71851c58 --- /dev/null +++ b/packages/component/src/Styles/StyleSet/FileContent.js @@ -0,0 +1,32 @@ +export default function createFileContentStyle({ accent, bubbleTextColor, paddingRegular, primaryFont }) { + return { + color: bubbleTextColor, + display: 'flex', + fontFamily: primaryFont, + padding: paddingRegular, + + '& .webchat__fileContent__badge': { + justifyContent: 'center' + }, + + '& .webchat__fileContent__buttonLink': { + alignItems: 'center', + color: bubbleTextColor, + textDecoration: 'none', + + '&:focus': { + backgroundColor: 'rgba(0, 0, 0, .1)' + } + }, + + '& .webchat__fileContent__downloadIcon': { + fill: accent, + marginLeft: paddingRegular, + padding: paddingRegular + }, + + '& .webchat__fileContent__fileName': { + color: accent + } + }; +} diff --git a/packages/component/src/Styles/StyleSet/UploadAttachment.js b/packages/component/src/Styles/StyleSet/UploadAttachment.js deleted file mode 100644 index e828eb72e7..0000000000 --- a/packages/component/src/Styles/StyleSet/UploadAttachment.js +++ /dev/null @@ -1,12 +0,0 @@ -export default function createUploadAttachmentStyle({ accent, bubbleTextColor, paddingRegular, primaryFont }) { - return { - color: bubbleTextColor, - fontFamily: primaryFont, - padding: paddingRegular, - textDecoration: 'none', - - '& > .name': { - color: accent - } - }; -} diff --git a/packages/component/src/Styles/createStyleSet.js b/packages/component/src/Styles/createStyleSet.js index 4f8aeebe52..f86b2f0a53 100644 --- a/packages/component/src/Styles/createStyleSet.js +++ b/packages/component/src/Styles/createStyleSet.js @@ -9,9 +9,9 @@ import createCarouselFilmStrip from './StyleSet/CarouselFilmStrip'; import createCarouselFlipper from './StyleSet/CarouselFlipper'; import createConnectivityNotification from './StyleSet/ConnectivityNotification'; import createDictationInterimsStyle from './StyleSet/DictationInterims'; -import createDownloadAttachmentStyle from './StyleSet/DownloadAttachment'; import createErrorBoxStyle from './StyleSet/ErrorBox'; import createErrorNotificationStyle from './StyleSet/ErrorNotification'; +import createFileContentStyle from './StyleSet/FileContent'; import createMicrophoneButtonStyle from './StyleSet/MicrophoneButton'; import createRootStyle from './StyleSet/Root'; import createScrollToEndButtonStyle from './StyleSet/ScrollToEndButton'; @@ -29,7 +29,6 @@ import createSuggestedActionStyle from './StyleSet/SuggestedAction'; import createTextContentStyle from './StyleSet/TextContent'; import createTypingAnimationStyle from './StyleSet/TypingAnimation'; import createTypingIndicatorStyle from './StyleSet/TypingIndicator'; -import createUploadAttachmentStyle from './StyleSet/UploadAttachment'; import createUploadButtonStyle from './StyleSet/UploadButton'; import createVideoAttachmentStyle from './StyleSet/VideoAttachment'; import createVideoContentStyle from './StyleSet/VideoContent'; @@ -179,9 +178,9 @@ export default function createStyleSet(options) { carouselFlipper: createCarouselFlipper(options), connectivityNotification: createConnectivityNotification(options), dictationInterims: createDictationInterimsStyle(options), - downloadAttachment: createDownloadAttachmentStyle(options), errorBox: createErrorBoxStyle(options), errorNotification: createErrorNotificationStyle(options), + fileContent: createFileContentStyle(options), microphoneButton: createMicrophoneButtonStyle(options), options: { ...options, @@ -202,7 +201,6 @@ export default function createStyleSet(options) { textContent: createTextContentStyle(options), typingAnimation: createTypingAnimationStyle(options), typingIndicator: createTypingIndicatorStyle(options), - uploadAttachment: createUploadAttachmentStyle(options), uploadButton: createUploadButtonStyle(options), videoAttachment: createVideoAttachmentStyle(options), videoContent: createVideoContentStyle(options), diff --git a/packages/component/src/index.tsx b/packages/component/src/index.tsx index b3dc84665b..060f2483e6 100644 --- a/packages/component/src/index.tsx +++ b/packages/component/src/index.tsx @@ -13,6 +13,7 @@ import StackedLayout, { connectStackedLayout } from './Activity/StackedLayout'; import Timestamp from './Middleware/ActivityStatus/Timestamp'; import AudioContent from './Attachment/AudioContent'; +import FileContent from './Attachment/FileContent'; import HTMLVideoContent from './Attachment/HTMLVideoContent'; import ImageContent from './Attachment/ImageContent'; import TextContent from './Attachment/TextContent'; @@ -47,6 +48,7 @@ const Components = { // Components for recomposing activities and attachments AudioContent, + FileContent, HTMLVideoContent, ImageContent, TextContent, diff --git a/packages/core/src/reducers/activities.js b/packages/core/src/reducers/activities.js index cce3206ec6..a58fbc4d0e 100644 --- a/packages/core/src/reducers/activities.js +++ b/packages/core/src/reducers/activities.js @@ -11,6 +11,8 @@ import { POST_ACTIVITY_FULFILLED, POST_ACTIVITY_PENDING, POST_ACTIVITY_REJECTED import { SEND_FAILED, SENDING, SENT } from '../constants/ActivityClientState'; const DEFAULT_STATE = []; +const DIRECT_LINE_PLACEHOLDER_URL = + 'https://docs.botframework.com/static/devportal/client/images/bot-framework-default-placeholder.png'; function getClientActivityID({ channelData: { clientActivityID } = {} }) { return clientActivityID; @@ -20,7 +22,26 @@ function findByClientActivityID(clientActivityID) { return activity => getClientActivityID(activity) === clientActivityID; } +function patchActivity(activity) { + // Direct Line channel will return a placeholder image for the user-uploaded image. + // As observed, the URL for the placeholder image is https://docs.botframework.com/static/devportal/client/images/bot-framework-default-placeholder.png. + // To make our code simpler, we are removing the value if "contentUrl" is pointing to a placeholder image. + + // TODO: [P2] #2869 This "contentURL" removal code should be moved to DirectLineJS adapter. + + // Also, if the "contentURL" starts with "blob:", this means the user is uploading a file (the URL is constructed by URL.createObjectURL) + // Although the copy/reference of the file is temporary in-memory, to make the UX consistent across page refresh, we do not allow the user to re-download the file either. + + return updateIn(activity, ['attachments', () => true, 'contentUrl'], contentUrl => { + if (contentUrl !== DIRECT_LINE_PLACEHOLDER_URL && !/^blob:/iu.test(contentUrl)) { + return contentUrl; + } + }); +} + function upsertActivityWithSort(activities, nextActivity) { + nextActivity = patchActivity(nextActivity); + const { channelData: { clientActivityID: nextClientActivityID } = {} } = nextActivity; const nextTimestamp = Date.parse(nextActivity.timestamp); @@ -77,7 +98,7 @@ export default function activities(state = DEFAULT_STATE, { meta, payload, type case POST_ACTIVITY_FULFILLED: state = updateIn(state, [findByClientActivityID(meta.clientActivityID)], () => // We will replace the activity with the version from the server - updateIn(payload.activity, ['channelData', 'state'], () => SENT) + updateIn(patchActivity(payload.activity), ['channelData', 'state'], () => SENT) ); break;