Merge branch 'master' into fix-2341

.github/CONTRIBUTING.md

@@ -37,8 +37,7 @@ There are 3 types of build tasks in the build process.
 We built a playground app for testing Web Chat so we can test certain Web Chat specific features.
 
 ```sh
-cd packages/playground
-npm start
+npm run start:playground
 ```
 
 Then browse to http://localhost:3000/, and click on one of the connection options on the upper right corner.

CHANGELOG.md

@@ -65,6 +65,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
    -  `samples/*`: Move to production version of Web Chat, and bump to [`react@16.8.6`](https://www.npmjs.com/package/react) and [`react-dom@16.8.6`](https://www.npmjs.com/package/react-dom)
 -  Moved the typing indicator to the send box and removed the typing indicator logic from the sagas, by [@tdurnford](https://github.com/tdurnford), in PR [#2321](https://github.com/microsoft/BotFramework-WebChat/pull/2321)
 -  `component`: Move `Composer` to React hooks and functional components, by [@compulim](https://github.com), in PR [#2308](https://github.com/compulim/BotFramework-WebChat/pull/2308)
+-  `component`: Fix [#1818](https://github.com/microsoft/BotFramework-WebChat/issues/1818) Move to functional components by [@corinagum](https://github.com/corinagum), in PR [#2322](https://github.com/microsoft/BotFramework-WebChat/pull/2322)
 
 ### Fixed
 

__tests__/README.md

@@ -2,10 +2,10 @@
 
 Automated testing in Web Chat is using multiple open-source technologies.
 
--  [Travis CI](https://travis-ci.org/) for automatic testing
+-  [Azure DevOps](https://azure.microsoft.com/en-us/services/devops/) for automatic testing
 -  Test against [MockBot](https://github.com/compulim/BotFramework-MockBot)
    -  Try it out with this [live demo](https://microsoft.github.io/BotFramework-WebChat/01.a.getting-started-full-bundle)
--  Visual regression test (a.k.a. compare screenshots)
+-  Visual regression tests (a.k.a. compare screenshots)
    -  Generated on [Chrome on Docker](https://github.com/SeleniumHQ/docker-selenium)
    -  Compared using [`pixelmatch`](https://npmjs.com/package/pixelmatch) via [`jest-image-snapshot`](https://npmjs.com/package/jest-image-snapshot)
 -  Run under [`Jest`](https://jestjs.io/)
@@ -16,7 +16,8 @@ Automated testing in Web Chat is using multiple open-source technologies.
 
 -  Install Docker
 -  On Windows, set environment variable `COMPOSE_CONVERT_WINDOWS_PATHS=1`
--  `docker-compose up --build`
+-   `npm run start:docker`
+- In a separate terminal, run:
 -  `npm test`
 
 ### Running tests under local box

__tests__/clockSkew.js

@@ -1,6 +1,7 @@
-import { By, Condition, Key } from 'selenium-webdriver';
+import { By } from 'selenium-webdriver';
 
 import { imageSnapshotOptions, timeouts } from './constants.json';
+import allImagesLoaded from './setup/conditions/allImagesLoaded';
 import allOutgoingActivitiesSent from './setup/conditions/allOutgoingActivitiesSent';
 import minNumActivitiesShown from './setup/conditions/minNumActivitiesShown';
 import uiConnected from './setup/conditions/uiConnected';
@@ -11,8 +12,10 @@ import uiConnected from './setup/conditions/uiConnected';
 jest.setTimeout(timeouts.test);
 
 describe('Clock skew', () => {
-  test('should not have any effects', async () => {
-    const { driver, pageObjects } = await setupWebDriver({
+  let driver, pageObjects;
+
+  beforeEach(async () => {
+    const result = await setupWebDriver({
       createDirectLine: options => {
         const workingDirectLine = window.WebChat.createDirectLine(options);
         const activityBroker = window.createProduceConsumeBroker();
@@ -67,6 +70,11 @@ describe('Clock skew', () => {
       }
     });
 
+    driver = result.driver;
+    pageObjects = result.pageObjects;
+  });
+
+  test('should not have any effects', async () => {
     await driver.wait(uiConnected(), timeouts.directLine);
     await pageObjects.sendMessageViaSendBox('card bingsports', { waitForSend: false });
 
@@ -75,16 +83,17 @@ describe('Clock skew', () => {
     await driver.wait(minNumActivitiesShown(2), timeouts.directLine);
     await driver.wait(allOutgoingActivitiesSent(), timeouts.directLine);
 
-    // Specifically set a large clock skew to make sure the 2nd user-originated activity has a positive clock skew.
+    // Specifically set a large clock skew to make sure the 2nd user-originated activity has a positive clock skew (appears on bottom).
     // A large positive clock skew is used to test if it is below the 2nd bot-originated activity or not.
+    // Recently, we found the clock in the Docker image has a clockskew of 1.1 hours. So putting a huge number like 10 days should make it work.
     await pageObjects.dispatchAction({
-      payload: { value: 120000 },
+      payload: { value: 864000000 },
       type: 'WEB_CHAT/SET_CLOCK_SKEW_ADJUSTMENT'
     });
 
     // Make sure the clock skew is set correctly.
     // If it is not set, the result could be false-positive.
-    await expect(pageObjects.getStore()).resolves.toHaveProperty('clockSkewAdjustment', 120000);
+    await expect(pageObjects.getStore()).resolves.toHaveProperty('clockSkewAdjustment', 864000000);
 
     await pageObjects.sendMessageViaSendBox('echo This outgoing activity should be the last in the list.', {
       waitForSend: false
@@ -103,4 +112,46 @@ describe('Clock skew', () => {
 
     expect(await driver.takeScreenshot()).toMatchImageSnapshot(imageSnapshotOptions);
   });
+
+  test('faking clock skew should affect activity order', async () => {
+    await driver.wait(uiConnected(), timeouts.directLine);
+    await pageObjects.sendMessageViaSendBox('card bingsports', { waitForSend: false });
+
+    await driver.executeScript(() => window.WebChatTest.releaseActivity(2));
+
+    await driver.wait(minNumActivitiesShown(2), timeouts.directLine);
+    await driver.wait(allOutgoingActivitiesSent(), timeouts.directLine);
+
+    // Specifically set a large clock skew to make sure the 2nd user-originated activity has a negative clock skew (appears on top).
+    await pageObjects.dispatchAction({
+      payload: { value: -864000000 },
+      type: 'WEB_CHAT/SET_CLOCK_SKEW_ADJUSTMENT'
+    });
+
+    // Make sure the clock skew is set correctly.
+    // If it is not set, the result could be false-positive.
+    await expect(pageObjects.getStore()).resolves.toHaveProperty('clockSkewAdjustment', -864000000);
+
+    await pageObjects.sendMessageViaSendBox(
+      'echo This outgoing activity should be the first in the list before echo back, and last after the echo back.',
+      {
+        waitForSend: false
+      }
+    );
+
+    await driver.wait(minNumActivitiesShown(3), timeouts.directLine);
+
+    const firstActivity = await driver.findElement(By.css('[role="list"] > li:first-child p'));
+
+    await expect(firstActivity.getText()).resolves.toBe(
+      'echo This outgoing activity should be the first in the list before echo back, and last after the echo back.'
+    );
+
+    await driver.executeScript(() => window.WebChatTest.releaseActivity(3));
+    await driver.wait(minNumActivitiesShown(5), timeouts.directLine);
+    await driver.wait(allOutgoingActivitiesSent(), timeouts.directLine);
+    await driver.wait(allImagesLoaded(), timeouts.fetch);
+
+    expect(await driver.takeScreenshot()).toMatchImageSnapshot(imageSnapshotOptions);
+  });
 });

__tests__/disabledUI.js

@@ -1,4 +1,5 @@
 import { imageSnapshotOptions, timeouts } from './constants.json';
+import uiConnected from './setup/conditions/uiConnected.js';
 
 // selenium-webdriver API doc:
 // https://seleniumhq.github.io/selenium/docs/api/javascript/module/selenium-webdriver/index_exports_WebDriver.html
@@ -12,6 +13,8 @@ describe('tests the UI of disabled Web Chat', () => {
 
     const { driver } = await setupWebDriver({ props: { styleOptions, disabled } });
 
+    await driver.wait(uiConnected(), timeouts.directLine);
+
     const base64PNG = await driver.takeScreenshot();
 
     expect(base64PNG).toMatchImageSnapshot(imageSnapshotOptions);

__tests__/offlineUI.js

@@ -384,6 +384,9 @@ describe('offline UI', () => {
     await driver.wait(minNumActivitiesShown(2), timeouts.directLine);
     await driver.wait(actionDispatched('WEB_CHAT/SAGA_ERROR'), timeouts.directLine);
 
+    // Wait until error status come up
+    await driver.wait(async () => /render error/iu.test(await pageObjects.getNotificationText()), timeouts.ui);
+
     const base64PNG = await driver.takeScreenshot();
 
     expect(base64PNG).toMatchImageSnapshot(imageSnapshotOptions);

__tests__/setup/conditions/allImagesLoaded.js

@@ -2,7 +2,7 @@ import { Condition } from 'selenium-webdriver';
 
 export default function allImagesLoaded() {
   return new Condition(
-    'Waiting for all images to be loaded',
+    'all images to be loaded',
     async driver =>
       await driver.executeScript(() => [].every.call(document.querySelectorAll('img'), ({ complete }) => complete))
   );

__tests__/setup/pageObjects/getNotificationText.js

@@ -0,0 +1,9 @@
+import { By } from 'selenium-webdriver';
+
+export default async function getNotificationText(driver) {
+  const notificationTextAriaLabel = driver.findElement(By.css('[role="status"] > [aria-label]'));
+
+  if (notificationTextAriaLabel) {
+    return await notificationTextAriaLabel.getAttribute('innerText');
+  }
+}

__tests__/setup/pageObjects/index.js

@@ -5,6 +5,7 @@ import dispatchAction from './dispatchAction';
 import endSpeechSynthesize from './endSpeechSynthesize';
 import errorSpeechSynthesize from './errorSpeechSynthesize';
 import executePromiseScript from './executePromiseScript';
+import getNotificationText from './getNotificationText';
 import getNumActivitiesShown from './getNumActivitiesShown';
 import getSendBoxText from './getSendBoxText';
 import getStore from './getStore';
@@ -35,6 +36,7 @@ export default function pageObjects(driver) {
       endSpeechSynthesize,
       errorSpeechSynthesize,
       executePromiseScript,
+      getNotificationText,
       getNumActivitiesShown,
       getSendBoxText,
       getStore,

package.json

@@ -76,6 +76,8 @@
     "lerna-publish": "lerna publish",
     "prepublishOnly": "lerna run --scope=botframework-webchat* --scope=playground --stream prepublishOnly",
     "prettier-readmes": "prettier --write **/**/*.md --tab-width 3 --single-quote true",
+    "start:docker": "npm run build && docker-compose up --build",
+    "start:playground": "cd packages/playground && npm run start",
     "test": "jest --no-cache",
     "test:all": "lerna run --parallel --stream test",
     "watch": "lerna run --parallel --scope=botframework-webchat* --stream watch"

packages/bundle/.eslintrc.yml

@@ -4,6 +4,7 @@ extends:
 parser: "@typescript-eslint/parser"
 plugins:
 - prettier
+- react-hooks
 - "@typescript-eslint"
 rules:
   # We will rework on these rules
@@ -95,3 +96,5 @@ rules:
     beforeSelfClosing: always
     closingSlash: never
   react/jsx-wrap-multilines: error
+  react-hooks/rules-of-hooks: error
+  react-hooks/exhaustive-deps: warn

packages/bundle/package.json

@@ -67,6 +67,7 @@
     "concurrently": "^4.1.1",
     "eslint-plugin-prettier": "^3.1.0",
     "eslint-plugin-react": "^7.14.2",
+    "eslint-plugin-react-hooks": "^1.7.0",
     "prettier": "^1.18.2",
     "terser-webpack-plugin": "^1.3.0",
     "typescript": "^3.5.3",

packages/bundle/src/adaptiveCards/Attachment/AdaptiveCardAttachment.js

@@ -1,6 +1,5 @@
-import memoize from 'memoize-one';
 import PropTypes from 'prop-types';
-import React from 'react';
+import React, { useMemo } from 'react';
 
 import AdaptiveCardRenderer from './AdaptiveCardRenderer';
 
@@ -17,11 +16,9 @@ function stripSubmitAction(card) {
   return { ...card, nextActions };
 }
 
-export default class AdaptiveCardAttachment extends React.Component {
-  constructor(props) {
-    super(props);
-
-    this.createAdaptiveCard = memoize((adaptiveCards, content) => {
+const AdaptiveCardAttachment = ({ adaptiveCardHostConfig, adaptiveCards, attachment: { content }, renderMarkdown }) => {
+  const { card } = useMemo(() => {
+    if (content) {
       const card = new adaptiveCards.AdaptiveCard();
       const errors = [];
 
@@ -41,18 +38,19 @@ export default class AdaptiveCardAttachment extends React.Component {
         card,
         errors
       };
-    });
-  }
-
-  render() {
-    const {
-      props: { adaptiveCardHostConfig, adaptiveCards, attachment, renderMarkdown }
-    } = this;
-    const { card } = this.createAdaptiveCard(adaptiveCards, attachment.content, renderMarkdown);
+    }
+  }, [adaptiveCards, content]);
+
+  return (
+    <AdaptiveCardRenderer
+      adaptiveCard={card}
+      adaptiveCardHostConfig={adaptiveCardHostConfig}
+      renderMarkdown={renderMarkdown}
+    />
+  );
+};
 
-    return <AdaptiveCardRenderer adaptiveCard={card} adaptiveCardHostConfig={adaptiveCardHostConfig} />;
-  }
-}
+export default AdaptiveCardAttachment;
 
 AdaptiveCardAttachment.propTypes = {
   // TODO: [P2] We should rename adaptiveCards to adaptiveCardsPolyfill

packages/bundle/src/adaptiveCards/Attachment/AnimationCardAttachment.js

@@ -1,61 +1,31 @@
 /* eslint react/no-array-index-key: "off" */
 
 import { Components, connectToWebChat } from 'botframework-webchat-component';
-import memoize from 'memoize-one';
 import PropTypes from 'prop-types';
 import React from 'react';
 
-import { AdaptiveCardBuilder } from './AdaptiveCardBuilder';
 import CommonCard from './CommonCard';
 
 const { ImageContent, VideoContent } = Components;
 
-class AnimationCardAttachment extends React.Component {
-  constructor(props) {
-    super(props);
-
-    this.buildCard = memoize((adaptiveCards, content, styleOptions) => {
-      const builder = new AdaptiveCardBuilder(adaptiveCards, styleOptions);
-
-      (content.images || []).forEach(image => builder.addImage(image.url, null, image.tap));
-
-      builder.addCommon(content);
-
-      return builder.card;
-    });
-  }
-
-  render() {
-    const {
-      adaptiveCardHostConfig,
-      adaptiveCards,
-      attachment,
-      attachment: { content: { media = [] } = {} } = {},
-      styleSet
-    } = this.props;
-
-    return (
-      <div className={styleSet.animationCardAttachment}>
-        <ul className="media-list">
-          {media.map(({ profile = '', url }, index) => (
-            <li key={index}>
-              {/\.gif$/iu.test(url) ? (
-                <ImageContent alt={profile} src={url} />
-              ) : (
-                <VideoContent alt={profile} src={url} />
-              )}
-            </li>
-          ))}
-        </ul>
-        <CommonCard
-          adaptiveCardHostConfig={adaptiveCardHostConfig}
-          adaptiveCards={adaptiveCards}
-          attachment={attachment}
-        />
-      </div>
-    );
-  }
-}
+const AnimationCardAttachment = ({
+  adaptiveCardHostConfig,
+  adaptiveCards,
+  attachment,
+  attachment: { content: { media = [] } } = {},
+  styleSet
+}) => (
+  <div className={styleSet.animationCardAttachment}>
+    <ul className="media-list">
+      {media.map(({ profile = '', url }, index) => (
+        <li key={index}>
+          {/\.gif$/iu.test(url) ? <ImageContent alt={profile} src={url} /> : <VideoContent alt={profile} src={url} />}
+        </li>
+      ))}
+    </ul>
+    <CommonCard adaptiveCardHostConfig={adaptiveCardHostConfig} adaptiveCards={adaptiveCards} attachment={attachment} />
+  </div>
+);
 
 AnimationCardAttachment.propTypes = {
   adaptiveCardHostConfig: PropTypes.any.isRequired,