Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bump deps and remove remark and strip-markdown #3917

Merged
merged 37 commits into from
Jun 3, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
253627f
Bump deps
compulim May 27, 2021
8098c36
Go back to v1
compulim May 27, 2021
c81250a
Remove remark and strip-markdown
compulim May 27, 2021
2b7332d
Revert to v1
compulim May 27, 2021
02d1878
Revert to v1
compulim May 27, 2021
b76f630
Revert to v1
compulim May 27, 2021
3de4f40
Supports summary field
compulim May 27, 2021
e6e8656
Ignore summary field
compulim May 27, 2021
3848258
Strip Markdown from HTML
compulim May 28, 2021
6844630
Add speak field not present
compulim May 28, 2021
e24b0eb
Update strip Markdown
compulim May 28, 2021
7070d0b
Narrate empty text field
compulim May 28, 2021
8326dd0
Use different text computation for HTML/Markdown
compulim May 28, 2021
2bdea47
Clean up
compulim May 28, 2021
b5c8e2e
Update with samples
compulim May 28, 2021
ea061d1
More inline elements
compulim May 28, 2021
01ecd81
Clean up
compulim May 28, 2021
6bf131d
MessageBack must not be Markdown
compulim May 28, 2021
e136c8f
Lockdown performance
compulim May 28, 2021
884c0bc
Add snapshots
compulim May 28, 2021
e4c9ef1
Fix MessageBack
compulim May 28, 2021
8f9ba7f
Remove renderMarkdownAsHTML
compulim May 28, 2021
6b6a974
Fix ESLint
compulim May 28, 2021
36afb79
Assume plain text if renderMarkdown is null
compulim May 28, 2021
1000321
Compute text alternatives using alt field
compulim Jun 1, 2021
ab7242b
Change to webchat:fallback-text field
compulim Jun 2, 2021
cf2c82f
Bump to [email protected]
compulim Jun 2, 2021
70375f5
Typo
compulim Jun 2, 2021
4bc5c60
Update entry
compulim Jun 2, 2021
246eb64
Typo
compulim Jun 2, 2021
510348f
Update entry
compulim Jun 2, 2021
80d6a27
Link to issue
compulim Jun 2, 2021
86fc365
Undo package*.json for core/embed
compulim Jun 2, 2021
c85d9dd
Move allTextContents into testHelpers
compulim Jun 3, 2021
9d862ef
Add @param
compulim Jun 3, 2021
8ee4888
Apply PR suggestions
compulim Jun 3, 2021
f9060f2
Update text
compulim Jun 3, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
- Cleanup repo URLs to point to main branch, by [@corinagum](https://github.com/corinagum), in PR [#3870](https://github.com/microsoft/BotFramework-WebChat/pull/3870)
- Resolves [#3557](https://github.com/microsoft/BotFramework-WebChat/issues/3557) and [#3736](https://github.com/microsoft/BotFramework-WebChat/issues/3736). Improved test harness and added browser pooling, by [@compulim](https://github.com/compulim), in PR [#3871](https://github.com/microsoft/BotFramework-WebChat/pull/3871)
- Resolves [#3788](https://github.com/microsoft/BotFramework-WebChat/issues/3788). Added `localTimestamp` and `localTimezone` (if available) to all outgoing activities, by [@compulim](https://github.com/compulim), in PR [#3896](https://github.com/microsoft/BotFramework-WebChat/pull/3896)
- Resolves [#3918](https://github.com/microsoft/BotFramework-WebChat/issues/3918). Supports [`activity.summary` field](https://github.com/microsoft/botframework-sdk/blob/main/specs/botframework-activity/botframework-activity.md#summary) if available, by [@compulim](https://github.com/compulim), in PR [#3917](https://github.com/microsoft/BotFramework-WebChat/pull/3917)

### Fixed

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,23 +53,12 @@
directLine: testHelpers.createDirectLineWithTranscript([
{
...baseActivity,
text: 'No "speak" or "summary" properties.'
text: 'No "speak" property.'
},
{
...baseActivity,
speak: 'Only "speak" property.',
text: 'Only "speak" property.'
},
{
...baseActivity,
summary: '2 documents.',
text: 'Only "summary" property.'
},
{
...baseActivity,
speak: 'Both "speak" and "summary" properties.',
summary: 'This should not be narrated.',
text: 'Both "speak" and "summary" properties.'
speak: 'With "speak" property.',
text: 'With "speak" property.'
},
{
...baseActivity,
Expand All @@ -86,30 +75,23 @@
);

await pageConditions.uiConnected();
await pageConditions.numActivitiesShown(3);

const screenReaderTexts = [].map.call(
document.querySelector('[aria-roledescription="transcript"][role="log"]').children,
child => allTextContents(child).join('\n')
);

// Verify compliance of https://github.com/microsoft/botframework-sdk/blob/main/specs/botframework-activity/botframework-activity.md#summary.

// The last activity have "speak" property set to empty string. It is treated as role="presentation" and not emitted DOM element for screen reader.
expect(screenReaderTexts).toHaveLength(4);
expect(screenReaderTexts).toHaveLength(2);

// In screen reader transcript, the attachment are rendered and narrated.
expect(screenReaderTexts[0]).toBe(
'Bot said:\nNo "speak" or "summary" properties.\nA text: Hello.\nA text: World!\nSent at January 1 at 12:34 PM'
'Bot said:\nNo "speak" property.\nA text: Hello.\nA text: World!\nSent at January 1 at 12:34 PM'
);

expect(screenReaderTexts[1]).toBe('Bot said:\nOnly "speak" property.\nSent at January 1 at 12:34 PM');
expect(screenReaderTexts[2]).toBe(
'Bot said:\nOnly "summary" property.\n2 documents.\nSent at January 1 at 12:34 PM'
);

expect(screenReaderTexts[3]).toBe(
'Bot said:\nBoth "speak" and "summary" properties.\nSent at January 1 at 12:34 PM'
);
// No attachments should be narrated.
expect(screenReaderTexts[1]).toBe('Bot said:\nWith "speak" property.\nSent at January 1 at 12:34 PM');

const activityAlts = [].map.call(
document.querySelectorAll(
Expand All @@ -119,16 +101,14 @@
);

// The last activity have "speak" property set to empty string. It is treated as role="presentation" and not emitted DOM element for screen reader.
expect(screenReaderTexts).toHaveLength(4);
expect(screenReaderTexts).toHaveLength(2);

// In interactive transcript, this is narrated as "2 attachments".
expect(activityAlts[0]).toBe(
'Bot said:\nNo "speak" or "summary" properties.\n2 attachments.\nSent at January 1 at 12:34 PM'
'Bot said:\nNo "speak" property.\n2 attachments.\nSent at January 1 at 12:34 PM'
);

expect(activityAlts[1]).toBe('Bot said:\nOnly "speak" property.\nSent at January 1 at 12:34 PM');
expect(activityAlts[2]).toBe('Bot said:\nOnly "summary" property.\n2 documents.\nSent at January 1 at 12:34 PM');
expect(activityAlts[3]).toBe('Bot said:\nBoth "speak" and "summary" properties.\nSent at January 1 at 12:34 PM');
expect(activityAlts[1]).toBe('Bot said:\nWith "speak" property.\nSent at January 1 at 12:34 PM');
});
</script>
</body>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
/** @jest-environment ./packages/test/harness/src/host/jest/WebDriverEnvironment.js */

// Verify compliance of https://github.com/microsoft/botframework-sdk/blob/main/specs/botframework-activity/botframework-activity.md#speak.
describe('accessibility requirement', () => {
test('when "speak" field present, it should override all attachments', () =>
runHTML('accessibility.liveRegionActivity.speakOverrideAttachments.html'));
});
6 changes: 0 additions & 6 deletions __tests__/html/accessibility.liveRegionActivity.summary.js

This file was deleted.

5 changes: 2 additions & 3 deletions __tests__/html/accessibility.liveRegionActivity.text.html
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@
);

await pageConditions.uiConnected();
await pageConditions.numActivitiesShown(9);

const screenReaderTexts = [].map.call(
document.querySelector('[aria-roledescription="transcript"][role="log"]').children,
Expand Down Expand Up @@ -180,9 +181,7 @@
);

// Expectation: when we concatenate the text content, we must not add whitespaces. Some languages don't have whitespaces.
expect(screenReaderTexts[5]).toBe(
'Bot said:\n今日天氣很好。\nSent at January 1 at 12:34 PM'
);
expect(screenReaderTexts[5]).toBe('Bot said:\n今日天氣很好。\nSent at January 1 at 12:34 PM');

expect(screenReaderTexts[6]).toBe('Bot said:\nSay another thing.\nSent at January 1 at 12:34 PM');
expect(screenReaderTexts[7]).toBe('You said:\nText from MessageBack.\nSent at January 1 at 12:34 PM');
Expand Down
1 change: 1 addition & 0 deletions __tests__/html/accessibility.liveRegionActivity.text.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/** @jest-environment ./packages/test/harness/src/host/jest/WebDriverEnvironment.js */

// Verify compliance of https://github.com/microsoft/botframework-sdk/blob/main/specs/botframework-activity/botframework-activity.md#speak.
describe('accessibility requirement', () => {
test('should clean up "speak" property for live region text', () =>
runHTML('accessibility.liveRegionActivity.text.html'));
Expand Down
48 changes: 44 additions & 4 deletions docs/ACCESSIBILITY.md
Original file line number Diff line number Diff line change
Expand Up @@ -259,11 +259,11 @@ To make the live region more consistent across browsers and easier to control, w

### Technical Limitation

- Activity Time Stamp Announcement : Related to [#3136](https://github.com/microsoft/BotFramework-WebChat/issues/3136)
- Problem definition : Even when a user overrides the 'grouptimestamp' props and sets it to true or to some interval, AT still announces every activity with it's associated Time stamp.
- Explanation of current behavior : Once activity is marked as sent, it is written to DOM as well as it's Timestamp; the timestamp grouping logic is executed only when the next activity arrives. As mention earlier once a text is queued for narration and even if DOM element is removed it will still be announced by the screen reader as it is not technically possible to removed from the narration queue.
- Activity timestamp announcement: related to [#3136](https://github.com/microsoft/BotFramework-WebChat/issues/3136)
- Problem definition: when the developer overrides the 'groupTimestamp' props and sets it to `true` or to some interval, screen reader still announces every activity with its associated timestamp.
- Explanation of current behavior: Once activity is marked as sent, it is written to DOM as well as its timestamp; the timestamp grouping logic is executed only when the next activity arrives. As mention earlier once a text is queued for narration and even if DOM element is removed it will still be announced by the screen reader as it is not technically possible to removed from the narration queue.
- Given above limitation even if we removed the timestamp element from DOM after group timestamp logic is executed this will not change the screen reader behavior.
- As per Accessibility team review/recommendation : There is no hiding or loss of information in this case - so will keep the current behavior as is.
- As per accessibility team review/recommendation: there is no hiding or loss of information in this case - so will keep the current behavior as is.

## Do's and don't

Expand Down Expand Up @@ -293,6 +293,46 @@ To make the live region more consistent across browsers and easier to control, w
- Only change focus synchronous to user gesture
- Related to [#3135](https://github.com/microsoft/BotFramework-WebChat/issues/3135)

# Controlling the narration of activities and attachments

> This is related to [#3360](https://github.com/microsoft/BotFramework-WebChat/issues/3360), [#3615](https://github.com/microsoft/BotFramework-WebChat/issues/3615), [#3918](https://github.com/microsoft/BotFramework-WebChat/issues/3918).

## User story

A bot developer want to set the narration for a message activity. The activity may or may not have attachments.
compulim marked this conversation as resolved.
Show resolved Hide resolved

For example, it is required for the following user stories:

- The message contains Markdown
- For example, the `text` field is `"Hello, *World!*"`
- Desirable: "hello world"
- Undesirable: if `speak` field is not set, it will be narrated as "hello (pause) asterisk world asterisk"
- The message contains HTML
- For example, the `text` field is `"## Exchange rate:\n\n<table><tr><th>USD</th><td>1.00</td></tr><tr><th>JPY</th><td>0.91</td></tr></table>"`
- Desirable: "Exchange rate for 1 USD dollar is 0.91 Japanese yen."
- Undesirable: any HTML or Markdown syntax
- The message contains a document, such as an insurance policy
- For example, the `text` field is `"Insurance policy:"`, and the attachment contains a file named `12345678-1234-5678-abcd-12345678abcd.doc`
- Desirable: "The insurance policy is ready to download."
- Undesirable: any narration containing the bogus file name

## Implementation

Based on [Bot Framework Activity spec](https://github.com/microsoft/botframework-sdk/blob/main/specs/botframework-activity/botframework-activity.md), the following fields can be used to control the narration: `speak` and `text`.

We implemented the following logic for computing the text for screen reader:

1. If `speak` field present
1. If `speak` field is not an empty string, narrate the field, don't narrate attachments
compulim marked this conversation as resolved.
Show resolved Hide resolved
2. If `speak` field is an empty string, don't narrate the whole activity (treat it as `aria-hidden="true"` or `role="presentation"`)
2. Otherwise
- Narrate `text` field, followed by every attachment rendered via `attachmentForScreenReader` middleware
- Note the `text` field is optional

If `speak` field is present, the attachments will not be narrated, as stated in the [Bot Framework Activity spec](https://github.com/microsoft/botframework-sdk/blob/main/specs/botframework-activity/botframework-activity.md#speak), excerpt:

> (`speak` field) replaces speech synthesis for any content within the activity, including text, attachments, and summaries.

# 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.
Expand Down
14 changes: 4 additions & 10 deletions packages/component/src/ScreenReaderActivity.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ const ACTIVITY_NUM_ATTACHMENTS_ALT_IDS = {
two: 'ACTIVITY_NUM_ATTACHMENTS_TWO_ALT'
};

const GenericScreenReaderAttachments = ({ activity, renderAttachments }) => {
const ScreenReaderAttachments = ({ activity, renderAttachments }) => {
const { attachments = [] } = activity;
const createAttachmentForScreenReaderRenderer = useCreateAttachmentForScreenReaderRenderer();
const localizeWithPlural = useLocalizer({ plural: true });
Expand Down Expand Up @@ -61,7 +61,7 @@ const GenericScreenReaderAttachments = ({ activity, renderAttachments }) => {
);
};

GenericScreenReaderAttachments.propTypes = {
ScreenReaderAttachments.propTypes = {
activity: PropTypes.shape({
attachments: PropTypes.array
}).isRequired,
Expand All @@ -77,7 +77,7 @@ const ScreenReaderActivity = ({ activity, children, id, renderAttachments, textA
const localize = useLocalizer();
const rootClassName = useStyleToEmotionObject()(ROOT_STYLE) + '';

const { from: { role } = {}, speak, summary, timestamp } = activity;
const { from: { role } = {}, speak, timestamp } = activity;

const fromUser = role === 'user';

Expand All @@ -102,13 +102,7 @@ const ScreenReaderActivity = ({ activity, children, id, renderAttachments, textA
<span>{greetingAlt}</span>
<span>{textAlt}</span>
</p>
{speak ? (
false
) : summary ? (
<p>{summary}</p>
) : (
<GenericScreenReaderAttachments activity={activity} renderAttachments={renderAttachments} />
)}
{!speak && <ScreenReaderAttachments activity={activity} renderAttachments={renderAttachments} />}
<p className="webchat__screen-reader-activity__timestamp">{timestampAlt}</p>
{children}
</article>
Expand Down