Skip to content

Commit

Permalink
Fixed open bot dialog url validation a11y issue.
Browse files Browse the repository at this point in the history
  • Loading branch information
tonyanziano committed Oct 8, 2019
1 parent 558fab3 commit 98e7b21
Show file tree
Hide file tree
Showing 6 changed files with 19 additions and 26 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -65,6 +65,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- [1902](https://github.com/microsoft/BotFramework-Emulator/pull/1902)
- [1893](https://github.com/microsoft/BotFramework-Emulator/pull/1893)
- [1916](https://github.com/microsoft/BotFramework-Emulator/pull/1916)
- [1918](https://github.com/microsoft/BotFramework-Emulator/pull/1918)

- [client] Fixed an issue with the transcripts path input inside of the resource settings dialog in PR [1836](https://github.com/microsoft/BotFramework-Emulator/pull/1836)
- [client] Implemented HTML app menu for Windows in PR [1893](https://github.com/microsoft/BotFramework-Emulator/pull/1893)
Expand Down
Expand Up @@ -249,16 +249,6 @@ describe('The OpenBotDialog', () => {
expect(instance.state.botUrl).toBe('http://localhost:3978');
});

it('should announce any validation error messages', () => {
// make sure there are no leftover alerts from previous test(s)
const preExistingAlerts = document.querySelectorAll('body > span#alert-from-service');
preExistingAlerts.forEach(alert => alert.remove());
const spy = jest.spyOn(ariaAlertService, 'alert').mockReturnValueOnce(undefined);
instance.announceErrorMessage('Invalid bot url.');

expect(spy).toHaveBeenCalledWith('For Bot URL, Invalid bot url.');
});

it('should call the appropriate command when onAnchorClick is called', () => {
instance.props.onAnchorClick('http://blah');
expect(mockDispatch).toHaveBeenCalledWith(
Expand Down
10 changes: 0 additions & 10 deletions packages/app/client/src/ui/dialogs/openBotDialog/openBotDialog.tsx
Expand Up @@ -46,12 +46,9 @@ import * as React from 'react';
import { ChangeEvent, Component, MouseEvent, ReactNode } from 'react';
import { EmulatorMode } from '@bfemulator/sdk-shared';

import { debounce } from '../../../utils';

import * as openBotStyles from './openBotDialog.scss';

export interface OpenBotDialogProps {
createAriaAlert?: (msg: string) => void;
mode?: EmulatorMode;
isDebug?: boolean;
onAnchorClick?: (url: string) => void;
Expand Down Expand Up @@ -136,7 +133,6 @@ export class OpenBotDialog extends Component<OpenBotDialogProps, OpenBotDialogSt
const { botUrl, appId, appPassword, mode, isDebug, isAzureGov } = this.state;
const validationResult = OpenBotDialog.validateEndpoint(botUrl);
const errorMessage = OpenBotDialog.getErrorMessage(validationResult);
errorMessage && this.announceErrorMessage(errorMessage);
const shouldBeDisabled =
validationResult === ValidationResult.Invalid || validationResult === ValidationResult.Empty;
const botUrlLabel = 'Bot URL';
Expand Down Expand Up @@ -258,10 +254,4 @@ export class OpenBotDialog extends Component<OpenBotDialogProps, OpenBotDialogSt
}
return null;
}

/** Announces the error message to screen reader technologies */
private announceErrorMessage = debounce((msg: string): void => {
// ensure that we aren't spamming aria alerts each time the input is validated
this.props.createAriaAlert(`For Bot URL, ${msg}`);
}, 2000);
}
Expand Up @@ -38,16 +38,12 @@ import { Action } from 'redux';
import { openBotViaFilePathAction, openBotViaUrlAction } from '../../../state/actions/botActions';
import { DialogService } from '../service';
import { RootState } from '../../../state/store';
import { ariaAlertService } from '../../a11y';
import { executeCommand } from '../../../state/actions/commandActions';

import { OpenBotDialog, OpenBotDialogProps, OpenBotDialogState } from './openBotDialog';

const mapDispatchToProps = (dispatch: (action: Action) => void): OpenBotDialogProps => {
return {
createAriaAlert: (msg: string) => {
ariaAlertService.alert(msg);
},
onAnchorClick: (url: string) => {
dispatch(executeCommand(true, SharedConstants.Commands.Electron.OpenExternal, null, url));
},
Expand Down
Expand Up @@ -76,6 +76,13 @@ describe('<AutoComplete />', () => {
expect(instance.errorMessage).toBeTruthy();
});

it('should generate an error message id if there is an error message', () => {
expect(instance.errorMessageId).toBe(undefined);

wrapper.setProps({ errorMessage: 'something broke :(' });
expect(instance.errorMessageId).toBe(`auto-complete-err-msg-${wrapper.state().id}`);
});

it('should generate a label id if a label has been provided', () => {
expect(instance.labelId).toBe(undefined);

Expand Down
13 changes: 11 additions & 2 deletions packages/sdk/ui-react/src/widget/autoComplete/autoComplete.tsx
Expand Up @@ -74,7 +74,7 @@ export class AutoComplete extends Component<AutoCompleteProps, AutoCompleteState
}

public render(): ReactNode {
const { onBlur, onChange, onFocus, onKeyDown, value } = this;
const { errorMessageId, onBlur, onChange, onFocus, onKeyDown, value } = this;
const { autoFocus, className = '', errorMessage, disabled, placeholder } = this.props;
const invalidClassName = errorMessage ? styles.invalid : '';

Expand Down Expand Up @@ -102,6 +102,7 @@ export class AutoComplete extends Component<AutoCompleteProps, AutoCompleteState
aria-activedescendant={this.getOptionId(this.state.selectedIndex)}
aria-autocomplete="list"
aria-controls={this.listboxId}
aria-labelledby={errorMessageId}
/>
{this.errorMessage}
{this.results}
Expand Down Expand Up @@ -148,11 +149,19 @@ export class AutoComplete extends Component<AutoCompleteProps, AutoCompleteState
private get errorMessage(): ReactNode {
const { errorMessage } = this.props;
if (errorMessage) {
return <sub className={styles.errorMessage}>{errorMessage}</sub>;
return (
<sub id={this.errorMessageId} className={styles.errorMessage}>
{errorMessage}
</sub>
);
}
return undefined;
}

private get errorMessageId(): string {
return this.props.errorMessage ? `auto-complete-err-msg-${this.state.id}` : undefined;
}

private get labelId(): string {
// label id is only necessary if we have a label
return this.props.label ? `auto-complete-label-${this.state.id}` : undefined;
Expand Down

0 comments on commit 98e7b21

Please sign in to comment.