Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Direct Line Speech null attachments and update test snapshots #3154

Merged
merged 4 commits into from
May 12, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 11 additions & 5 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,18 +40,14 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
- Fixes [#2933](https://github.com/microsoft/BotFramework-WebChat/issues/2933). Fix `text` should not be ignored in `messageBack` action in hero card, by [@geea-develop](https://github.com/geea-develop) and [@compulim](https://github.com/compulim) in PR [#3003](https://github.com/microsoft/BotFramework-WebChat/pull/3003)
- Fixes [#2562](https://github.com/microsoft/BotFramework-WebChat/issues/2562). Fix timestamps should not stop updating, by [@compulim](https://github.com/compulim) in PR [#3066](https://github.com/microsoft/BotFramework-WebChat/pull/3066)
- Fixes [#2953](https://github.com/microsoft/BotFramework-WebChat/issues/2953). Direct Line Speech should not synthesize when the `speak` property is falsy, by [@compulim](https://github.com/compulim) in PR [#3059](https://github.com/microsoft/BotFramework-WebChat/pull/3059)
- Fixes [#3075](https://github.com/microsoft/BotFramework-WebChat/issues/3075). Fix usability issues around accessibility, by [@compulim](https://github.com/compulim) in PR [#3076](https://github.com/microsoft/BotFramework-WebChat/issue/3076)
- Fix timestamp should not be narrated more than once.
- Associate the activity text with its attachments, by adding a `role="region"` to the activity DOM element.
- Fixes [#3074](https://github.com/microsoft/BotFramework-WebChat/issues/3074). Keep `props.locale` when sending to the bot, by [@compulim](https://github.com/compulim) in PR [#3095](https://github.com/microsoft/BotFramework-WebChat/issue/3095)
- Fixes [#3096](https://github.com/microsoft/BotFramework-WebChat/issues/3096). Use `<ScreenReaderText>` instead of `aria-label` for message bubbles, by [@compulim](https://github.com/compulim) in PR [#3097](https://github.com/microsoft/BotFramework-WebChat/issue/3097)
- Fixes [#2876](https://github.com/microsoft/BotFramework-WebChat/issues/2876). `messageBack` and `postBack` should send even if both `text` and `value` is falsy or `undefined`, by [@compulim](https://github.com/compulim) in PR [#3120](https://github.com/microsoft/BotFramework-WebChat/issues/3120)
- Fixes [#2668](https://github.com/microsoft/BotFramework-WebChat/issues/2668). Disable Web Audio on insecure connections, by [@compulim](https://github.com/compulim) in PR [#3079](https://github.com/microsoft/BotFramework-WebChat/issue/3079)
- Fixes [#2850](https://github.com/microsoft/BotFramework-WebChat/issues/2850). After click suggested action, should focus to send box without keyboard, by [@compulim](https://github.com/compulim) in PR [#3123](https://github.com/microsoft/BotFramework-WebChat/issue/3123)
- Fixes [#3133](https://github.com/microsoft/BotFramework-WebChat/issues/3133). Associate ARIA labels with buttons in hero card and Adaptive Cards, by [@compulim](https://github.com/compulim) in PR [#3146](https://github.com/microsoft/BotFramework-WebChat/issues/3146).
- Remove browser-based detection from `<ScreenReaderText>` because it is no longer needed.
- After stripping Markdown syntax for accessibility labels, cache the result to improve rendering performance.
- Skip stripping Markdown for non-Markdown text content.
- Fixes [#3155](https://github.com/microsoft/BotFramework-WebChat/issues/3155). Patch incoming activities with null fields, by [@compulim](https://github.com/compulim) in PR [#3154](https://github.com/microsoft/BotFramework-WebChat/pull/3154)

### Changed

Expand Down Expand Up @@ -119,6 +115,16 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
- Resolves [#2806](https://github.com/microsoft/BotFramework-WebChat/issues/2806), added [Single sign-on with On Behalf Of Token Authentication](https://webchat-sample-obo.azurewebsites.net/) sample, by [@tdurnford](https://github.com/tdurnford) in [#2865](https://github.com/microsoft/BotFramework-WebChat/pull/2865)
- Resolves [#2481](https://github.com/microsoft/BotFramework-WebChat/issues/2481), added selectable audio input device sample, by [@compulim](https://github.com/compulim) in PR [#3079](https://github.com/microsoft/BotFramework-WebChat/pull/3079)

## [4.8.1] - 2020-04-15

### Fixed

- Fixes [#3075](https://github.com/microsoft/BotFramework-WebChat/issues/3075). Fix usability issues around accessibility, by [@compulim](https://github.com/compulim) in PR [#3076](https://github.com/microsoft/BotFramework-WebChat/issue/3076)
- Fix timestamp should not be narrated more than once.
- Associate the activity text with its attachments, by adding a `role="region"` to the activity DOM element.
- Fixes [#3074](https://github.com/microsoft/BotFramework-WebChat/issues/3074). Keep `props.locale` when sending to the bot, by [@compulim](https://github.com/compulim) in PR [#3095](https://github.com/microsoft/BotFramework-WebChat/issue/3095)
- Fixes [#3096](https://github.com/microsoft/BotFramework-WebChat/issues/3096). Use `<ScreenReaderText>` instead of `aria-label` for message bubbles, by [@compulim](https://github.com/compulim) in PR [#3097](https://github.com/microsoft/BotFramework-WebChat/issue/3097)

## [4.8.0] - 2020-03-05

### Breaking changes
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.
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.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
101 changes: 101 additions & 0 deletions __tests__/html/chatAdapter.nullFields.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
<!DOCTYPE html>
<html lang="en-US">
<head>
<script crossorigin="anonymous" src="https://unpkg.com/@babel/standalone@7/babel.min.js"></script>
<script crossorigin="anonymous" src="https://unpkg.com/regenerator-runtime/runtime.js"></script>
<script crossorigin="anonymous" src="https://unpkg.com/react@16.8.6/umd/react.development.js"></script>
<script crossorigin="anonymous" src="https://unpkg.com/react-dom@16.8.6/umd/react-dom.development.js"></script>
<script
crossorigin="anonymous"
src="https://unpkg.com/react-dom@16.8.6/umd/react-dom-test-utils.development.js"
></script>
<script
crossorigin="anonymous"
src="https://unpkg.com/simple-update-in@2.1.1/dist/simple-update-in.production.min.js"
></script>
<script crossorigin="anonymous" src="/__dist__/testharness.js"></script>
<link href="/assets/index.css" rel="stylesheet" type="text/css" />
</head>
<body>
<div id="webchat"></div>
<script crossorigin="anonymous" src="/__dist__/webchat-es5.js"></script>
<script type="text/babel" data-presets="env,stage-3">
const updateIn = window.simpleUpdateIn;
const { conditions, createStore, host, pageObjects, shareObservable, timeouts, token } = window.WebChatTest;

(async function() {
function createDirectLineForTest(options) {
const UNINITIALIZED = 0;
const workingDirectLine = window.WebChat.createDirectLine(options);

return {
...workingDirectLine,
activity$: new Observable(observer => {
const subscription = workingDirectLine.activity$.subscribe({
complete() {
observer.complete();
},
error(error) {
observer.error(error);
},
next(value) {
// Fill as much null as possible, it should not break rendering.
value = [
'attachmentLayout',
'attachments',
'channelData',
'conversation',
'entities',
'from',
'inputHint',
'locale',
'name',
'recipient',
'speak',
'suggestedActions',
'text',
'textFormat',
'timestamp',
'type'
].reduce((activity, name) => updateIn(activity, [name], value => value || null), value);

observer.next(value);
}
});

return () => subscription.unsubscribe();
}),
postActivity: workingDirectLine.postActivity.bind(workingDirectLine),
connectionStatus$: workingDirectLine.connectionStatus$
};
}

window.WebChat.renderWebChat(
{
directLine: createDirectLineForTest({ token: await token.fetchDirectLineToken() }),
store: createStore()
},
document.getElementById('webchat')
);

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

await pageObjects.sendMessageViaSendBox('echo Hello, World!', { waitForSend: true });
await pageObjects.wait(conditions.minNumActivitiesShown(3), timeouts.directLine);

await pageObjects.sendMessageViaSendBox('suggested-actions', { waitForSend: true });
await pageObjects.wait(conditions.minNumActivitiesShown(5), timeouts.directLine);
await pageObjects.wait(conditions.suggestedActionsShown(), timeouts.ui);
await pageObjects.wait(conditions.allImagesLoaded(), timeouts.network);

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/chatAdapter.nullFields.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
/**
* @jest-environment ./__tests__/html/__jest__/WebChatEnvironment.js
*/

describe('chat adapter', () => {
test('should render properly if some activity fields are null', () => runHTMLTest('chatAdapter.nullFields.html'));
});
31 changes: 31 additions & 0 deletions packages/core/src/sagas/incomingActivitySaga.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import observeEach from './effects/observeEach';
import setSuggestedActions from '../actions/setSuggestedActions';
import whileConnected from './effects/whileConnected';

const PASSTHRU_FN = value => value;

function patchActivityWithFromRole(activity, userID) {
// Some activities, such as "ConversationUpdate", does not have "from" defined.
// And although "role" is defined in Direct Line spec, it was not sent over the wire.
Expand All @@ -29,8 +31,37 @@ function patchActivityWithFromRole(activity, userID) {
return activity;
}

function patchNullAsUndefined(activity) {
// These fields are known used in Web Chat and in any cases, they should not be null, but undefined.
// The only field omitted is "value", as it could be null purposefully.

return [
'attachmentLayout',
'attachments',
'channelData',
'conversation',
'entities',
'from',
'inputHint',
'locale',
'name',
'recipient',
'speak',
'suggestedActions',
'text',
'textFormat',
'timestamp',
'type'
].reduce((activity, name) => {
const { [name]: value } = activity;

return updateIn(activity, [name], typeof value === 'undefined' || value === null ? undefined : PASSTHRU_FN);
}, activity);
}

function* observeActivity({ directLine, userID }) {
yield observeEach(directLine.activity$, function* observeActivity(activity) {
activity = patchNullAsUndefined(activity);
activity = patchActivityWithFromRole(activity, userID);

yield put(incomingActivity(activity));
Expand Down
6 changes: 3 additions & 3 deletions packages/directlinespeech/__tests__/sendSpeechActivity.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@ test('should echo back "Bellevue" when saying "bellview"', async () => {
`);
});

test('should not synthesis when "speak" is empty', async () => {
// 2020-05-11: Direct Line Speech protocol was updated to synthesize "text" if "speak" property is not set.
test('should synthesis if "speak" is empty', async () => {
const { directLine, sendTextAsSpeech } = await createTestHarness();

const connectedPromise = waitForConnected(directLine);
Expand All @@ -74,6 +75,5 @@ test('should not synthesis when "speak" is empty', async () => {
const activities = await activitiesPromise;
const activityUtterances = await Promise.all(activities.map(activity => recognizeActivityAsText(activity)));

expect(activityUtterances).toHaveProperty('length', 1);
expect(activityUtterances[0]).toBeFalsy();
expect(activityUtterances).toEqual([`Don't speak anything.`]);
});
6 changes: 6 additions & 0 deletions packages/testharness/src/conditions/allImagesLoaded.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
export default function allImagesLoaded() {
return {
message: 'all images to be loaded',
fn: () => [].every.call(document.querySelectorAll('img'), ({ complete }) => complete)
};
}
2 changes: 2 additions & 0 deletions packages/testharness/src/conditions/index.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import actionDispatched from './actionDispatched';
import allImagesLoaded from './allImagesLoaded';
import allOutgoingActivitiesSent from './allOutgoingActivitiesSent';
import connectivityStatusShown from './connectivityStatusShown';
import minNumActivitiesShown from './minNumActivitiesShown';
Expand All @@ -12,6 +13,7 @@ import webChatRendered from './webChatRendered';

export {
actionDispatched,
allImagesLoaded,
allOutgoingActivitiesSent,
connectivityStatusShown,
minNumActivitiesShown,
Expand Down