Fix Narrator reading out attachments and timestamps in transcript (#2226

) * Fix README sample link * Update Legacy lifecycle methods to UNSAFE_ * fix #2193: Attachments not spoken by Narrator * Add timestamp back to aria-labels * fixes * PR fixes * Apply PR fixes
microsoft · Jul 29, 2019 · f5b8db5 · f5b8db5
1 parent a98f074
commit f5b8db5
Show file tree

Hide file tree

Showing 24 changed files with 169 additions and 87 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -55,7 +55,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 -  Fix [#2160](https://github.com/microsoft/BotFramework-WebChat/issues/2160). Clear suggested actions after clicking on a suggested actions of type `openUrl`, by [@tdurnford](https://github.com/tdurnford) in PR [#2190](https://github.com/microsoft/BotFramework-WebChat/pull/2190)
 -  Fix [#2187](https://github.com/microsoft/BotFramework-WebChat/issues/2187). Bump core-js and update core-js modules on index-es5.js, by [@corinagum](https://github.com/corinagum) in PR [#2195](https://github.com/microsoft/BotFramework-WebChat/pull/2195)
--  Fix [#1954](https://github.com/microsoft/BotFramework-WebChat/issues/1954). Estimate clock skew and adjust timestamp for outgoing activity, by [@compulim](https://github.com/compulim) in PR [#2208](https://github.com/microsoft/BotFramework-WebChat/pull/2208) and [#2230](https://github.com/microsoft/BotFramework-WebChat/pull/2230)
+-  Fix [#1954](https://github.com/microsoft/BotFramework-WebChat/issues/1954). Estimate clock skew and adjust timestamp for outgoing activity, by [@compulim](https://github.com/compulim) in PR [#2208](https://github.com/microsoft/BotFramework-WebChat/pull/2208)
+-  Fix [#2193](https://github.com/microsoft/BotFramework-WebChat/issues/2193). Fix Adaptive Card/attachments do not get read by Narrator, by [@corinagum](https://github.com/corinagum) in PR [#2226](https://github.com/microsoft/BotFramework-WebChat/pull/2226)
+-  Fix [#2150](https://github.com/microsoft/BotFramework-WebChat/issues/2193). Fix timestamps read by Narrator, by [@corinagum](https://github.com/corinagum) in PR [#2226](https://github.com/microsoft/BotFramework-WebChat/pull/2226)
+
 
 ### Added
 

diff --git a/packages/component/src/Activity/Bubble.js b/packages/component/src/Activity/Bubble.js
@@ -36,7 +36,7 @@ const Bubble = ({ 'aria-hidden': ariaHidden, children, className, fromUser, nub,
 );
 
 Bubble.defaultProps = {
-  'aria-hidden': true,
+  'aria-hidden': false,
   children: undefined,
   className: '',
   fromUser: false,

diff --git a/packages/component/src/Activity/CarouselFilmStrip.js b/packages/component/src/Activity/CarouselFilmStrip.js
@@ -130,7 +130,7 @@ const WebChatCarouselFilmStrip = ({
         <ul className={classNames({ webchat__carousel__item_indented: indented })} ref={itemContainerRef}>
           {attachments.map((attachment, index) => (
             <li key={index}>
-              <Bubble fromUser={fromUser} key={index} nub={false}>
+              <Bubble aria-hidden={true} fromUser={fromUser} key={index} nub={false}>
                 {children({ attachment })}
               </Bubble>
             </li>
@@ -140,7 +140,7 @@ const WebChatCarouselFilmStrip = ({
           {state === SENDING || state === SEND_FAILED ? (
             <SendStatus activity={activity} />
           ) : (
-            <Timestamp activity={activity} aria-hidden={true} className={timestampClassName} />
+            <Timestamp activity={activity} className={timestampClassName} />
           )}
         </div>
       </div>

diff --git a/packages/component/src/Activity/SendStatus.js b/packages/component/src/Activity/SendStatus.js
@@ -2,8 +2,8 @@ import { Constants } from 'botframework-webchat-core';
 import PropTypes from 'prop-types';
 import React from 'react';
 
+import { localize } from '../Localization/Localize';
 import connectToWebChat from '../connectToWebChat';
-import Localize, { localize } from '../Localization/Localize';
 
 const {
   ActivityClientState: { SEND_FAILED, SENDING }
@@ -29,31 +29,37 @@ const connectSendStatus = (...selectors) =>
 const SendStatus = ({ activity: { channelData: { state } = {} }, language, retrySend, styleSet }) => {
   // TODO: [P4] Currently, this is the only place which use a templated string
   //       We could refactor this into a general component if there are more templated strings
+  const localizedSending = localize('Sending', language);
+  const localizedSendStatus = localize('SendStatus', language);
   const sendFailedText = localize('SEND_FAILED_KEY', language);
   const sendFailedRetryMatch = /\{Retry\}/u.exec(sendFailedText);
 
   return (
-    <span className={styleSet.sendStatus}>
-      {state === SENDING ? (
-        <Localize text="Sending" />
-      ) : state === SEND_FAILED ? (
-        sendFailedRetryMatch ? (
-          <React.Fragment>
-            {sendFailedText.substr(0, sendFailedRetryMatch.index)}
+    <React.Fragment>
+      {/* Because of differences in browser implementations, <span aria-label> is used to make the screen reader perform the same on different browsers in Edge v44 */}
+      <span aria-label={localizedSendStatus + localizedSending} />
+      <span aria-hidden={true} className={styleSet.sendStatus}>
+        {state === SENDING ? (
+          localizedSending
+        ) : state === SEND_FAILED ? (
+          sendFailedRetryMatch ? (
+            <React.Fragment>
+              {sendFailedText.substr(0, sendFailedRetryMatch.index)}
+              <button onClick={retrySend} type="button">
+                {localize('Retry', language)}
+              </button>
+              {sendFailedText.substr(sendFailedRetryMatch.index + sendFailedRetryMatch[0].length)}
+            </React.Fragment>
+          ) : (
             <button onClick={retrySend} type="button">
-              {localize('Retry', language)}
+              {sendFailedText}
             </button>
-            {sendFailedText.substr(sendFailedRetryMatch.index + sendFailedRetryMatch[0].length)}
-          </React.Fragment>
+          )
         ) : (
-          <button onClick={retrySend} type="button">
-            {sendFailedText}
-          </button>
-        )
-      ) : (
-        false
-      )}
-    </span>
+          false
+        )}
+      </span>
+    </React.Fragment>
   );
 };
 

diff --git a/packages/component/src/Activity/StackedLayout.js b/packages/component/src/Activity/StackedLayout.js
@@ -131,7 +131,9 @@ const StackedLayout = ({ activity, avatarInitials, children, language, styleSet,
           </div>
         ) : (
           !!activityDisplayText && (
-            <div aria-label={ariaLabel} className="webchat__row message">
+            <div className="webchat__row message">
+              {/* Because of differences in browser implementations, <span aria-label> is used to make the screen reader perform the same on different browsers in Edge v44 */}
+              <span aria-label={ariaLabel} />
               <Bubble aria-hidden={true} className="bubble" fromUser={fromUser} nub={true}>
                 {children({
                   activity,
@@ -145,12 +147,16 @@ const StackedLayout = ({ activity, avatarInitials, children, language, styleSet,
             </div>
           )
         )}
+        {/* Because of differences in browser implementations, aria-label=" " is used to make the screen reader not repeat the same text multiple times in Chrome v75 */}
         {attachments.map((attachment, index) => (
           <div
+            aria-label=" "
             className={classNames('webchat__row attachment', { webchat__stacked_item_indented: indented })}
             key={index}
           >
-            <Bubble aria-hidden={true} className="attachment bubble" fromUser={fromUser} key={index} nub={false}>
+            {/* Because of differences in browser implementations, <span aria-label> is used to make the screen reader perform the same on different browsers in Edge v44 */}
+            <span aria-label={fromUser ? localize('UserSent', language) : localize('BotSent', language)} />
+            <Bubble className="attachment bubble" fromUser={fromUser} key={index} nub={false}>
               {children({ attachment })}
             </Bubble>
           </div>
@@ -159,7 +165,7 @@ const StackedLayout = ({ activity, avatarInitials, children, language, styleSet,
           {showSendStatus ? (
             <SendStatus activity={activity} className="timestamp" />
           ) : (
-            <Timestamp activity={activity} aria-hidden={true} className={classNames('timestamp', timestampClassName)} />
+            <Timestamp activity={activity} className={classNames('timestamp', timestampClassName)} />
           )}
           <div className="filler" />
         </div>

diff --git a/packages/component/src/Activity/Timestamp.js b/packages/component/src/Activity/Timestamp.js
@@ -5,22 +5,20 @@ import React from 'react';
 import connectToWebChat from '../connectToWebChat';
 import TimeAgo from '../Utils/TimeAgo';
 
-const Timestamp = ({ activity: { timestamp }, 'aria-hidden': ariaHidden, className, styleSet }) => (
-  <span aria-hidden={ariaHidden} className={classNames(styleSet.timestamp + '', className + '')}>
+const Timestamp = ({ activity: { timestamp }, className, styleSet }) => (
+  <span className={classNames(styleSet.timestamp + '', (className || '') + '')}>
     <TimeAgo value={timestamp} />
   </span>
 );
 
 Timestamp.defaultProps = {
-  'aria-hidden': true,
   className: ''
 };
 
 Timestamp.propTypes = {
   activity: PropTypes.shape({
     timestamp: PropTypes.string.isRequired
   }).isRequired,
-  'aria-hidden': PropTypes.bool,
   className: PropTypes.string,
   styleSet: PropTypes.shape({
     timestamp: PropTypes.any.isRequired

diff --git a/packages/component/src/Attachment/Assets/DownloadIcon.js b/packages/component/src/Attachment/Assets/DownloadIcon.js
@@ -3,27 +3,19 @@ import React from 'react';
 
 const ICON_SIZE_FACTOR = 22;
 
-const DownloadIcon = ({ className, label, size }) => (
-  <svg
-    aria-label={label}
-    className={className}
-    height={ICON_SIZE_FACTOR * size}
-    viewBox="0 0 31.8 46"
-    width={ICON_SIZE_FACTOR * size}
-  >
+const DownloadIcon = ({ className, size }) => (
+  <svg className={className} height={ICON_SIZE_FACTOR * size} viewBox="0 0 31.8 46" width={ICON_SIZE_FACTOR * size}>
     <path d="M26.8,23.8l-10.9,11L5,23.8l1.6-1.6l8.2,8.3V5H17v25.5l8.2-8.3L26.8,23.8z M5.8,41v-2.2H26V41H5.8z" />
   </svg>
 );
 
 DownloadIcon.defaultProps = {
   className: '',
-  label: '',
   size: 1
 };
 
 DownloadIcon.propTypes = {
   className: PropTypes.string,
-  label: PropTypes.string,
   size: PropTypes.number
 };
 

diff --git a/packages/component/src/Attachment/Assets/TypingAnimation.js b/packages/component/src/Attachment/Assets/TypingAnimation.js
@@ -3,9 +3,15 @@ import React from 'react';
 
 import connectToWebChat from '../../connectToWebChat';
 
-const TypingAnimation = ({ styleSet }) => <div className={styleSet.typingAnimation} />;
+const TypingAnimation = ({ 'aria-label': ariaLabel, styleSet }) => (
+  <React.Fragment>
+    <span aria-label={ariaLabel} />
+    <div aria-hidden={true} className={styleSet.typingAnimation} />
+  </React.Fragment>
+);
 
 TypingAnimation.propTypes = {
+  'aria-label': PropTypes.string.isRequired,
   styleSet: PropTypes.shape({
     typingAnimation: PropTypes.any.isRequired
   }).isRequired

diff --git a/packages/component/src/Attachment/AudioContent.js b/packages/component/src/Attachment/AudioContent.js
@@ -3,17 +3,27 @@ import React from 'react';
 
 import connectToWebChat from '../connectToWebChat';
 
-const AudioContent = ({ autoPlay, loop, poster, src, styleSet }) => (
-  <audio autoPlay={autoPlay} className={styleSet.audioContent} controls={true} loop={loop} poster={poster} src={src} />
+const AudioContent = ({ alt, autoPlay, loop, poster, src, styleSet }) => (
+  <audio
+    aria-label={alt}
+    autoPlay={autoPlay}
+    className={styleSet.audioContent}
+    controls={true}
+    loop={loop}
+    poster={poster}
+    src={src}
+  />
 );
 
 AudioContent.defaultProps = {
+  alt: '',
   autoPlay: false,
   loop: false,
   poster: ''
 };
 
 AudioContent.propTypes = {
+  alt: PropTypes.string,
   autoPlay: PropTypes.bool,
   loop: PropTypes.bool,
   poster: PropTypes.string,

diff --git a/packages/component/src/Attachment/DownloadAttachment.js b/packages/component/src/Attachment/DownloadAttachment.js
@@ -13,19 +13,30 @@ const DownloadAttachment = ({
   styleSet
 }) => {
   const attachmentIndex = attachments.indexOf(attachment);
-  const label = localize('Download file', language);
+  const downloadLabel = localize('Download file', language);
   const size = attachmentSizes[attachmentIndex];
-
+  const formattedSize = typeof size === 'number' && format(size);
+  const downloadFileWithFileSizeLabel = localize(
+    'DownloadFileWithFileSize',
+    language,
+    downloadLabel,
+    attachment.name,
+    formattedSize
+  );
   return (
-    <div className={styleSet.downloadAttachment}>
-      <a href={attachment.contentUrl} rel="noopener noreferrer" target="_blank">
-        <div className="details">
-          <div className="name">{attachment.name}</div>
-          {typeof size === 'number' && <div className="size">{format(size)}</div>}
-        </div>
-        <DownloadIcon className="icon" label={label} size={1.5} />
-      </a>
-    </div>
+    <React.Fragment>
+      {/* Because of differences in browser implementations, <span aria-label> is used to make the screen reader perform the same on different browsers in Edge v44 */}
+      <span aria-label={downloadFileWithFileSizeLabel} />
+      <div aria-hidden={true} className={styleSet.downloadAttachment}>
+        <a href={attachment.contentUrl} rel="noopener noreferrer" target="_blank">
+          <div className="details">
+            <div className="name">{attachment.name}</div>
+            <div className="size">{formattedSize}</div>
+          </div>
+          <DownloadIcon className="icon" size={1.5} />
+        </a>
+      </div>
+    </React.Fragment>
   );
 };
 

diff --git a/packages/component/src/Attachment/TextContent.js b/packages/component/src/Attachment/TextContent.js
@@ -10,15 +10,24 @@ import connectToWebChat from '../connectToWebChat';
 
 const TextContent = ({ contentType, renderMarkdown, styleSet, text }) =>
   contentType === 'text/markdown' && renderMarkdown ? (
-    <div
-      className={classNames('markdown', styleSet.textContent + '')}
-      dangerouslySetInnerHTML={{ __html: renderMarkdown(text || '') }}
-    />
+    <React.Fragment>
+      {/* Because of differences in browser implementations, <span aria-label> is used to make the screen reader perform the same on different browsers in Edge v44 */}
+      <span aria-label={text} />
+      <div
+        aria-hidden={true}
+        className={classNames('markdown', styleSet.textContent + '')}
+        dangerouslySetInnerHTML={{ __html: renderMarkdown(text || '') }}
+      />
+    </React.Fragment>
   ) : (
     (text || '').split('\n').map((line, index) => (
-      <p className={classNames('plain', styleSet.textContent + '')} key={index}>
-        {line.trim()}
-      </p>
+      <React.Fragment key={index}>
+        {/* Because of differences in browser implementations, <span aria-label> is used to make the screen reader perform the same on different browsers in Edge v44 */}
+        <span aria-label={text} />
+        <p aria-hidden={true} className={classNames('plain', styleSet.textContent + '')}>
+          {line.trim()}
+        </p>
+      </React.Fragment>
     ))
   );
 

diff --git a/packages/component/src/Attachment/TypingActivity.js b/packages/component/src/Attachment/TypingActivity.js
@@ -1,16 +1,18 @@
 import PropTypes from 'prop-types';
 import React from 'react';
 
+import { localize } from '../Localization/Localize';
 import connectToWebChat from '../connectToWebChat';
 import TypingAnimation from './Assets/TypingAnimation';
 
-const TypingActivity = ({ styleSet }) => (
+const TypingActivity = ({ language, styleSet }) => (
   <div className={styleSet.typingActivity}>
-    <TypingAnimation />
+    <TypingAnimation ariaLabel={localize('TypingIndicator', language)} />
   </div>
 );
 
 TypingActivity.propTypes = {
+  language: PropTypes.string.isRequired,
   styleSet: PropTypes.shape({
     typingActivity: PropTypes.any.isRequired
   }).isRequired

diff --git a/packages/component/src/BasicTranscript.js b/packages/component/src/BasicTranscript.js
@@ -95,6 +95,8 @@ const BasicTranscript = ({
           >
             {activityElements.map(({ activity, element }, index) => (
               <li
+                /* Because of differences in browser implementations, aria-label=" " is used to make the screen reader not repeat the same text multiple times in Chrome v75 */
+                aria-label=" "
                 className={classNames(styleSet.activity + '', {
                   // Hide timestamp if same timestamp group with the next activity
                   'hide-timestamp': sameTimestampGroup(

diff --git a/packages/component/src/BasicWebChat.js b/packages/component/src/BasicWebChat.js
@@ -1,4 +1,5 @@
 /* eslint no-magic-numbers: ["error", { "ignore": [0, 1, 2] }] */
+/* eslint react/no-unsafe: off */
 
 import { css } from 'glamor';
 import classNames from 'classnames';
@@ -80,7 +81,7 @@ export default class BasicWebChat extends React.Component {
   }
 
   // TODO: [P2] Move to React 16 APIs
-  componentWillReceiveProps({
+  UNSAFE_componentWillReceiveProps({
     activityMiddleware: nextActivityMiddleware,
     attachmentMiddleware: nextAttachmentMiddleware
   }) {

diff --git a/packages/component/src/Composer.js b/packages/component/src/Composer.js
@@ -1,3 +1,5 @@
+/* eslint react/no-unsafe: off */
+
 import {
   Composer as ScrollToBottomComposer,
   FunctionContext as ScrollToBottomFunctionContext
@@ -205,7 +207,7 @@ class Composer extends React.Component {
     };
   }
 
-  componentWillMount() {
+  UNSAFE_componentWillMount() {
     const { props } = this;
     const { directLine, userID, username } = props;