#3625 Update 'no screen reader for custom activity middleware' warning

.github/CONTRIBUTING.md

@@ -117,7 +117,6 @@ If your development box has less than 4 cores, you will need to reduce the numbe
 
 Our CI pipeline run tests with 4 agents simultaneously. If new tests are added, please make sure they can run simultaneously.
 
-
 ### Troubleshooting the test suite
 
 We run test suite on every commit and requires 100% test pass. If the test suite did not complete successfully, they are likely:

.github/ISSUE_TEMPLATE/question-template.md

@@ -20,5 +20,4 @@ This repo focuses on the development of Web Chat, a client/channel for Bot Frame
 | Bot Framework Questions     | Ask implementation questions related to the BotFramework SDK | https://stackoverflow.com/questions/tagged/botframework |
 | Bot Builder                 | A comprehensive list of Bot Framework SDKs and tools         | https://github.com/microsoft/BotBuilder                 |
 
-
 [Question]

CHANGELOG.md

@@ -43,6 +43,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 -  Fixes [#3672](https://github.com/microsoft/BotFramework-WebChat/issues/3672). Center the icon of send box buttons vertically and horizontally, by [@compulim](https://github.com/compulim) in PR [#3673](https://github.com/microsoft/BotFramework-WebChat/pull/3673)
 -  Fixes [#3683](https://github.com/microsoft/BotFramework-WebChat/issues/3683). Activities should be acknowledged when user scrolls to bottom, by [@compulim](https://github.com/compulim) in PR [#3684](https://github.com/microsoft/BotFramework-WebChat/pull/3684)
 -  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)
 
 ### Changed
 
@@ -96,8 +97,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 ### Samples
 
 -  Fixes [#3473](https://github.com/microsoft/BotFramework-WebChat/issues/3473). Fix samples using activityMiddleware (from 4.10.0 breaking changes), by [@corinagum](https://github.com/corinagum) in PR [#3601](https://github.com/microsoft/BotFramework-WebChat/pull/3601)
--  Fixes [#3434](https://github.com/microsoft/BotFramework-WebChat/issues/3434). Dispatched event, postBack, or messageBack + activityMiddleware causes fatal error, by [@amal-khalaf](https://github.com/amal-khalaf) in PR [#3671](https://github.com/microsoft/BotFramework-WebChat/pull/3671)
 -  Fixes [#3582](https://github.com/microsoft/BotFramework-WebChat/issues/3582). Fix Disable Adaptive Cards sample, by [@corinagum](https://github.com/corinagum) in PR [#3687](https://github.com/microsoft/BotFramework-WebChat/pull/3687)
+-  Fixes [#3434](https://github.com/microsoft/BotFramework-WebChat/issues/3434). Dispatched event, postBack, or messageBack + activityMiddleware causes fatal error, by [@amal-khalaf](https://github.com/amal-khalaf) in PR [#3671](https://github.com/microsoft/BotFramework-WebChat/pull/3671)
 -  Fixes [#3215](https://github.com/microsoft/BotFramework-WebChat/issues/3215). Fix SSO samples `window.opener.postMessage`, by [@corinagum](https://github.com/corinagum) in PR [#3696](https://github.com/microsoft/BotFramework-WebChat/pull/3696)
 
 ## [4.11.0] - 2020-11-04

README.md

@@ -19,8 +19,8 @@ This repo is part of the [Microsoft Bot Framework](https://github.com/microsoft/
 > This section points out important version notes. For further information, please see the related links and check the [`CHANGELOG.md`](https://github.com/microsoft/BotFramework-WebChat/blob/master/CHANGELOG.md)
 
 ## API refactor into new package in Web Chat 4.11.0
-The Web Chat API has been refactored into a separate package. To learn more, check out the [API refactor summary](https://github.com/microsoft/BotFramework-WebChat/pull/3543).
 
+The Web Chat API has been refactored into a separate package. To learn more, check out the [API refactor summary](https://github.com/microsoft/BotFramework-WebChat/pull/3543).
 
 ## Direct Line Speech support in Web Chat 4.7.0
 

__tests__/adaptiveCards.js

@@ -1,5 +1,6 @@
-import { imageSnapshotOptions, timeouts } from './constants.json';
+import { logging } from 'selenium-webdriver';
 
+import { imageSnapshotOptions, timeouts } from './constants.json';
 import allImagesLoaded from './setup/conditions/allImagesLoaded';
 import minNumActivitiesShown from './setup/conditions/minNumActivitiesShown';
 import scrollToBottomCompleted from './setup/conditions/scrollToBottomCompleted';
@@ -152,3 +153,17 @@ test('broken card of invalid version', async () => {
 
   expect(base64PNG).toMatchImageSnapshot(imageSnapshotOptions);
 });
+
+test('unknown card', async () => {
+  const { driver, pageObjects } = await setupWebDriver();
+
+  await driver.wait(uiConnected(), timeouts.directLine);
+  await pageObjects.sendMessageViaSendBox('card unknown', { waitForSend: true });
+
+  await driver.wait(minNumActivitiesShown(2), timeouts.directLine);
+
+  const browserConsoleErrors = await driver.manage().logs().get(logging.Type.BROWSER);
+
+  expect(browserConsoleErrors[0].level.name_).toEqual('WARNING');
+  expect(browserConsoleErrors[0].message).toContain('No renderer for attachment for screen reader of type');
+});

docs/ACCESSIBILITY.md

@@ -187,7 +187,7 @@ To make the live region more consistent across browsers and easier to control, w
       -  0-100 ms: Chrome and TalkBack on Android may miss some of the activities
       -  The development team settled on using one second after some experimentation
 
-## Do and don't
+## Do's and don't
 
 ### Do
 
@@ -211,6 +211,22 @@ To make the live region more consistent across browsers and easier to control, w
    -  It is okay to disable all buttons, as long as the answer will be read by the screen reader
    -  Related to [#3135](https://github.com/microsoft/BotFramework-WebChat/issues/3135)
 -  Don't move focus when an activity arrives (or asynchronously)
-   -  Screen reader reading will be interrupted when focus change
+   -  Screen reader reading will be interrupted when focus changes
    -  Only change focus synchronous to user gesture
    -  Related to [#3135](https://github.com/microsoft/BotFramework-WebChat/issues/3135)
+
+# Screen reader renderer for custom activities and attachments
+
+Web Chat render components are accompanied by a screen reader renderer to maximize accessibility. In the case of custom components, the bot/Web Chat developer will need to implement a screen reader renderer for the equivalent custom visual component.
+
+![image: Console warning: "No renderer for attachment for screen reader of type "application/mnd.microsoft.card.adaptive"](https://user-images.githubusercontent.com/14900841/106323546-6f47ed80-622c-11eb-96d7-de6f72818525.png)
+
+The Web Chat team **DOES NOT** recommend disabling warning messages regarding screen readers and accessibility. However, if the developer decides to suppress these messages, it can be done by adding the following code to `attachmentForScreenReaderMiddleware` in the `Composer` props.
+
+```js
+const attachmentForScreenReaderMiddleware = () => next => () => {
+   return false;
+};
+```
+
+This will prevent the screen reader renderer warning from appearing in the browser console.

lint-staged.config.js

@@ -6,8 +6,10 @@ function prettierMarkdown(filenames) {
   return filenames.map(filename => `prettier --write ${filename} --tab-width 3 --single-quote true`);
 }
 
+// eslint-disable-next-line no-undef
 module.exports = {
-  '{__tests__,samples}/**/*.{html,js,jsx,ts,tsx}': prettierCode,
+  '{__tests__,docs,samples}/**/*.{html,js,jsx,ts,tsx}': prettierCode,
   '**/*.md': prettierMarkdown,
-  'packages/{bundle,component,core,embed,playground}/src/**/*.{js,jsx,ts,tsx}': prettierCode
+  'packages/{api,bundle,component,core,directlinespeech,playground}/src/**/*.{js,jsx,ts,tsx}': prettierCode,
+  '*.{js,jsx,ts,tsx}': 'npm run eslint'
 };

package.json

@@ -17,17 +17,15 @@
   "homepage": "https://github.com/microsoft/BotFramework-WebChat#readme",
   "husky": {
     "hooks": {
-      "pre-commit": "lint-staged --no-stash"
+      "pre-commit": "lint-staged --no-stash --config lint-staged.config.js"
     }
   },
   "keywords": [],
   "scripts": {
     "bootstrap": "lerna bootstrap --ci",
     "build": "lerna run --ignore playground --stream build",
     "coveralls": "cat ./coverage/lcov.info | coveralls",
-    "eslint": "lerna run --parallel --stream eslint",
-    "posteslint": "npm run prettier-readmes",
-    "prettier-readmes": "prettier --write **/**/*.md --tab-width 3 --single-quote true",
+    "eslint": "lerna run --parallel --stream eslint -- ",
     "start": "concurrently --kill-others --raw \"serve\" \"serve -p 5001 -c serve-test.json\" \"lerna run --ignore playground --parallel --stream start\"",
     "start:docker": "npm run start:docker:build && npm run start:docker:up",
     "start:docker:build": "docker-compose -f docker-compose-wsl2.yml build --parallel",
@@ -75,4 +73,4 @@
     "xmlbuilder": "^15.1.1"
   },
   "dependencies": {}
-}
+}

packages/api/package.json

@@ -21,7 +21,7 @@
     "build:babel": "babel src --copy-files --extensions .js,.ts,.tsx --ignore **/*.spec.js,**/*.spec.ts,**/*.spec.tsx,**/*.test.js,**/*.test.ts,**/*.test.tsx,__tests__/**/*.js,__tests__/**/*.ts,__tests__/**/*.tsx --no-copy-ignored --out-dir lib --verbose",
     "build:globalize": "node scripts/createPrecompiledGlobalize.js",
     "build:typescript": "tsc --project src/tsconfig.json",
-    "eslint": "eslint src/**/*.js src/**/*.ts",
+    "eslint": "eslint src/**/*.js src/**/*.ts --fix",
     "prestart": "npm run build:babel",
     "start": "concurrently --kill-others --names \"babel,globalize,tsc\" \"npm run start:babel\" \"npm run start:globalize\" \"npm run start:typescript\"",
     "start:babel": "npm run build:babel -- --skip-initial-build --watch",

packages/api/src/hooks/Composer.js

@@ -92,10 +92,11 @@ function createCardActionContext({ cardActionMiddleware, directLine, dispatch })
                 const { value } = cardAction;
 
                 if (directLine.getSessionId) {
-                  // TODO: [P3] We should change this one to async/await.
-                  //       This is the first place in this project to use async.
-                  //       Thus, we need to add @babel/plugin-transform-runtime and @babel/runtime.
-
+                  /**
+                   * @todo TODO: [P3] We should change this one to async/await.
+                   *       This is the first place in this project to use async.
+                   *       Thus, we need to add @babel/plugin-transform-runtime and @babel/runtime.
+                   */
                   return observableToPromise(directLine.getSessionId()).then(
                     sessionId => `${value}${encodeURIComponent(`&code_challenge=${sessionId}`)}`
                   );
@@ -211,7 +212,9 @@ const Composer = ({
     );
 
     return () => {
-      // TODO: [P3] disconnect() is an async call (pending -> fulfilled), we need to wait, or change it to reconnect()
+      /**
+       * @todo TODO: [P3] disconnect() is an async call (pending -> fulfilled), we need to wait, or change it to reconnect()
+       */
       dispatch(disconnect());
     };
   }, [dispatch, directLine, userID, username]);
@@ -319,12 +322,18 @@ const Composer = ({
         'attachment for screen reader',
         { strict: true },
         ...singleToArray(attachmentForScreenReaderMiddleware),
-        () => () => ({ attachment }) => () => {
+        () => () => ({ attachment }) => {
           if (attachment) {
-            throw new Error(`No renderer for attachment for screen reader of type "${attachment.contentType}"`);
-          } else {
-            throw new Error('No attachment to render');
+            console.warn(`No renderer for attachment for screen reader of type "${attachment.contentType}"`);
+            return false;
           }
+
+          return () => {
+            /**
+             * @todo TODO: [P4] Might be able to throw without returning a function -- investigate and possibly fix
+             */
+            throw new Error('No attachment to render');
+          };
         }
       )({}),
     [attachmentForScreenReaderMiddleware]
@@ -403,17 +412,17 @@ const Composer = ({
     );
   }, [typingIndicatorMiddleware, typingIndicatorRenderer]);
 
-  // This is a heavy function, and it is expected to be only called when there is a need to recreate business logic, e.g.
-  // - User ID changed, causing all send* functions to be updated
-  // - send
-
-  // TODO: [P3] We should think about if we allow the user to change onSendBoxValueChanged/sendBoxValue, e.g.
-  // 1. Turns text into UPPERCASE
-  // 2. Filter out profanity
-
-  // TODO: [P4] Revisit all members of context
-  //       This context should consist of members that are not in the Redux store
-  //       i.e. members that are not interested in other types of UIs
+  /**
+   * This is a heavy function, and it is expected to be only called when there is a need to recreate business logic, e.g.
+   * - User ID changed, causing all send* functions to be updated
+   * - send
+   * @todo TODO: [P3] We should think about if we allow the user to change onSendBoxValueChanged/sendBoxValue, e.g.
+   * 1. Turns text into UPPERCASE
+   * 2. Filter out profanity
+   * @todo TODO: [P4] Revisit all members of context
+   *       This context should consist of members that are not in the Redux store
+   *       i.e. members that are not interested in other types of UIs
+   */
   const context = useMemo(
     () => ({
       ...cardActionContext,
@@ -526,11 +535,12 @@ ComposeWithStore.propTypes = {
 
 export default ComposeWithStore;
 
-// TODO: [P3] We should consider moving some data from Redux store to props
-//       Although we use `connectToWebChat` to hide the details of accessor of Redux store,
-//       we should clean up the responsibility between Context and Redux store
-//       We should decide which data is needed for React but not in other environment such as CLI/VSCode
-
+/**
+ * @todo TODO: [P3] We should consider moving some data from Redux store to props
+ *       Although we use `connectToWebChat` to hide the details of accessor of Redux store,
+ *       we should clean up the responsibility between Context and Redux store
+ *       We should decide which data is needed for React but not in other environment such as CLI/VSCode
+ */
 Composer.defaultProps = {
   activityMiddleware: undefined,
   activityRenderer: undefined,

packages/api/src/hooks/internal/ErrorBox.js

@@ -1,3 +1,4 @@
+/* eslint-disable no-console */
 import { createElement, useEffect } from 'react';
 import PropTypes from 'prop-types';
 

packages/api/src/hooks/middleware/applyMiddleware.js

@@ -10,17 +10,21 @@ export default function applyMiddleware(type, ...middleware) {
       throw new Error(`reached terminator of ${type}`);
     });
 }
-
+/**
+ *
+ * @param {string} type Required. String equivalent of type of container to be rendered.
+ * @param { strict = false } - Used to enforce new middleware format which cooperates with new activity grouping.
+ * @see See {@link https://github.com/microsoft/BotFramework-WebChat/blob/master/CHANGELOG.md#4100---2020-08-18} and {@link https://github.com/microsoft/BotFramework-WebChat/pull/3365} for middleware breaking changes.
+ * @param  {middleware[]} middleware list of middleware to be applied.
+ * 'createRendererArgs' is "what to render"; for example, an activity.
+ * @returns  Returns a function if there is a renderer *committed* to render OR returns false if nothing should be rendered.
+ */
 export function forRenderer(type, { strict = false } = {}, ...middleware) {
   return (...setupArgs) => {
     const runMiddleware = concatMiddleware(...middleware)(...setupArgs)(() => (
       <ErrorBox error={new Error(`reached terminator of ${type}`)} type={type} />
     ));
 
-    // The createRendererArgs is "what to render", for example, activity.
-    // The function should return with only one of the two results:
-    // - Returns a function if there is a renderer *committed* to render;
-    // - Returns false if nothing should be rendered.
     return (...createRendererArgs) => {
       try {
         const render = runMiddleware(...createRendererArgs);

packages/api/src/hooks/utils/ErrorBoundary.js

@@ -11,13 +11,18 @@ class ErrorBoundary extends Component {
   }
 
   componentDidCatch(error) {
+    const { onError } = this.props;
+
     this.setState({ hasError: true });
 
-    this.props.onError(error);
+    onError(error);
   }
 
   render() {
-    return !this.state.hasError && <RenderChildrenFunction>{this.props.children}</RenderChildrenFunction>;
+    const { children } = this.props;
+    const { hasError } = this.state;
+
+    return !hasError && <RenderChildrenFunction>{children}</RenderChildrenFunction>;
   }
 }
 

packages/api/src/patchStyleOptions.js

@@ -22,6 +22,7 @@ function parseBorder(border) {
 
 const PIXEL_UNIT_PATTERN = /^\d+px$/u;
 
+// eslint-disable-next-line complexity
 export default function patchStyleOptions(
   options,
   { groupTimestamp: groupTimestampFromProps, sendTimeout: sendTimeoutFromProps }

packages/bundle/package.json

@@ -21,7 +21,7 @@
     "build:babel": "babel src --extensions .js,.ts,.tsx --ignore **/*.spec.js,**/*.spec.ts,**/*.spec.tsx,**/*.test.js,**/*.test.ts,**/*.test.tsx,__tests__/**/*.js,__tests__/**/*.ts,__tests__/**/*.tsx --out-dir lib --verbose",
     "build:typescript": "tsc --project src/tsconfig.json",
     "build:webpack": "webpack-cli",
-    "eslint": "eslint src/**/*.js src/**/*.ts",
+    "eslint": "eslint src/**/*.js src/**/*.ts --fix",
     "prestart": "npm run build:babel",
     "start": "concurrently --kill-others --names \"babel,tsc,webpack\" \"npm run start:babel\" \"npm run start:typescript\" \"npm run start:webpack\"",
     "start:babel": "npm run build:babel -- --skip-initial-build --watch",

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

@@ -11,6 +11,7 @@ import useAdaptiveCardsPackage from '../hooks/useAdaptiveCardsPackage';
 const { ErrorBox } = Components;
 const { useDisabled, useLocalizer, usePerformCardAction, useRenderMarkdownAsHTML, useScrollToEnd, useStyleSet } = hooks;
 
+// eslint-disable-next-line no-undef
 const node_env = process.env.node_env || process.env.NODE_ENV;
 
 function addClass(element, className) {

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

@@ -1,3 +1,5 @@
+/* eslint-disable react/no-array-index-key */
+/* eslint-disable react/forbid-dom-props */
 import { hooks } from 'botframework-webchat-component';
 import PropTypes from 'prop-types';
 import React, { useMemo } from 'react';
@@ -50,8 +52,9 @@ AdaptiveCardChoiceSetInput.propTypes = {
         value: PropTypes.any
       })
     ),
+    defaultValue: PropTypes.any,
     value: PropTypes.any
-  })
+  }).isRequired
 };
 
 const AdaptiveCardAttachment = ({ content }) => {
@@ -93,7 +96,18 @@ const AdaptiveCardAttachment = ({ content }) => {
     });
 
     return inputs;
-  }, [card]);
+  }, [
+    card,
+    ChoiceSetInput,
+    DateInput,
+    NumberInput,
+    OpenUrlAction,
+    ShowCardAction,
+    SubmitAction,
+    TextInput,
+    TimeInput,
+    ToggleInput
+  ]);
 
   const cardLabel = localize('ATTACHMENT_CARD', card.speak || '', '', '');
 

packages/bundle/src/adaptiveCards/AttachmentForScreenReader/RichCardAttachment.js

@@ -1,3 +1,5 @@
+/* eslint-disable react/forbid-dom-props */
+/* eslint-disable react/no-array-index-key */
 import PropTypes from 'prop-types';
 import React from 'react';
 import { hooks } from 'botframework-webchat-component';

packages/bundle/src/createCognitiveServicesSpeechServicesPonyfillFactory.js

@@ -69,6 +69,7 @@ export default function createCognitiveServicesSpeechServicesPonyfillFactory({
     //             We will not need this code when using Speech SDK 1.14.0 or up.
     // TODO: [P1] #3575 Remove the following lines when bumping to Speech SDK 1.14.0 or higher
     source.createAudioContext = () => {
+      // eslint-disable-next-line no-extra-boolean-cast
       if (!!source.privContext) {
         return;
       }

packages/bundle/src/createCognitiveServicesSpeechServicesPonyfillFactory.spec.js

@@ -1,3 +1,4 @@
+/* eslint-disable no-global-assign */
 let consoleWarns;
 let createCognitiveServicesSpeechServicesPonyfillFactory;
 let createPonyfill;