Skip to content

Commit

Permalink
#3453 Fixes plain text file attachments to show download link when up…
Browse files Browse the repository at this point in the history
…loaded (#3711)

* Add test for uploading .txt attachment

* Update CHANGELOG.md

* Update CHANGELOG.md

* Fix breaking tests

* Clean up

* Update packages/component/src/Middleware/AttachmentForScreenReader/createCoreMiddleware.js

Co-authored-by: William Wong <compulim@users.noreply.github.com>

* Linting fixes from previous PR

* More linting fixes from previous PR

* Fix logic and add tests

Co-authored-by: William Wong <compulim@users.noreply.github.com>
  • Loading branch information
corinagum and compulim authored Feb 10, 2021
1 parent 9cea4cd commit abb2203
Show file tree
Hide file tree
Showing 19 changed files with 199 additions and 20 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
- Fixes [#3431](https://github.com/microsoft/BotFramework-WebChat/issues/3431). Race condition between the first bot activity and first user activity should not cause the first bot activity to be delayed, by [@compulim](https://github.com/compulim) in PR [#3705](https://github.com/microsoft/BotFramework-WebChat/pull/3705)
- Fixes [#3676](https://github.com/microsoft/BotFramework-WebChat/issues/3676). Activities without text should not generate bogus `aria-labelledby`, by [@compulim](https://github.com/compulim) in PR [#3697](https://github.com/microsoft/BotFramework-WebChat/pull/3697)
- Fixes [#3625](https://github.com/microsoft/BotFramework-WebChat/issues/3625). Update 'no screen reader for custom activity middleware' warning and add screen reader renderer documentation to `ACCESSIBILITY.md`, by [@corinagum](https://github.com/corinagum) in PR [#3689](https://github.com/microsoft/BotFramework-WebChat/pull/3689)
- Fixes [#3453](https://github.com/microsoft/BotFramework-WebChat/issues/3453). Fixes plain text file attachments to show download link when uploaded, by [@corinagum](https://github.com/corinagum) in PR [#3711](https://github.com/microsoft/BotFramework-WebChat/pull/3711)
- Fixes [#3612](https://github.com/microsoft/BotFramework-WebChat/issues/3612). Carousel flippers in suggested actions are given extra padding, by [@compulim](https://github.com/compulim) and [@Quirinevwm](https://github.com/Quirinevwm) in PR [#3704](https://github.com/microsoft/BotFramework-WebChat/pull/3704)
- Fixes [#3411](https://github.com/microsoft/BotFramework-WebChat/issues/3411). With Direct Line Speech, clicking on microphone button during speech recognition should no longer stop working, by [@compulim](https://github.com/compulim) in PR [#3694](https://github.com/microsoft/BotFramework-WebChat/pull/3694)
- Although it no locker lock up microphone, clicking on the microphone button has no effect because Direct Line Speech does not support aborting speech recognition
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
67 changes: 67 additions & 0 deletions __tests__/html/sendFiles.attachmentUrl.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
<!DOCTYPE html>
<html lang="en-US">
<head>
<script crossorigin="anonymous" src="/__dist__/testharness.js"></script>
<script crossorigin="anonymous" src="/__dist__/webchat-es5.js"></script>
</head>
<body>
<div id="webchat"></div>
<script type="text/babel" data-presets="env,stage-3,react">
const {
WebChatTest: {
conditions,
createDirectLineWithTranscript,
createStore,
getConsoleHistory,
host,
pageObjects,
timeouts,
token
}
} = window;

(async function () {
window.WebChat.renderWebChat(
{
directLine: createDirectLineWithTranscript([
{
type: 'message',
id: 'CONVERSATION_ID-o|00000',
timestamp: '2000-01-23T12:34:56.12345Z',
channelId: 'directline',
from: {
id: 'webchat-mockbot',
name: 'webchat-mockbot'
},
conversation: {
id: 'CONVERSATION_ID-o'
},
locale: 'en-US',
attachments: [
{
contentUrl: 'Hello, World!',
contentType: 'text/plain',
name: 'test.txt'
}
]
}
]),
sendTypingIndicator: true,
store: createStore()
},
document.getElementById('webchat')
);

await pageObjects.wait(conditions.uiConnected(), timeouts.directLine);
await pageObjects.wait(conditions.numActivitiesShown(1), timeouts.directLine);

await host.snapshot();
await host.done();
})().catch(async err => {
console.error(err);

await host.error(err);
});
</script>
</body>
</html>
7 changes: 7 additions & 0 deletions __tests__/html/sendFiles.attachmentUrl.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
/**
* @jest-environment ./__tests__/html/__jest__/WebChatEnvironment.js
*/

describe('text url attachment sent from bot should be rendered', () => {
test('', () => runHTMLTest('sendFiles.attachmentUrl.html'));
});
67 changes: 67 additions & 0 deletions __tests__/html/sendFiles.textFiles.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
<!DOCTYPE html>
<html lang="en-US">
<head>
<script crossorigin="anonymous" src="/__dist__/testharness.js"></script>
<script crossorigin="anonymous" src="/__dist__/webchat-es5.js"></script>
</head>
<body>
<div id="webchat"></div>
<script type="text/babel" data-presets="env,stage-3,react">
const {
WebChatTest: {
conditions,
createDirectLineWithTranscript,
createStore,
getConsoleHistory,
host,
pageObjects,
timeouts,
token
}
} = window;

(async function () {
window.WebChat.renderWebChat(
{
directLine: createDirectLineWithTranscript([
{
type: 'message',
id: 'CONVERSATION_ID-o|00000',
timestamp: '2000-01-23T12:34:56.12345Z',
channelId: 'directline',
from: {
id: 'webchat-mockbot',
name: 'webchat-mockbot'
},
conversation: {
id: 'CONVERSATION_ID-o'
},
locale: 'en-US',
attachments: [
{
content: 'Hello, World!',
contentType: 'text/plain',
name: 'test.txt'
}
]
}
]),
sendTypingIndicator: true,
store: createStore()
},
document.getElementById('webchat')
);

await pageObjects.wait(conditions.uiConnected(), timeouts.directLine);
await pageObjects.wait(conditions.numActivitiesShown(1), timeouts.directLine);

await host.snapshot();
await host.done();
})().catch(async err => {
console.error(err);

await host.error(err);
});
</script>
</body>
</html>
7 changes: 7 additions & 0 deletions __tests__/html/sendFiles.textFiles.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
/**
* @jest-environment ./__tests__/html/__jest__/WebChatEnvironment.js
*/

describe('text file sent from bot should be rendered', () => {
test('', () => runHTMLTest('sendFiles.textFiles.html'));
});
Empty file added __tests__/setup/local/empty.txt
Empty file.
36 changes: 28 additions & 8 deletions __tests__/upload.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
/* eslint no-magic-numbers: "off" */
/* eslint no-undef: "off" */

import { imageSnapshotOptions, timeouts } from './constants.json';

import allImagesLoaded from './setup/conditions/allImagesLoaded';
Expand Down Expand Up @@ -28,7 +31,7 @@ describe('upload a picture', () => {
});
}
},
// TODO: [P3] Offline bot did not reply with a downloadable attachment, we need to use production bot
// TODO: [P3] Offline bot did not reply with a downloadable attachment, so we need to use production bot
useProductionBot: true
});

Expand Down Expand Up @@ -132,7 +135,7 @@ describe('upload a picture', () => {
uploadThumbnailWidth: 120
}
},
// TODO: [P3] Offline bot did not reply with a downloadable attachment, we need to use production bot
// TODO: [P3] Offline bot did not reply with a downloadable attachment, so we need to use production bot
useProductionBot: true
});

Expand All @@ -154,7 +157,7 @@ describe('upload a picture', () => {
uploadThumbnailQuality: 0.1
}
},
// TODO: [P3] Offline bot did not reply with a downloadable attachment, we need to use production bot
// TODO: [P3] Offline bot did not reply with a downloadable attachment, so we need to use production bot
useProductionBot: true
});

Expand All @@ -176,7 +179,7 @@ describe('upload a picture', () => {
enableUploadThumbnail: false
}
},
// TODO: [P3] Offline bot did not reply with a downloadable attachment, we need to use production bot
// TODO: [P3] Offline bot did not reply with a downloadable attachment, so we need to use production bot
useProductionBot: true
});

Expand All @@ -194,7 +197,7 @@ describe('upload a picture', () => {
describe('without Web Worker', () => {
test('', async () => {
const { driver, pageObjects } = await setupWebDriver({
// TODO: [P3] Offline bot did not reply with a downloadable attachment, we need to use production bot
// TODO: [P3] Offline bot did not reply with a downloadable attachment, so we need to use production bot
useProductionBot: true
});

Expand All @@ -221,7 +224,7 @@ describe('upload a picture', () => {
uploadThumbnailWidth: 120
}
},
// TODO: [P3] Offline bot did not reply with a downloadable attachment, we need to use production bot
// TODO: [P3] Offline bot did not reply with a downloadable attachment, so we need to use production bot
useProductionBot: true
});

Expand All @@ -246,7 +249,7 @@ describe('upload a picture', () => {
uploadThumbnailQuality: 0.1
}
},
// TODO: [P3] Offline bot did not reply with a downloadable attachment, we need to use production bot
// TODO: [P3] Offline bot did not reply with a downloadable attachment, so we need to use production bot
useProductionBot: true
});

Expand All @@ -268,7 +271,7 @@ describe('upload a picture', () => {

test('upload a ZIP file', async () => {
const { driver, pageObjects } = await setupWebDriver({
// TODO: [P3] Offline bot did not reply with a downloadable attachment, we need to use production bot
// TODO: [P3] Offline bot did not reply with a downloadable attachment, so we need to use production bot
useProductionBot: true
});

Expand All @@ -282,3 +285,20 @@ test('upload a ZIP file', async () => {

expect(base64PNG).toMatchImageSnapshot(imageSnapshotOptions);
});

test('upload a .txt (plain) file', async () => {
const { driver, pageObjects } = await setupWebDriver({
// TODO: [P3] Offline bot did not reply with a downloadable attachment, so we need to use production bot
useProductionBot: true
});

await driver.wait(uiConnected(), timeouts.directLine);

await pageObjects.sendFile('empty.txt');
await driver.wait(minNumActivitiesShown(2), timeouts.directLine);
await driver.wait(allImagesLoaded(), timeouts.fetchImage);

const base64PNG = await driver.takeScreenshot();

expect(base64PNG).toMatchImageSnapshot(imageSnapshotOptions);
});
2 changes: 1 addition & 1 deletion lint-staged.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ function prettierMarkdown(filenames) {

// eslint-disable-next-line no-undef
module.exports = {
'{__tests__,docs,samples}/**/*.{html,js,jsx,ts,tsx}': prettierCode,
'{docs,samples}/**/*.{html,js,jsx,ts,tsx}': prettierCode,
'**/*.md': prettierMarkdown,
'packages/{api,bundle,component,core,directlinespeech,playground}/src/**/*.{js,jsx,ts,tsx}': prettierCode,
'*.{js,jsx,ts,tsx}': 'npm run eslint'
Expand Down
7 changes: 4 additions & 3 deletions packages/component/src/Attachment/TextAttachment.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,13 @@ import React from 'react';

import TextContent from './TextContent';

const TextAttachment = ({ attachment: { content, contentType } = {} }) =>
!!content && <TextContent contentType={contentType} text={content} />;
const TextAttachment = ({ attachment: { content = '', contentType } = {} }) => (
<TextContent contentType={contentType} text={content} />
);

TextAttachment.propTypes = {
attachment: PropTypes.shape({
content: PropTypes.string.isRequired,
content: PropTypes.string,
contentType: PropTypes.string.isRequired
}).isRequired
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ export default function createCoreMiddleware() {
}
] = args;

return role === 'user' && !/^text\//u.test(contentType) && !thumbnailUrl ? (
const isText = /^text\//u.test(contentType);

return (isText ? !attachment.content : role === 'user' && !thumbnailUrl) ? (
<FileAttachment activity={activity} attachment={attachment} />
) : /^audio\//u.test(contentType) ? (
<AudioAttachment activity={activity} attachment={attachment} />
Expand All @@ -31,7 +33,7 @@ export default function createCoreMiddleware() {
<VideoAttachment activity={activity} attachment={attachment} />
) : contentUrl || contentType === 'application/octet-stream' ? (
<FileAttachment activity={activity} attachment={attachment} />
) : /^text\//u.test(contentType) ? (
) : isText ? (
<TextAttachment activity={activity} attachment={attachment} />
) : (
next(...args)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ export default function createCoreMiddleware() {
}
] = args;

return role === 'user' && !/^text\//u.test(contentType) && !thumbnailUrl
const isText = /^text\//u.test(contentType);

return (isText ? !attachment.content : role === 'user' && !thumbnailUrl)
? () => <FileAttachment attachment={attachment} />
: /^audio\//u.test(contentType)
? () => <AudioAttachment />
Expand All @@ -27,7 +29,7 @@ export default function createCoreMiddleware() {
? () => <VideoAttachment />
: contentUrl || contentType === 'application/octet-stream'
? () => <FileAttachment attachment={attachment} />
: /^text\//u.test(contentType)
: isText
? () => <TextAttachment attachment={attachment} />
: next(...args);
}
Expand Down
1 change: 1 addition & 0 deletions packages/component/src/SendBox/Assets/MicrophoneIcon.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import React from 'react';

// eslint-disable-next-line react/prop-types
const MicrophoneIcon = ({ className }) => (
<svg className={className} height={28} viewBox="0 0 34.75 46" width={28}>
<path
Expand Down
2 changes: 1 addition & 1 deletion packages/component/src/Styles/StyleSet/SuggestedActions.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* eslint no-empty-pattern: "off" */
/* eslint no-magic-numbers: ["error", { "ignore": [2] }] */
/* eslint no-magic-numbers: ["error", { "ignore": [1.5, 2] }] */

export default function createSuggestedActionsStyle({
paddingRegular,
Expand Down
2 changes: 1 addition & 1 deletion packages/directlinespeech/src/DirectLineSpeech.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* eslint no-magic-numbers: ["error", { "ignore": [0, 1, 2, 36] }] */
/* eslint no-magic-numbers: ["error", { "ignore": [0, 1, 2, 4, 36] }] */

import Observable from 'core-js/features/observable';
import random from 'math-random';
Expand Down
6 changes: 5 additions & 1 deletion packages/directlinespeech/src/createAdapters.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import resolveFunctionOrReturnValue from './resolveFunctionOrReturnValue';
const DIRECT_LINE_TOKEN_RENEWAL_INTERVAL = 900000; // 15 minutes
const TOKEN_RENEWAL_INTERVAL = 120000;

// eslint-disable-next-line complexity
export default async function create({
audioConfig,
audioContext,
Expand Down Expand Up @@ -238,7 +239,10 @@ export default async function create({

config.setProperty(PropertyId.Conversation_Agent_Connection_Id, refreshedDirectLineToken);

dialogServiceConnector.properties.setProperty(PropertyId.Conversation_Agent_Connection_Id, refreshedDirectLineToken);
dialogServiceConnector.properties.setProperty(
PropertyId.Conversation_Agent_Connection_Id,
refreshedDirectLineToken
);
dialogServiceConnector.connect();
}, DIRECT_LINE_TOKEN_RENEWAL_INTERVAL);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* eslint no-magic-numbers: ["error", { "ignore": [0, 1, 8, 16, 32, 128, 1000, 32768, 96000, 2147483648] }] */
/* eslint no-magic-numbers: ["error", { "ignore": [0, 1, 8, 16, 32, 128, 1000, 16000, 32768, 96000, 2147483648] }] */
/* eslint no-await-in-loop: "off" */
/* eslint prefer-destructuring: "off" */

Expand Down

0 comments on commit abb2203

Please sign in to comment.