From 71f675880244f07ecdc612baa379503dbe4d4e55 Mon Sep 17 00:00:00 2001
From: Corina <v-corgum@microsoft.com>
Date: Thu, 20 Dec 2018 17:18:04 -0800
Subject: [PATCH] #1499 Fix screen reader handling of name, activity, &
 timestamp

---
 packages/component/src/Activity/Avatar.js     |  2 +
 packages/component/src/Activity/Bubble.js     |  2 +
 .../src/Activity/CarouselFilmStrip.js         | 48 +++++++++++++++----
 packages/component/src/Activity/SendStatus.js |  2 +-
 .../component/src/Activity/StackedLayout.js   | 43 +++++++++++++----
 packages/component/src/BasicTranscript.js     |  6 +--
 .../component/src/Localization/Localize.js    |  6 +--
 packages/component/src/Localization/en-US.js  | 10 ++++
 8 files changed, 94 insertions(+), 25 deletions(-)

diff --git a/packages/component/src/Activity/Avatar.js b/packages/component/src/Activity/Avatar.js
index 963789a98c..fa0fdac0a2 100644
--- a/packages/component/src/Activity/Avatar.js
+++ b/packages/component/src/Activity/Avatar.js
@@ -25,6 +25,7 @@ const connectAvatar = (...selectors) => connectToWebChat(
 //       We have 2 different upstreamers <CarouselFilmStrip> and <StackedLayout>
 
 const Avatar = ({
+  'aria-hidden': ariaHidden,
   avatarImage,
   avatarInitials,
   className,
@@ -33,6 +34,7 @@ const Avatar = ({
 }) =>
   !!(avatarImage || avatarInitials) &&
     <div
+      aria-hidden={ ariaHidden }
       className={ classNames(
         styleSet.avatar + '',
         { 'from-user': fromUser },
diff --git a/packages/component/src/Activity/Bubble.js b/packages/component/src/Activity/Bubble.js
index 783553aca1..01c9c155c6 100644
--- a/packages/component/src/Activity/Bubble.js
+++ b/packages/component/src/Activity/Bubble.js
@@ -7,12 +7,14 @@ export default connectToWebChat(
   ({ styleSet }) => ({ styleSet })
 )(
   ({
+    'aria-label': ariaLabel,
     children,
     className,
     fromUser,
     styleSet
   }) =>
     <div
+      aria-label={ ariaLabel }
       className={ classNames(
         styleSet.bubble + '',
         { 'from-user': fromUser },
diff --git a/packages/component/src/Activity/CarouselFilmStrip.js b/packages/component/src/Activity/CarouselFilmStrip.js
index 3158fb45d7..1ca541cef5 100644
--- a/packages/component/src/Activity/CarouselFilmStrip.js
+++ b/packages/component/src/Activity/CarouselFilmStrip.js
@@ -5,13 +5,14 @@ import React from 'react';
 
 import { Constants } from 'botframework-webchat-core';
 
+import { localize } from '../Localization/Localize';
 import Avatar from './Avatar';
 import Bubble from './Bubble';
+import connectToWebChat from '../connectToWebChat';
 import SendStatus from './SendStatus';
+import textFormatToContentType from '../Utils/textFormatToContentType';
 import Timestamp from './Timestamp';
 
-import connectToWebChat from '../connectToWebChat';
-import textFormatToContentType from '../Utils/textFormatToContentType';
 
 const { ActivityClientState: { SENDING, SEND_FAILED } } = Constants;
 
@@ -62,22 +63,44 @@ const ROOT_CSS = css({
 });
 
 const connectCarouselFilmStrip = (...selectors) => connectToWebChat(
-  ({ language }) => ({ language }),
+  ({
+    language,
+    styleSet: {
+      options: {
+        botAvatarInitials,
+        userAvatarInitials
+      }
+    }
+  }, { activity }) => ({
+      avatarInitials: activity.from && activity.from.role === 'user' ? userAvatarInitials : botAvatarInitials,
+      language
+    }),
   ...selectors
 )
 
 const ConnectedCarouselFilmStrip = connectCarouselFilmStrip(
-  ({ styleSet }) => ({ styleSet })
+  ({
+    avatarInitials,
+    language,
+    styleSet
+  }) => ({
+    avatarInitials,
+    language,
+    styleSet
+  })
 )(
   ({
     activity,
+    avatarInitials,
     children,
+    language,
     className,
     filmContext,
     showTimestamp,
     styleSet,
   }) => {
     const fromUser = activity.from.role === 'user';
+    const ariaLabel = localize('Bot said something', language, avatarInitials, activity.text, activity.timestamp)
 
     return (
       <div
@@ -88,12 +111,17 @@ const ConnectedCarouselFilmStrip = connectCarouselFilmStrip(
         ) }
         ref={ filmContext._setFilmStripRef }
       >
-        <Avatar className="avatar" fromUser={ fromUser } />
+        <Avatar
+          aria-hidden={ true }
+          className="avatar"
+          fromUser={ fromUser }
+        />
         <div className="content">
           {
             !!activity.text &&
               <div className="message">
                 <Bubble
+                  aria-label={ ariaLabel }
                   className="bubble"
                   fromUser={ fromUser }
                 >
@@ -122,8 +150,11 @@ const ConnectedCarouselFilmStrip = connectCarouselFilmStrip(
               )
             }
           </ul>
-          {
-            (
+          <div
+            aria-hidden={ true }
+            className="row"
+          >
+            {(
               activity.channelData
               && (
                 activity.channelData.state === SENDING
@@ -133,7 +164,8 @@ const ConnectedCarouselFilmStrip = connectCarouselFilmStrip(
               <SendStatus activity={ activity } />
             : showTimestamp &&
               <Timestamp activity={ activity } />
-          }
+            }
+          </div>
         </div>
       </div>
     );
diff --git a/packages/component/src/Activity/SendStatus.js b/packages/component/src/Activity/SendStatus.js
index 749840b1ed..04168f21e7 100644
--- a/packages/component/src/Activity/SendStatus.js
+++ b/packages/component/src/Activity/SendStatus.js
@@ -58,7 +58,7 @@ export default connectSendStatus(
     retrySend,
     styleSet
   }) =>
-    <span className={ styleSet.sendStatus }>
+    <span aria-live="polite" className={ styleSet.sendStatus }>
       {
         state === SENDING ?
           <Localize text="Sending" />
diff --git a/packages/component/src/Activity/StackedLayout.js b/packages/component/src/Activity/StackedLayout.js
index 19d6fb20d1..9ac8234dca 100644
--- a/packages/component/src/Activity/StackedLayout.js
+++ b/packages/component/src/Activity/StackedLayout.js
@@ -4,13 +4,13 @@ import React from 'react';
 
 import { Constants } from 'botframework-webchat-core';
 
+import { localize } from '../Localization/Localize';
 import Avatar from './Avatar';
 import Bubble from './Bubble';
-import SendStatus from './SendStatus';
-import Timestamp from './Timestamp';
-
 import connectToWebChat from '../connectToWebChat';
+import SendStatus from './SendStatus';
 import textFormatToContentType from '../Utils/textFormatToContentType';
+import Timestamp from './Timestamp';
 
 const { ActivityClientState: { SENDING, SEND_FAILED } } = Constants;
 
@@ -55,29 +55,47 @@ const ROOT_CSS = css({
 
 const connectStackedLayout = (...selectors) => connectToWebChat(
   ({
-    botAvatarInitials,
     language,
-    userAvatarInitials
-  }) => ({
-    botAvatarInitials,
+    styleSet: {
+      options: {
+        botAvatarInitials,
+        userAvatarInitials
+      }
+    }
+  }, { activity }) => ({
+    avatarInitials: activity.from && activity.from.role === 'user' ? userAvatarInitials : botAvatarInitials,
     language,
+
+    // TODO: [P4] We want to deprecate botAvatarInitials/userAvatarInitials because they are not as helpful as avatarInitials
+    botAvatarInitials,
     userAvatarInitials
   }),
   ...selectors
 );
 
 export default connectStackedLayout(
-  ({ styleSet }) => ({ styleSet })
+  ({
+    avatarInitials,
+    language,
+    styleSet
+  }) => ({
+    avatarInitials,
+    language,
+    styleSet
+  })
 )(
   ({
     activity,
+    avatarInitials,
     children,
+    language,
     showTimestamp,
-    styleSet
+    styleSet,
   }) => {
     const fromUser = activity.from.role === 'user';
     const { state } = activity.channelData || {};
     const showSendStatus = state === SENDING || state === SEND_FAILED;
+    const ariaLabel = localize(fromUser ? 'User said something' : 'Bot said something', language, avatarInitials, activity.text, activity.timestamp);
 
     return (
       <div
@@ -88,6 +106,7 @@ export default connectStackedLayout(
         ) }
       >
         <Avatar
+          aria-hidden={ true }
           className="avatar"
           fromUser={ fromUser }
         />
@@ -106,6 +125,7 @@ export default connectStackedLayout(
             : !!activity.text &&
               <div className="row message">
                 <Bubble
+                  aria-label={ ariaLabel }
                   className="bubble"
                   fromUser={ fromUser }
                 >
@@ -137,7 +157,10 @@ export default connectStackedLayout(
           }
           {
             (showSendStatus || showTimestamp) &&
-              <div className="row">
+              <div
+                aria-hidden={ true }
+                className="row"
+              >
                 { showSendStatus ?
                     <SendStatus activity={ activity } className="timestamp" />
                   :
diff --git a/packages/component/src/BasicTranscript.js b/packages/component/src/BasicTranscript.js
index cf2e31d4fa..e8f7645263 100644
--- a/packages/component/src/BasicTranscript.js
+++ b/packages/component/src/BasicTranscript.js
@@ -94,9 +94,9 @@ const BasicTranscript = ({
           speechSynthesis={ speechSynthesis }
           speechSynthesisUtterance={ SpeechSynthesisUtterance }
         >
-          <ul 
+          <ul
             aria-live="polite"
-            className={ classNames(LIST_CSS + '', styleSet.activities + '') } 
+            className={ classNames(LIST_CSS + '', styleSet.activities + '') }
             role="list"
           >
             {
@@ -106,7 +106,7 @@ const BasicTranscript = ({
                 return (
                   <li
                     className={ styleSet.activity }
-                    key={ activity.id || (activity.channelData && activity.channelData.clientActivityID) || index }
+                    key={ (activity.channelData && activity.channelData.clientActivityID) || activity.id || index }
                     role="listitem"
                   >
                     { activityRenderer({ activity, showTimestamp })(({ attachment }) => attachmentRenderer({ activity, attachment })) }
diff --git a/packages/component/src/Localization/Localize.js b/packages/component/src/Localization/Localize.js
index 4363f19e29..2b9c5793af 100644
--- a/packages/component/src/Localization/Localize.js
+++ b/packages/component/src/Localization/Localize.js
@@ -56,11 +56,11 @@ function getStrings(language) {
   }
 }
 
-function localize(text, language, args) {
+function localize(text, language, ...args) {
   const string = (getStrings(language) || {})[text] || enUS[text];
 
   if (typeof string === 'function') {
-    return string(args);
+    return string(...args);
   } else {
     return string || text;
   }
@@ -68,7 +68,7 @@ function localize(text, language, args) {
 
 export default connectToWebChat(
   ({ language }) => ({ language })
-)(({ args, language, text }) => localize(text, language, args))
+)(({ args, language, text }) => localize(text, language, ...(args || [])))
 
 export { localize }
 
diff --git a/packages/component/src/Localization/en-US.js b/packages/component/src/Localization/en-US.js
index 5f770d3e2f..42aa30616e 100644
--- a/packages/component/src/Localization/en-US.js
+++ b/packages/component/src/Localization/en-US.js
@@ -1,3 +1,11 @@
+function botSaidSomething(avatarInitials, text, timestamp) {
+  return `Bot ${ avatarInitials } said, ${ text }, ${ xMinutesAgo(timestamp) }`;
+}
+
+function userSaidSomething(avatarInitials, text, timestamp) {
+  return `User ${ avatarInitials } said, ${ text }, ${ xMinutesAgo(timestamp) }`;
+}
+
 function xMinutesAgo(date) {
   const now = Date.now();
   const deltaInMs = now - new Date(date).getTime();
@@ -24,6 +32,8 @@ function xMinutesAgo(date) {
 }
 
 export default {
+  'Bot said something': botSaidSomething,
+  'User said something': userSaidSomething,
   'X minutes ago': xMinutesAgo,
   ...[
     // '[File of type '%1']",