diff --git a/.dockerignore b/.dockerignore
index 9a97864a16..527d2f98e2 100644
--- a/.dockerignore
+++ b/.dockerignore
@@ -4,3 +4,4 @@
!/packages/bundle/dist
!/packages/playground/build
!/packages/testharness/dist
+!/serve-test.json
diff --git a/CHANGELOG.md b/CHANGELOG.md
index ec6287b901..756ad90b52 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -25,6 +25,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
### Fixed
- Fixes [#1340](https://github.com/microsoft/BotFramework-WebChat/issues/1340). Card container should not be focusable if they do not have `tapAction`, by [@compulim](https://github.compulim) in PR [#3193](https://github.com/microsoft/BotFramework-WebChat/issues/3193)
+- Fixed [#3196](https://github.com/microsoft/BotFramework-WebChat/issues/3196). Cards with `tapAction` should be executable by ENTER or SPACEBAR key, by [@compulim](https://github.com/compulim) in PR [#3197](https://github.com/microsoft/BotFramework-WebChat/pull/3197)
### Changed
diff --git a/Dockerfile-testharness2 b/Dockerfile-testharness2
index 5a5d6e8cbf..bbd9b780f9 100644
--- a/Dockerfile-testharness2
+++ b/Dockerfile-testharness2
@@ -8,8 +8,9 @@ ENV PORT=80
EXPOSE 80
RUN npm install serve@11.3.0 -g
WORKDIR /web/
-ENTRYPOINT ["npx", "--no-install", "serve", "-p", "80", "/web/__tests__/html/"]
+ENTRYPOINT ["npx", "--no-install", "serve", "-c", "serve-test.json", "-p", "80", "/web/"]
+ADD serve-test.json /web/
ADD __tests__/html/ /web/__tests__/html/
ADD packages/bundle/dist/webchat-es5.js /web/packages/bundle/dist/
ADD packages/testharness/dist/testharness.js /web/packages/testharness/dist/
diff --git a/__tests__/__image_snapshots__/html/adaptive-cards-tap-action-js-adaptive-cards-with-tap-action-prop-should-react-to-click-enter-and-spacebar-1-snap.png b/__tests__/__image_snapshots__/html/adaptive-cards-tap-action-js-adaptive-cards-with-tap-action-prop-should-react-to-click-enter-and-spacebar-1-snap.png
new file mode 100644
index 0000000000..7e9d3e015e
Binary files /dev/null and b/__tests__/__image_snapshots__/html/adaptive-cards-tap-action-js-adaptive-cards-with-tap-action-prop-should-react-to-click-enter-and-spacebar-1-snap.png differ
diff --git a/__tests__/html/__jest__/WebChatEnvironment.js b/__tests__/html/__jest__/WebChatEnvironment.js
index 125546917b..4f753831ae 100644
--- a/__tests__/html/__jest__/WebChatEnvironment.js
+++ b/__tests__/html/__jest__/WebChatEnvironment.js
@@ -4,7 +4,7 @@ const NodeEnvironment = require('jest-environment-node');
const { browserName } = require('../../constants.json');
const hostServe = require('./hostServe');
-const serveJSON = require('../serve.json');
+const serveJSON = require('../../../serve-test.json');
class WebChatEnvironment extends NodeEnvironment {
constructor(config, context) {
diff --git a/__tests__/html/adaptiveCards.tapAction.html b/__tests__/html/adaptiveCards.tapAction.html
new file mode 100644
index 0000000000..1ebe2ad67f
--- /dev/null
+++ b/__tests__/html/adaptiveCards.tapAction.html
@@ -0,0 +1,113 @@
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/__tests__/html/adaptiveCards.tapAction.js b/__tests__/html/adaptiveCards.tapAction.js
new file mode 100644
index 0000000000..5b07edc033
--- /dev/null
+++ b/__tests__/html/adaptiveCards.tapAction.js
@@ -0,0 +1,8 @@
+/**
+ * @jest-environment ./__tests__/html/__jest__/WebChatEnvironment.js
+ */
+
+describe('Adaptive Cards', () => {
+ test('with "tapAction" prop should react to click, ENTER, and SPACEBAR', () =>
+ runHTMLTest('adaptiveCards.tapAction.html'));
+});
diff --git a/__tests__/setup/NUnitTestReporter.js b/__tests__/setup/NUnitTestReporter.js
index b86d71f51f..2f34d0779d 100644
--- a/__tests__/setup/NUnitTestReporter.js
+++ b/__tests__/setup/NUnitTestReporter.js
@@ -185,9 +185,11 @@ class NUnitTestReporter {
{ minStartTime: Infinity, maxEndTime: -Infinity }
);
- xml['test-run']['@start-time'] = new Date(minStartTime).toISOString();
- xml['test-run']['@end-time'] = new Date(maxEndTime).toISOString();
- xml['test-run']['@duration'] = Math.max(0, maxEndTime - minStartTime) / 1000;
+ if (isFinite(minStartTime) && isFinite(maxEndTime)) {
+ xml['test-run']['@start-time'] = new Date(minStartTime).toISOString();
+ xml['test-run']['@end-time'] = new Date(maxEndTime).toISOString();
+ xml['test-run']['@duration'] = Math.max(0, maxEndTime - minStartTime) / 1000;
+ }
xml['test-run']['test-suite'] = xml['test-run']['test-suite'].map(testSuite => {
const { 'test-case': testCases } = testSuite;
diff --git a/package.json b/package.json
index ae1f627c14..f8d31b46ef 100644
--- a/package.json
+++ b/package.json
@@ -28,7 +28,7 @@
"eslint": "lerna run --parallel --stream eslint",
"posteslint": "npm run prettier-readmes",
"prettier-readmes": "prettier --write **/**/*.md --tab-width 3 --single-quote true",
- "start": "concurrently --kill-others --raw \"serve\" \"serve -p 5001 -c __tests__/html/serve.json __tests__/html\" \"lerna run --ignore playground --parallel --stream start\"",
+ "start": "concurrently --kill-others --raw \"serve\" \"serve -p 5001 -c serve-test.json\" \"lerna run --ignore playground --parallel --stream start\"",
"tableflip": "npx lerna clean --yes --concurrency 8 && npx rimraf node_modules && npm ci && npm run bootstrap -- --concurrency 8",
"test": "jest --watch",
"test:ci": "npm run test -- --ci --coverage true --no-watch",
diff --git a/packages/bundle/src/adaptiveCards/Attachment/AdaptiveCardRenderer.js b/packages/bundle/src/adaptiveCards/Attachment/AdaptiveCardRenderer.js
index 716a38b122..d6246d1828 100644
--- a/packages/bundle/src/adaptiveCards/Attachment/AdaptiveCardRenderer.js
+++ b/packages/bundle/src/adaptiveCards/Attachment/AdaptiveCardRenderer.js
@@ -222,7 +222,7 @@ const AdaptiveCardRenderer = ({ actionPerformedClassName, adaptiveCard, disabled
const [actionsPerformed, setActionsPerformed] = useState([]);
const [adaptiveCardsHostConfig] = useAdaptiveCardsHostConfig();
const [disabledFromComposer] = useDisabled();
- const [error, setError] = useState();
+ const [errors, setErrors] = useState([]);
const [lastRender, setLastRender] = useState(0);
const activeElementIndexRef = useRef(-1);
const adaptiveCardElementRef = useRef();
@@ -235,18 +235,46 @@ const AdaptiveCardRenderer = ({ actionPerformedClassName, adaptiveCard, disabled
const disabled = disabledFromComposer || disabledFromProps;
- const handleClick = useCallback(
- ({ target }) => {
+ // TODO: [P2] We should consider using `adaptiveCard.selectAction` instead.
+ // This callback will not listen to "click" or "keypress" event if the component is disabled or does not have "tapAction" prop.
+ const handleClickAndKeyPress = useCallback(
+ event => {
+ const { key, target, type } = event;
+
// Some items, e.g. tappable text, cannot be disabled thru DOM attributes
- if (!disabled) {
+ const { current } = contentRef;
+ const adaptiveCardRoot = current.querySelector('.ac-adaptiveCard[tabindex="0"]');
+
+ if (!adaptiveCardRoot) {
+ return console.warn(
+ 'botframework-webchat: No Adaptive Card root container can be found, probably on an unsupported Adaptive Card version.'
+ );
+ }
+
+ // For "keypress" event, we only listen to ENTER and SPACEBAR key.
+ if (type === 'keypress') {
+ if (key !== 'Enter' && key !== ' ') {
+ return;
+ }
+
+ event.preventDefault();
+ }
+
+ // We will call performCardAction if either:
+ // 1. We are on the target, or
+ // 2. The event-dispatching element is not interactive
+ if (target !== adaptiveCardRoot) {
const tabIndex = getTabIndex(target);
// If the user is clicking on something that is already clickable, do not allow them to click the card.
// E.g. a hero card can be tappable, and image and buttons inside the hero card can also be tappable.
- if (typeof tabIndex !== 'number' || tabIndex < 0) {
- tapAction && performCardAction(tapAction);
+ if (typeof tabIndex === 'number' && tabIndex >= 0) {
+ return;
}
}
+
+ performCardAction(tapAction);
+ scrollToEnd();
},
[disabled, performCardAction, tapAction]
);
@@ -331,10 +359,9 @@ const AdaptiveCardRenderer = ({ actionPerformedClassName, adaptiveCard, disabled
const { failures } = adaptiveCard.validateProperties();
if (failures.length) {
- // TODO: [P3] Since this can be called from `componentDidUpdate` and potentially error, we should fix a better way to propagate the error.
- const errors = failures.reduce((items, { errors }) => [...items, ...errors], []);
-
- return setError(errors);
+ return setErrors(
+ failures.reduce((items, { errors }) => [...items, ...errors.map(({ message }) => new Error(message))], [])
+ );
}
let element;
@@ -342,14 +369,15 @@ const AdaptiveCardRenderer = ({ actionPerformedClassName, adaptiveCard, disabled
try {
element = adaptiveCard.render();
} catch (error) {
- return setError(error);
+ return setErrors([error]);
}
if (!element) {
- return setError('Adaptive Card rendered as empty element');
+ return setErrors([new Error('Adaptive Card rendered as empty element')]);
}
- error && setError(null);
+ // Clear errors on next render
+ setErrors([]);
restoreInputValues(element, inputValuesRef.current);
@@ -369,7 +397,7 @@ const AdaptiveCardRenderer = ({ actionPerformedClassName, adaptiveCard, disabled
adaptiveCardElementRef.current = undefined;
};
- }, [adaptiveCard, adaptiveCardsHostConfig, contentRef, error, HostConfig, renderMarkdownAsHTML]);
+ }, [adaptiveCard, adaptiveCardsHostConfig, contentRef, HostConfig, renderMarkdownAsHTML, setErrors]);
useEffect(() => {
// Set onExecuteAction without causing unnecessary re-render.
@@ -404,14 +432,19 @@ const AdaptiveCardRenderer = ({ actionPerformedClassName, adaptiveCard, disabled
return () => undoStack.forEach(undo => undo && undo());
}, [actionsPerformed, actionPerformedClassName, lastRender]);
- return error ? (
-
- {JSON.stringify(error, null, 2)}
+ const handleClickAndKeyPressForTapAction = !disabled && tapAction ? handleClickAndKeyPress : undefined;
+
+ return errors.length ? (
+
+ {errors.map(({ error, message }, index) => (
+ {JSON.stringify({ error, message }, null, 2)}
+ ))}
) : (
);
diff --git a/packages/testharness/src/index.js b/packages/testharness/src/index.js
index 7425ed43e8..58182b38ce 100644
--- a/packages/testharness/src/index.js
+++ b/packages/testharness/src/index.js
@@ -38,6 +38,21 @@ import shareObservable from './utils/shareObservable';
import sleep from './utils/sleep';
import subscribeConsole, { getHistory as getConsoleHistory } from './utils/subscribeConsole';
+function waitForFinishKey() {
+ const { promise, resolve } = createDeferred();
+ const handler = event => {
+ (event.code === 'CapsLock' || event.code === 'ShiftRight') && resolve();
+ };
+
+ window.addEventListener('keyup', handler);
+
+ log('WebChatTest: After you complete the last step, press CAPSLOCK or right SHIFT key to continue.');
+
+ return promise.finally(() => {
+ window.removeEventListener('keyup', handler);
+ });
+}
+
window.Babel.registerPlugin(
'@babel/plugin-proposal-async-generator-functions',
BabelPluginProposalAsyncGeneratorFunctions
@@ -78,18 +93,22 @@ if (!webDriverMode) {
break;
case 'send keys':
- log(`WebChatTest: Please press this key sequence: ${job.payload.keys.join(', ')}.`);
- await sleep(1000);
+ log(
+ `WebChatTest: Please press this key sequence: ${job.payload.keys
+ .map(key => (key === '\n' ? 'ENTER' : key === ' ' ? 'SPACEBAR' : key))
+ .join(', ')}.`
+ );
+ await waitForFinishKey();
break;
case 'send shift tab':
log(`WebChatTest: Please press SHIFT-TAB key.`);
- await sleep(1000);
+ await waitForFinishKey();
break;
case 'send tab':
log(`WebChatTest: Please press TAB key.`);
- await sleep(1000);
+ await waitForFinishKey();
break;
default:
diff --git a/packages/testharness/src/pageObjects/runHook.js b/packages/testharness/src/pageObjects/runHook.js
index 2353afa786..8278449792 100644
--- a/packages/testharness/src/pageObjects/runHook.js
+++ b/packages/testharness/src/pageObjects/runHook.js
@@ -8,6 +8,7 @@ export default function runHook(fn) {
activity: {
from: { role: 'user' },
name: '__RUN_HOOK',
+ ref: { count: 0 },
timestamp: new Date(0).toISOString(),
type: 'event',
value: { fn, resolve }
diff --git a/packages/testharness/src/token/fetchDirectLineToken.js b/packages/testharness/src/token/fetchDirectLineToken.js
index f445104d8f..2cfcf4651a 100644
--- a/packages/testharness/src/token/fetchDirectLineToken.js
+++ b/packages/testharness/src/token/fetchDirectLineToken.js
@@ -1,5 +1,5 @@
-export default async function() {
- const res = await fetch('https://webchat-mockbot.azurewebsites.net/directline/token', { method: 'POST' });
+export default async function(url = 'https://webchat-mockbot.azurewebsites.net/directline/token') {
+ const res = await fetch(url, { method: 'POST' });
if (!res.ok) {
throw new Error('Failed to fetch Direct Line token.');
diff --git a/packages/testharness/src/utils/createRunHookActivityMiddleware.js b/packages/testharness/src/utils/createRunHookActivityMiddleware.js
index 19d37b5e6c..def343445e 100644
--- a/packages/testharness/src/utils/createRunHookActivityMiddleware.js
+++ b/packages/testharness/src/utils/createRunHookActivityMiddleware.js
@@ -1,4 +1,4 @@
-const { createElement, useRef } = window.React;
+const { createElement } = window.React;
const RunHook = ({ fn, resolve }) => {
resolve(fn(window.WebChat.hooks));
@@ -6,16 +6,10 @@ const RunHook = ({ fn, resolve }) => {
return false;
};
-const RunHookOnce = props => {
- const renderCount = useRef(0);
-
- return !renderCount.current++ && createElement(RunHook, props);
-};
-
function createRunHookActivityMiddleware() {
return () => next => ({ activity, ...others }) => {
if (activity.type === 'event' && activity.name === '__RUN_HOOK') {
- return () => createElement(RunHookOnce, activity.value);
+ return () => !activity.ref.count++ && createElement(RunHook, activity.value);
}
return next({ activity, ...others });
diff --git a/__tests__/html/serve.json b/serve-test.json
similarity index 97%
rename from __tests__/html/serve.json
rename to serve-test.json
index 5aa3bb001c..7a8a4866c0 100644
--- a/__tests__/html/serve.json
+++ b/serve-test.json
@@ -10,7 +10,6 @@
]
}
],
- "public": "../..",
"rewrites": [
{
"source": "/assets/:filename",