Skip to content

Commit

Permalink
a11y: accessibility overhaul (#2426)
Browse files Browse the repository at this point in the history
* a11y: links to have underline

* a11y: json editor contast to meet aria

* a11y: revisit colors

* Revert "Fixed the focus when opening the Emulator Settings tab (#2310)"

This reverts commit b09b668.

* a11y: focus first interactive element upon editor activation

* a11y: add optional aria for optional fields

* a11y: add section to checkbox labels

* a11y: add selected only when selected

* a11y: aplit button to have aria-owns and size

* a11y: force button caption reading for reveal secret control

* a11y: make selected aria only appear if selected

* a11y: better aria for activity region

* fix typo

* a11y: menu to restore focus on task completion if it is lost

* a11y: turn off color adjust for high contrast theme

* fix: app updater can stuck due to unresolved promise

* fix: failing tests

* Revert "a11y: add optional aria for optional fields"

Conflicts with other AT besides JAWS

This reverts commit 9317e80.

* revert: conditional aria-selected

* a11y: alternative fix for split-button

* fix build

---------

Co-authored-by: Eugene Olonov <v-eolonov@microsoft.com>
  • Loading branch information
OEvgeny and Eugene Olonov committed Jul 10, 2023
1 parent b6c7dcc commit 515fc62
Show file tree
Hide file tree
Showing 23 changed files with 107 additions and 55 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,13 @@ export class BotCreationDialog extends React.Component<BotCreationDialogProps, B
const revealSecret = !this.state.revealSecret;
ariaAlertService.alert(`Secret ${revealSecret ? 'showing' : 'hidden'}.`);
this.setState({ revealSecret });

// Reset focus to update the button caption
const button = document.activeElement as HTMLButtonElement;
button?.blur();
setTimeout(() => {
button?.focus();
}, 100);
};

private onCopyClick = (): void => {
Expand Down
1 change: 0 additions & 1 deletion packages/app/client/src/ui/dialogs/dialogStyles.scss
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

.dialog-link {
color: var(--dialog-link-color);
text-decoration: none;
line-height: 20px;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ export interface AppSettingsEditorProps {
framework?: FrameworkSettings;
ngrokTunnelStatus?: TunnelStatus;
ngrokLastPingInterval?: TunnelCheckTimeInterval;
refreshHash?: string;

createAriaAlert?: (msg: string) => void;
discardChanges?: () => void;
Expand Down Expand Up @@ -93,7 +92,6 @@ export class AppSettingsEditor extends React.Component<AppSettingsEditorProps, A
return {
...newProps.framework,
dirty: newProps.dirty,
refreshHash: newProps.refreshHash,
pendingUpdate: false,
};
}
Expand All @@ -103,13 +101,6 @@ export class AppSettingsEditor extends React.Component<AppSettingsEditorProps, A
this.pathToNgrokInputRef.focus();
}
}

public componentDidUpdate() {
if (this.pathToNgrokInputRef) {
this.pathToNgrokInputRef.focus();
}
}

public render(): JSX.Element {
const {
ngrokPath = '',
Expand Down Expand Up @@ -165,6 +156,7 @@ export class AppSettingsEditor extends React.Component<AppSettingsEditorProps, A
checked={bypassNgrokLocalhost}
onChange={this.onChangeCheckBox}
id="ngrok-bypass"
aria-label="Bypass ngrok for local addresses, Service"
label="Bypass ngrok for local addresses"
name="bypassNgrokLocalhost"
/>
Expand All @@ -173,6 +165,7 @@ export class AppSettingsEditor extends React.Component<AppSettingsEditorProps, A
checked={runNgrokAtStartup}
onChange={this.onChangeCheckBox}
id="ngrok-startup"
aria-label="Run ngrok when the Emulator starts up, Service"
label="Run ngrok when the Emulator starts up"
name="runNgrokAtStartup"
/>
Expand Down Expand Up @@ -218,6 +211,7 @@ export class AppSettingsEditor extends React.Component<AppSettingsEditorProps, A
checked={use10Tokens}
onChange={this.onChangeCheckBox}
id="auth-token-version"
aria-label="Use version 1.0 authentication tokens, User settings"
label="Use version 1.0 authentication tokens"
name="use10Tokens"
/>
Expand All @@ -226,6 +220,7 @@ export class AppSettingsEditor extends React.Component<AppSettingsEditorProps, A
checked={useCodeValidation}
onChange={this.onChangeCheckBox}
id="use-validation-code"
aria-label="Use a sign-in verification code for OAuthCards, User settings"
label="Use a sign-in verification code for OAuthCards"
name="useCodeValidation"
/>
Expand All @@ -234,6 +229,7 @@ export class AppSettingsEditor extends React.Component<AppSettingsEditorProps, A
checked={useCustomId}
onChange={this.onChangeCheckBox}
id="use-custom-id"
aria-label="Use your own user ID to communicate with the bot, User settings"
label="Use your own user ID to communicate with the bot"
name="useCustomId"
/>
Expand Down Expand Up @@ -261,13 +257,15 @@ export class AppSettingsEditor extends React.Component<AppSettingsEditorProps, A
className={styles.checkboxOverrides}
checked={autoUpdate}
onChange={this.onChangeCheckBox}
aria-label="Automatically download and install updates, Application Updates"
label="Automatically download and install updates"
name="autoUpdate"
/>
<Checkbox
className={styles.checkboxOverrides}
checked={usePrereleases}
onChange={this.onChangeCheckBox}
aria-label="Use pre-release versions, Application Updates"
label="Use pre-release versions"
name="usePrereleases"
/>
Expand All @@ -278,6 +276,7 @@ export class AppSettingsEditor extends React.Component<AppSettingsEditorProps, A
className={styles.checkboxOverrides}
checked={collectUsageData}
onChange={this.onChangeCheckBox}
aria-label="Help improve the Emulator by allowing us to collect usage data, Data Collection"
label="Help improve the Emulator by allowing us to collect usage data."
name="collectUsageData"
/>
Expand Down
8 changes: 1 addition & 7 deletions packages/app/client/src/ui/editor/editor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -63,13 +63,7 @@ export class EditorFactory extends React.Component<EditorFactoryProps> {
);

case Constants.CONTENT_TYPE_APP_SETTINGS:
return (
<AppSettingsEditorContainer
documentId={document.documentId}
dirty={this.props.document.dirty}
refreshHash={Date.now().toString()}
/>
);
return <AppSettingsEditorContainer documentId={document.documentId} dirty={this.props.document.dirty} />;

case Constants.CONTENT_TYPE_WELCOME_PAGE:
return <WelcomePageContainer documentId={document.documentId} />;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ describe('<ActivityWrapper />', () => {
it('has the right a11y attributes', () => {
const component = render().find('.chat-activity');

expect(component.prop('role')).toEqual('checkbox');
expect(component.prop('role')).toEqual('region');
expect(component.prop('tabIndex')).toEqual(0);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ export class ActivityWrapper extends Component<ActivityWrapperProps> {
let classes = styles.chatActivity;
const restartConversationBubble = (
<div className={[styles.replayBubble, showRestartBubble ? '' : styles.hidden].join(' ')}>
<LinkButton ariaLabel="Restart from activity." linkRole={false} onClick={this.replayConversation}>
<LinkButton ariaLabel="Restart conversation from here." linkRole={false} onClick={this.replayConversation}>
Restart conversation from here
</LinkButton>
</div>
Expand All @@ -86,7 +86,7 @@ export class ActivityWrapper extends Component<ActivityWrapperProps> {
className={classes}
onClick={this.setSelectedActivity}
onKeyDown={this.onKeyDown}
role="checkbox"
role="region"
tabIndex={0}
title={'activity'}
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
height: 175px;
-webkit-mask: url('../../media/ic_bot_framework.svg');
-webkit-mask-size: 175px;
background-color: #007ACC;
background-color: #0063a6;
}
}

Expand Down
4 changes: 2 additions & 2 deletions packages/app/client/src/ui/shell/appMenu/appMenu.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ describe('<AppMenu />', () => {
expect(updateItem.disabled).toBe(false);

updateItem.onClick();
expect(dispatchSpy).toHaveBeenCalledWith(executeCommand(true, CheckForUpdates, null));
expect(dispatchSpy).toHaveBeenCalledWith(executeCommand(true, CheckForUpdates, expect.any(Function)));
});

it('should get an app update menu item for when the updater is idle', () => {
Expand All @@ -235,7 +235,7 @@ describe('<AppMenu />', () => {
expect(updateItem.disabled).toBe(false);

updateItem.onClick();
expect(dispatchSpy).toHaveBeenCalledWith(executeCommand(true, CheckForUpdates, null));
expect(dispatchSpy).toHaveBeenCalledWith(executeCommand(true, CheckForUpdates, expect.any(Function)));
});

it('should get a sign in menu item for a signed out user', async () => {
Expand Down
7 changes: 4 additions & 3 deletions packages/app/client/src/ui/shell/appMenu/appMenuContainer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,10 @@ function mapStateToProps(state: RootState): AppMenuProps {

function mapDispatchToProps(dispatch): AppMenuProps {
return {
checkForUpdates: () => {
dispatch(executeCommand(true, CheckForUpdates, null));
},
checkForUpdates: () =>
new Promise(resolve => {
dispatch(executeCommand(true, CheckForUpdates, resolve));
}),
invalidateAzureArmToken: () =>
new Promise(resolve => {
dispatch(executeCommand(false, InvalidateAzureArmToken, resolve));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ interface TabbedDocumentContentState {
}

class TabbedDocumentContentWrapperComponent extends Component<TabbedDocumentContentProps, TabbedDocumentContentState> {
contentRef = React.createRef<HTMLDivElement>();

constructor(props: TabbedDocumentContentProps) {
super(props);

Expand All @@ -79,6 +81,7 @@ class TabbedDocumentContentWrapperComponent extends Component<TabbedDocumentCont
hidden={this.props.hidden}
aria-hidden={this.props.hidden}
onClickCapture={this.onClick}
ref={this.contentRef}
>
{this.props.children}
<ContentOverlay documentId={this.props.documentId} />
Expand All @@ -99,6 +102,14 @@ class TabbedDocumentContentWrapperComponent extends Component<TabbedDocumentCont
}
}

public componentDidUpdate(prevProps: Readonly<TabbedDocumentContentProps>): void {
if (!this.props.hidden && prevProps.hidden && this.contentRef.current) {
(this.contentRef.current.querySelector(
'input, button:not([role="link"]), textarea, [tab-index]'
) as HTMLElement | null)?.focus();
}
}

private onClick = () => {
if (this.state.owningEditor !== this.props.activeEditor) {
this.props.setActiveEditor(this.state.owningEditor);
Expand Down
6 changes: 3 additions & 3 deletions packages/app/client/src/ui/styles/themes/high-contrast.css
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ html {
--inspector-link-color: var(--link-color);

/* Split Button */
--split-button-color: var(--neutral-5);
--split-button-color: var(--neutral-1);
--split-button-pressed-color: var(--neutral-1);
--split-button-disabled-color: var(--neutral-7);
--split-button-icon-color: #00BCF2;
Expand Down Expand Up @@ -314,6 +314,6 @@ html {
--custom-activity-editor-border: var(--neutral-10);
}

.dialog .ms-Button-label {
color: var(--neutral-15);
*, *::after, *::before {
forced-color-adjust: none;
}
6 changes: 3 additions & 3 deletions packages/app/client/src/ui/styles/themes/light.css
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ html {

/* Log panel custom colors */
--log-panel-timestamp: #327E36;
--log-panel-link: #007ACC;
--log-panel-link: #0063a6;
--log-panel-entry-hover-bg: var(--neutral-4);
--log-panel-entry-inspected-bg: var(--neutral-5);
--log-panel-item-info: var(--neutral-16);
Expand Down Expand Up @@ -232,12 +232,12 @@ html {
--status-bar-color: #FFFFFF;

/* Links */
--link-color: #007ACC;
--link-color: #0063a6;
--link-color-disabled: #C8C8C8;
--inspector-link-color: var(--link-color);

/* Split Button */
--split-button-color: #605e5c;
--split-button-color: var(--neutral-14);
--split-button-pressed-color: var(--neutral-16);
--split-button-disabled-color: var(--neutral-7);
--split-button-icon-color: #3062D6;
Expand Down
6 changes: 4 additions & 2 deletions packages/app/main/src/appUpdater.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ describe('AppUpdater', () => {

it('should get userInitiated', () => {
const tmp = (AppUpdater as any)._userInitiated;
(AppUpdater as any)._userInitiated = true;
(AppUpdater as any)._userInitiatedResolver = () => void 0;

expect(AppUpdater.userInitiated).toBe(true);
(AppUpdater as any)._userInitiated = tmp;
Expand Down Expand Up @@ -257,7 +257,9 @@ describe('AppUpdater', () => {

it('should check for updates from the stable release repo', async () => {
const mockSetFeedURL = jest.fn((_options: any) => null);
const mockCheckForUpdates = jest.fn(() => Promise.resolve());
const mockCheckForUpdates = jest.fn(async () => {
AppUpdater._userInitiatedResolver();
});
mockAutoUpdater.setFeedURL = mockSetFeedURL;
mockAutoUpdater.checkForUpdates = mockCheckForUpdates;

Expand Down
26 changes: 23 additions & 3 deletions packages/app/main/src/appUpdater.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class EmulatorUpdater extends EventEmitter {
@CommandServiceInstance()
private commandService: CommandServiceImpl;

private _userInitiated: boolean;
private _userInitiatedResolver: () => void;
private _autoDownload: boolean;
private _updaterStatus: UpdateStatus = UpdateStatus.Idle;
private _allowPrerelease: boolean;
Expand All @@ -58,7 +58,7 @@ class EmulatorUpdater extends EventEmitter {
private _installAfterDownload: boolean;

public get userInitiated(): boolean {
return this._userInitiated;
return !!this._userInitiatedResolver;
}

public get status(): UpdateStatus {
Expand Down Expand Up @@ -120,7 +120,18 @@ class EmulatorUpdater extends EventEmitter {
const settings = getSettings().framework;
this.allowPrerelease = !!settings.usePrereleases;
this.autoDownload = !!settings.autoUpdate;
this._userInitiated = userInitiated;
const updatePromise = new Promise<void>(resolve => {
if (userInitiated) {
const oneTimeResolver = () => {
resolve();
this._userInitiatedResolver = () => void 0;
};
this._userInitiatedResolver = oneTimeResolver;
} else {
this._userInitiatedResolver = undefined;
resolve();
}
});

electronUpdater.setFeedURL({
repo: this.repo,
Expand All @@ -134,6 +145,7 @@ class EmulatorUpdater extends EventEmitter {
auto: !userInitiated,
prerelease: this.allowPrerelease,
});
await updatePromise;
} catch (e) {
throw new Error(`There was an error while checking for the latest update: ${e}`);
}
Expand Down Expand Up @@ -170,6 +182,10 @@ class EmulatorUpdater extends EventEmitter {
} = SharedConstants.Commands.UI;
const result = await this.commandService.remoteCall<number>(ShowUpdateAvailableDialog, updateInfo.version);

if (this.userInitiated) {
this._userInitiatedResolver();
}

switch (result) {
// manually download the update, and the user can choose to install afterwards
case 0: {
Expand Down Expand Up @@ -240,6 +256,9 @@ class EmulatorUpdater extends EventEmitter {
const { ShowUpdateUnavailableDialog } = SharedConstants.Commands.UI;
await this.commandService.remoteCall(ShowUpdateUnavailableDialog);
}
if (userInitiated) {
this._userInitiatedResolver();
}
} catch (e) {
// eslint-disable-next-line no-console
console.error(`An error occurred in the updater's "up-to-date" event handler: ${e}`);
Expand All @@ -260,6 +279,7 @@ class EmulatorUpdater extends EventEmitter {
title: app.getName(),
message: `An error occurred while using the updater: ${err}`,
});
this._userInitiatedResolver();
}
};

Expand Down
4 changes: 2 additions & 2 deletions packages/app/main/src/commands/electronCommands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ export class ElectronCommands {
// ---------------------------------------------------------------------------
// Checks for app updates
@Command(Commands.CheckForUpdates)
protected checkForUpdates(userInitiated = true): void {
AppUpdater.checkForUpdates(userInitiated);
protected checkForUpdates(userInitiated = true): Promise<void> {
return AppUpdater.checkForUpdates(userInitiated);
}
}
4 changes: 2 additions & 2 deletions packages/extensions/json/src/themes/light.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@
//

// Colors we care about
const booleanNumber = '#47B07F';
const keys = '#007ACC';
const booleanNumber = '#35835f';
const keys = '#0063a6';
const string = '#A1260D';
const background = '#00000000';

Expand Down
6 changes: 5 additions & 1 deletion packages/sdk/ui-react/src/media/ic_chevron_up.svg
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.

0 comments on commit 515fc62

Please sign in to comment.