Skip to content

Commit

Permalink
Fix narration of "New messages" button (#3204)
Browse files Browse the repository at this point in the history
* Fix narration of "New messages" button

* Update tests

* Update tests

* Fix tests

* Fix ESLint

* Fix ESLint

* Fix build

* Fix screenshots

* Prettier

* Apply suggestions from code review

Co-authored-by: TJ Durnford <tjdford@gmail.com>

Co-authored-by: Corina <14900841+corinagum@users.noreply.github.com>
Co-authored-by: TJ Durnford <tjdford@gmail.com>
  • Loading branch information
3 people authored Jun 5, 2020
1 parent 274693e commit 270a1d4
Show file tree
Hide file tree
Showing 19 changed files with 114 additions and 50 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.

- Fixes [#1340](https://github.com/microsoft/BotFramework-WebChat/issues/1340). Card container should not be focusable if they do not have `tapAction`, by [@compulim](https://github.saobby.my.eu.orgpulim) in PR [#3193](https://github.com/microsoft/BotFramework-WebChat/issues/3193)
- Fixed [#3196](https://github.com/microsoft/BotFramework-WebChat/issues/3196). Cards with `tapAction` should be executable by <kbd>ENTER</kbd> or <kbd>SPACEBAR</kbd> key, by [@compulim](https://github.com/compulim) in PR [#3197](https://github.com/microsoft/BotFramework-WebChat/pull/3197)
- Fixed [#3203](https://github.com/microsoft/BotFramework-WebChat/issues/3203). "New messages" button should be narrated by assistive technology, by [@compulim](https://github.com/compulim) in PR [#3204](https://github.com/microsoft/BotFramework-WebChat/pull/3204)

### Changed

Expand Down
6 changes: 3 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -208,9 +208,9 @@ View the [telemetry documentation](https://github.com/microsoft/BotFramework-Web

Web Chat supports a wide-range of speech engines for a natural chat experience with a bot. This section outlines the different engines that are supported:

- [Direct Line Speech](#integrate-with-direct-line-speech)
- [Cognitive Services Speech Services](#integrate-with-cognitive-services-speech-services)
- [Browser-provided engine or other engines](#browser-provided-engine-or-other-engines)
- [Direct Line Speech](#integrate-with-direct-line-speech)
- [Cognitive Services Speech Services](#integrate-with-cognitive-services-speech-services)
- [Browser-provided engine or other engines](#browser-provided-engine-or-other-engines)

### Integrate with Direct Line Speech

Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion __tests__/html/newMessageButton.tabOrder.html
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@

const newMessageButton = elements.newMessageButton();

expect(listItems[2].children[0]).toBe(newMessageButton);
expect(listItems[2]).toBe(newMessageButton);

await host.snapshot();
await host.done();
Expand Down
4 changes: 3 additions & 1 deletion __tests__/setup/NUnitTestReporter.js
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,9 @@ class NUnitTestReporter {
'#cdata-section': error
? ErrorStackParser.parse(error)
.map(stackFrame => {
stackFrame.setFileName(relative(join(__dirname, '..', '..'), stackFrame.fileName));
stackFrame.setFileName(
relative(join(__dirname, '..', '..'), stackFrame.fileName || '.')
);

return stackFrame + '';
})
Expand Down
2 changes: 1 addition & 1 deletion __tests__/setup/conditions/scrollToBottomButtonVisible.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { By, until } from 'selenium-webdriver';

export default function scrollToBottomButtonVisible() {
return until.elementLocated(By.css(`button.webchat__scrollToEndButton`));
return until.elementLocated(By.css(`.webchat__scrollToEndButton`));
}
2 changes: 1 addition & 1 deletion __tests__/setup/elements/getScrollToBottomButton.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,5 @@ import getTranscript from './getTranscript';
export default async function getScrollToBottomButton(driver) {
const transcript = await getTranscript(driver);

return await transcript.findElement(By.css('button.webchat__scrollToEndButton'));
return await transcript.findElement(By.css('.webchat__scrollToEndButton'));
}
12 changes: 12 additions & 0 deletions docs/ACCESSIBILITY.md
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,18 @@ When the "New messages" button is clicked:
- If a [tabbable](#tabbable) UI is found, focus on it
- Otherwise, focus on the send box wihout soft keyboard

### ARIA role "separator"

We are using the [ARIA role "separator"](https://www.w3.org/TR/wai-aria-1.1/#separator) for the "New messages" button. This is because the button serves as a visible boundary between read and unread messages, similar to a [horizontal ruler](https://www.w3.org/TR/html52/grouping-content.html#the-hr-element). Per [HTML 5.2 specifications](https://www.w3.org/TR/html52/grouping-content.html#the-li-element), the separator role is allowed to be used in the `<li>` element.

The separator role has a property called [children presentational](https://www.w3.org/TR/wai-aria-1.1/#childrenArePresentational). This property hides all children from assistive technology. Its effect is very similar to setting `role="presentation"` to all descendants and is not overrideable. Thus, it prevented us from using `<button>` widget inside the `<li role="separator">` element.

Fortunately, the separator role has two modes of operation: static structure or focusable widget. The "New messages" button is using the latter mode, which supports interactivity and movement.

When the "New messages" separator is activated, it logically moves the separator to the bottom of the page, thus, marking all messages as read. And we only support one direction and one amount of movement.

Lastly, we style the "New messages" separator like a normal button, styled it to float on top of the transcript, and added `click` and `keypress` event listener for <kbd>ENTER</kbd> and <kbd>SPACEBAR</kbd> key for [activation](https://www.w3.org/TR/wai-aria-practices-1.1/#button).

## Do and don't

### Do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,16 @@ const AdaptiveCardRenderer = ({ actionPerformedClassName, adaptiveCard, disabled

adaptiveCardElementRef.current = undefined;
};
}, [adaptiveCard, adaptiveCardsHostConfig, contentRef, HostConfig, renderMarkdownAsHTML, setErrors]);
}, [
adaptiveCard,
adaptiveCardsHostConfig,
contentRef,
GlobalSettings,
HostConfig,
renderMarkdownAsHTML,
setErrors,
tapAction
]);

useEffect(() => {
// Set onExecuteAction without causing unnecessary re-render.
Expand Down Expand Up @@ -438,6 +447,7 @@ const AdaptiveCardRenderer = ({ actionPerformedClassName, adaptiveCard, disabled
return errors.length ? (
<ErrorBox message={localize('ADAPTIVE_CARD_ERROR_BOX_TITLE_RENDER')}>
{errors.map(({ error, message }, index) => (
/* eslint-disable-next-line react/no-array-index-key */
<pre key={index}>{JSON.stringify({ error, message }, null, 2)}</pre>
))}
</ErrorBox>
Expand Down
98 changes: 65 additions & 33 deletions packages/component/src/Activity/ScrollToEndButton.js
Original file line number Diff line number Diff line change
@@ -1,51 +1,83 @@
import classNames from 'classnames';
import PropTypes from 'prop-types';
import React, { useCallback } from 'react';
import React, { forwardRef, useCallback } from 'react';

import useDirection from '../hooks/useDirection';
import useLocalizer from '../hooks/useLocalizer';
import useScrollToEnd from '../hooks/useScrollToEnd';
import useStyleSet from '../hooks/useStyleSet';

const ScrollToEndButton = ({ className, onClick }) => {
const [{ scrollToEndButton: scrollToEndButtonStyleSet }] = useStyleSet();
const [direction] = useDirection();
const localize = useLocalizer();
const scrollToEnd = useScrollToEnd();

const handleClick = useCallback(
event => {
onClick && onClick(event);

scrollToEnd();
},
[onClick, scrollToEnd]
);

const newMessageText = localize('TRANSCRIPT_NEW_MESSAGES');

return (
<button
className={classNames(
'webchat__scrollToEndButton',
scrollToEndButtonStyleSet + '',
className + '',
direction === 'rtl' ? 'webchat__overlay--rtl' : ''
)}
onClick={handleClick}
type="button"
>
{newMessageText}
</button>
);
};
import { safari } from '../Utils/detectBrowser';

const ScrollToEndButton = forwardRef(
(
{ 'aria-valuemax': ariaValueMax, 'aria-valuemin': ariaValueMin, 'aria-valuenow': ariaValueNow, className, onClick },
ref
) => {
const [{ scrollToEndButton: scrollToEndButtonStyleSet }] = useStyleSet();
const [direction] = useDirection();
const localize = useLocalizer();
const scrollToEnd = useScrollToEnd();

const handleClick = useCallback(
event => {
onClick && onClick(event);
scrollToEnd();
},
[onClick, scrollToEnd]
);

const handleKeyPress = useCallback(
event => {
if (event.key === 'Enter' || event.key === ' ') {
event.preventDefault();

onClick && onClick(event);
scrollToEnd();
}
},
[onClick, scrollToEnd]
);

const newMessageText = localize('TRANSCRIPT_NEW_MESSAGES');

return (
<li
aria-label={newMessageText}
aria-valuemax={ariaValueMax}
aria-valuemin={ariaValueMin}
aria-valuenow={ariaValueNow}
className={classNames(
'webchat__scrollToEndButton',
scrollToEndButtonStyleSet + '',
className + '',
direction === 'rtl' ? 'webchat__overlay--rtl' : ''
)}
onClick={handleClick}
onKeyPress={handleKeyPress}
ref={ref}
// iOS VoiceOver does not support role="separator" and treat it as role="presentation", which become invisible to VoiceOver.
role={safari ? undefined : 'separator'}
tabIndex={0}
>
{newMessageText}
</li>
);
}
);

ScrollToEndButton.defaultProps = {
'aria-valuemin': 0,
className: '',
onClick: undefined
};

ScrollToEndButton.displayName = 'ScrollToEndButton';

ScrollToEndButton.propTypes = {
'aria-valuemax': PropTypes.number.isRequired,
'aria-valuemin': PropTypes.number,
'aria-valuenow': PropTypes.number.isRequired,
className: PropTypes.string,
onClick: PropTypes.func
};
Expand Down
18 changes: 10 additions & 8 deletions packages/component/src/BasicTranscript.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ const BasicTranscriptContent = ({ animating, sticky }) => {
const [{ hideScrollToEndButton }] = useStyleOptions();
const [activities] = useActivities();
const focus = useFocus();
const listRef = useRef();
const renderAttachment = useRenderAttachment();
const renderActivity = useRenderActivity(renderAttachment);
const renderActivityElement = useCallback(
Expand All @@ -104,14 +103,15 @@ const BasicTranscriptContent = ({ animating, sticky }) => {
}),
[renderActivity]
);
const scrollToEndButtonRef = useRef();

const handleScrollToEndButtonClick = useCallback(() => {
const { current } = listRef;
const { current } = scrollToEndButtonRef;

// After clicking on the "New messages" button, we should focus on the first unread element.
// This is for resolving the bug https://github.com/microsoft/BotFramework-WebChat/issues/3135.
if (current) {
const nextSiblings = nextSiblingAll(current.querySelector('li[role="separator"]'));
const nextSiblings = nextSiblingAll(current);

const firstUnreadTabbable = nextSiblings.reduce(
(result, unreadActivityElement) => result || firstTabbableDescendant(unreadActivityElement),
Expand All @@ -120,7 +120,7 @@ const BasicTranscriptContent = ({ animating, sticky }) => {

firstUnreadTabbable ? firstUnreadTabbable.focus() : focus('sendBoxWithoutKeyboard');
}
}, [focus, listRef]);
}, [focus, scrollToEndButtonRef]);

const memoizeRenderActivityElement = useMemoize(renderActivityElement);

Expand Down Expand Up @@ -194,7 +194,6 @@ const BasicTranscriptContent = ({ animating, sticky }) => {
aria-live="polite"
aria-relevant="additions"
className={classNames(LIST_CSS + '', activitiesStyleSet + '')}
ref={listRef}
role="list"
>
{activityElementsWithMetadata.map(({ activity, element, key, shouldSpeak }, index) => (
Expand All @@ -205,9 +204,12 @@ const BasicTranscriptContent = ({ animating, sticky }) => {
</li>
{/* We insert the "New messages" button here for tab ordering. Users should be able to TAB into the button. */}
{index === renderSeparatorAfterIndex && (
<li role="separator">
<ScrollToEndButton onClick={handleScrollToEndButtonClick} />
</li>
<ScrollToEndButton
aria-valuemax={activityElementsWithMetadata.length}
aria-valuenow={index + 1}
onClick={handleScrollToEndButtonClick}
ref={scrollToEndButtonRef}
/>
)}
</React.Fragment>
))}
Expand Down
4 changes: 4 additions & 0 deletions packages/component/src/Styles/StyleSet/ScrollToEndButton.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
export default function createScrollToEndButtonStyle({
newMessagesButtonFontSize,
paddingRegular,
primaryFont,
transcriptOverlayButtonBackground,
transcriptOverlayButtonBackgroundOnFocus,
transcriptOverlayButtonBackgroundOnHover,
Expand All @@ -19,6 +21,8 @@ export default function createScrollToEndButtonStyle({
borderWidth: 0,
bottom: 5,
color: transcriptOverlayButtonColor,
fontFamily: primaryFont,
fontSize: newMessagesButtonFontSize,
outline: 0,
padding: paddingRegular,
position: 'absolute',
Expand Down
1 change: 1 addition & 0 deletions packages/component/src/Styles/defaultStyleOptions.js
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ const DEFAULT_OPTIONS = {
timestampFormat: 'relative', // 'absolute'

// Transcript overlay buttons (e.g. carousel and suggested action flippers, scroll to bottom, etc.)
newMessagesButtonFontSize: '85%',
transcriptOverlayButtonBackground: 'rgba(0, 0, 0, .6)',
transcriptOverlayButtonBackgroundOnFocus: 'rgba(0, 0, 0, .8)',
transcriptOverlayButtonBackgroundOnHover: 'rgba(0, 0, 0, .8)',
Expand Down
2 changes: 1 addition & 1 deletion packages/testharness/src/elements/newMessageButton.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
export default function newMessageButton() {
return document.querySelector(`button.webchat__scrollToEndButton`);
return document.querySelector(`.webchat__scrollToEndButton`);
}

0 comments on commit 270a1d4

Please sign in to comment.