Skip to content

Commit

Permalink
Desktop: Accessibility: Fix errors found by automated accessibility t…
Browse files Browse the repository at this point in the history
…esting
  • Loading branch information
personalizedrefrigerator committed Oct 23, 2024
1 parent 1b00c77 commit 6c9b78b
Show file tree
Hide file tree
Showing 34 changed files with 134 additions and 20 deletions.
2 changes: 2 additions & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
Expand Up @@ -488,10 +488,12 @@ packages/app-desktop/integration-tests/util/createStartupArgs.js
packages/app-desktop/integration-tests/util/extendedExpect.js
packages/app-desktop/integration-tests/util/firstNonDevToolsWindow.js
packages/app-desktop/integration-tests/util/getImageSourceSize.js
packages/app-desktop/integration-tests/util/setDarkMode.js
packages/app-desktop/integration-tests/util/setFilePickerResponse.js
packages/app-desktop/integration-tests/util/setMessageBoxResponse.js
packages/app-desktop/integration-tests/util/test.js
packages/app-desktop/integration-tests/util/waitForNextOpenPath.js
packages/app-desktop/integration-tests/wcag.spec.js
packages/app-desktop/playwright.config.js
packages/app-desktop/plugins/GotoAnything.js
packages/app-desktop/services/autoUpdater/AutoUpdaterService.test.js
Expand Down
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -465,10 +465,12 @@ packages/app-desktop/integration-tests/util/createStartupArgs.js
packages/app-desktop/integration-tests/util/extendedExpect.js
packages/app-desktop/integration-tests/util/firstNonDevToolsWindow.js
packages/app-desktop/integration-tests/util/getImageSourceSize.js
packages/app-desktop/integration-tests/util/setDarkMode.js
packages/app-desktop/integration-tests/util/setFilePickerResponse.js
packages/app-desktop/integration-tests/util/setMessageBoxResponse.js
packages/app-desktop/integration-tests/util/test.js
packages/app-desktop/integration-tests/util/waitForNextOpenPath.js
packages/app-desktop/integration-tests/wcag.spec.js
packages/app-desktop/playwright.config.js
packages/app-desktop/plugins/GotoAnything.js
packages/app-desktop/services/autoUpdater/AutoUpdaterService.test.js
Expand Down
3 changes: 2 additions & 1 deletion packages/app-desktop/gui/Button/Button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,8 @@ const Button = React.forwardRef((props: Props, ref: any) => {
function renderIcon() {
if (!props.iconName) return null;
return <StyledIcon
aria-label={props.iconLabel ?? ''}
aria-label={props.iconLabel ?? undefined}
aria-hidden={!props.iconLabel}
animation={props.iconAnimation}
mr={iconOnly ? '0' : '6px'}
color={props.color}
Expand Down
2 changes: 1 addition & 1 deletion packages/app-desktop/gui/ConfigScreen/ConfigScreen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -438,7 +438,7 @@ class ConfigScreenComponent extends React.Component<any, any> {
}

return (
<div className="config-screen" style={{ display: 'flex', flexDirection: 'row', height: this.props.style.height }}>
<div className="config-screen" role="main" style={{ display: 'flex', flexDirection: 'row', height: this.props.style.height }}>
<Sidebar
selection={this.state.selectedSectionName}
onSelectionChange={this.sidebar_selectionChange}
Expand Down
4 changes: 2 additions & 2 deletions packages/app-desktop/gui/ConfigScreen/Sidebar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ export const StyledDivider = styled.div`
border-bottom: 1px solid ${(props: StyleProps) => props.theme.dividerColor};
background-color: ${(props: StyleProps) => props.theme.selectedColor2};
font-size: ${(props: StyleProps) => Math.round(props.theme.fontSize)}px;
opacity: 0.5;
opacity: 0.58;
`;

export const StyledListItemLabel = styled.span`
Expand Down Expand Up @@ -131,9 +131,9 @@ export default function Sidebar(props: Props) {
onKeyDown={onKeyDown}
>
<StyledListItemIcon
aria-label=''
className={Setting.sectionNameToIcon(section.name, AppType.Desktop)}
role='img'
aria-hidden='true'
/>
<StyledListItemLabel>
{Setting.sectionNameToLabel(section.name)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ interface Props {
}

const SettingDescription: React.FC<Props> = props => {
return props.text ? <div className='setting-description' id={props.id}>{props.text}</div> : null;
return <div className={`setting-description ${!props.text ? '-empty' : ''}`} id={props.id}>{props.text}</div>;
};

export default SettingDescription;
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,9 @@
font-style: italic;
max-width: 70em;
margin-top: 5px;

&.-empty {
margin: 0;
padding: 0;
}
}
2 changes: 1 addition & 1 deletion packages/app-desktop/gui/FolderIconBox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export default function(props: Props) {
} else if (folderIcon.type === FolderIconType.DataUrl) {
return <img style={{ width, height, opacity }} src={folderIcon.dataUrl} />;
} else if (folderIcon.type === FolderIconType.FontAwesome) {
return <i style={{ fontSize: 18, width, opacity }} className={folderIcon.name} role='img'></i>;
return <i style={{ fontSize: 18, width, opacity }} className={folderIcon.name} role='img' aria-hidden={true}></i>;
} else {
throw new Error(`Unsupported folder icon type: ${folderIcon.type}`);
}
Expand Down
1 change: 0 additions & 1 deletion packages/app-desktop/gui/ItemList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,6 @@ class ItemList<ItemType> extends React.Component<Props<ItemType>, State> {
id={this.props.id}
role={this.props.role}
aria-label={this.props['aria-label']}
aria-setsize={items.length}

onScroll={this.onScroll}
onKeyDown={this.onKeyDown}
Expand Down
4 changes: 3 additions & 1 deletion packages/app-desktop/gui/MainScreen/MainScreen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -791,7 +791,9 @@ class MainScreenComponent extends React.Component<Props, State> {
} else if (this.props.settingEditorCodeView && this.props.enableLegacyMarkdownEditor) {
bodyEditor = 'CodeMirror5';
}
return <NoteEditor key={key} bodyEditor={bodyEditor} />;
return <div className='note-editor-wrapper' role='main' aria-label={_('Note')}>
<NoteEditor key={key} bodyEditor={bodyEditor} />
</div>;
},
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,7 @@ const CodeMirror = (props: NoteBodyEditorProps, ref: ForwardedRef<NoteBodyEditor
spellcheckEnabled: Setting.value('editor.spellcheckBeta'),
keymap: keyboardMode,
indentWithTabs: true,
editorLabel: _('Markdown editor'),
};
}, [
props.contentMarkupLanguage, props.disabled, props.keyboardMode, styles.globalTheme,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ export default function NoteTitleBar(props: Props) {
type="text"
ref={props.titleInputRef}
placeholder={props.isProvisional ? (props.noteIsTodo ? _('Creating new to-do...') : _('Creating new note...')) : ''}
aria-label={props.isProvisional ? undefined : _('Note title')}
style={styles.titleInput}
readOnly={props.disabled}
onChange={props.onTitleChange}
Expand Down
9 changes: 5 additions & 4 deletions packages/app-desktop/gui/NoteList/NoteList2.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -281,12 +281,13 @@ const NoteList = (props: Props) => {
onItemContextMenu({ itemId: activeNoteId });
}, [onItemContextMenu, activeNoteId]);

const hasNotes = !!props.notes.length;
return (
<div
role='listbox'
aria-label={_('Notes')}
aria-activedescendant={getNoteElementIdFromJoplinId(activeNoteId)}
aria-multiselectable={true}
role={hasNotes ? 'listbox' : 'group'}
aria-label={hasNotes ? _('Notes') : null}
aria-activedescendant={activeNoteId ? getNoteElementIdFromJoplinId(activeNoteId) : undefined}
aria-multiselectable={hasNotes ? true : undefined}
tabIndex={0}

onFocus={onFocus}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ export default function NoteListWrapper(props: Props) {
};

return (
<StyledRoot>
<StyledRoot role='navigation' aria-label={_('Note list')}>
<NoteListControls
height={controlHeight}
width={noteListSize.width}
Expand Down
2 changes: 2 additions & 0 deletions packages/app-desktop/gui/NoteTextViewer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import * as React from 'react';
import { reg } from '@joplin/lib/registry';
import bridge from '../services/bridge';
import { focus } from '@joplin/lib/utils/focusHandler';
import { _ } from '@joplin/lib/locale';

interface Props {
// eslint-disable-next-line @typescript-eslint/ban-types -- Old code before rule was applied
Expand Down Expand Up @@ -234,6 +235,7 @@ export default class NoteTextViewerComponent extends React.Component<Props, any>
style={viewerStyle}
allow='fullscreen=* autoplay=* local-fonts=* encrypted-media=*'
allowFullScreen={true}
aria-label={_('Rendered note')}
src={`joplin-content://note-viewer/${__dirname}/note-viewer/index.html`}
></iframe>
);
Expand Down
2 changes: 1 addition & 1 deletion packages/app-desktop/gui/Sidebar/Sidebar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ const SidebarComponent = (props: Props) => {
);

return (
<StyledRoot className="sidebar">
<StyledRoot className="sidebar" role='navigation' aria-label={_('Sidebar')}>
<div style={{ flex: 1 }}><FolderAndTagList/></div>
<div style={{ flex: 0, padding: theme.mainPadding }}>
{syncReportComp}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ const useOnRenderListWrapper = ({ selectedIndex, onKeyDown }: Props) => {
<div
role='tree'
className='sidebar-list-items-wrapper'
aria-setsize={listItems.length}
tabIndex={allowContainerFocus ? 0 : undefined}
onKeyDown={onKeyDown}
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ const AllNotesItem: React.FC<Props> = props => {
itemCount={props.itemCount}
>
<EmptyExpandLink/>
<StyledAllNotesIcon aria-label='' role='img' className='icon-notes'/>
<StyledAllNotesIcon aria-hidden='true' role='img' className='icon-notes'/>
<StyledListItemAnchor
className="list-item"
isSpecialItem={true}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ const HeaderItem: React.FC<Props> = props => {
onDrop={props.onDrop}
>
<StyledHeader onClick={onClick}>
<StyledHeaderIcon aria-label='' role='img' className={item.iconName}/>
<StyledHeaderIcon aria-hidden='true' role='img' className={item.iconName}/>
<StyledHeaderLabel>{item.label}</StyledHeaderLabel>
</StyledHeader>
</ListItemWrapper>
Expand Down
2 changes: 1 addition & 1 deletion packages/app-desktop/gui/ToolbarButton/ToolbarButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export default function ToolbarButton(props: Props) {
const iconName = getProp(props, 'iconName');
if (iconName) {
const iconProps: React.HTMLProps<HTMLDivElement> = {
'aria-label': '',
'aria-hidden': true,
role: 'img',
className: `toolbar-icon ${title ? '-has-title' : ''} ${iconName}`,
};
Expand Down
1 change: 1 addition & 0 deletions packages/app-desktop/gui/styles/index.scss
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@
@use './editor-toolbar.scss';
@use './user-webview-dialog-container.scss';
@use './dialog-anchor-node.scss';
@use './note-editor-wrapper.scss';
6 changes: 6 additions & 0 deletions packages/app-desktop/gui/styles/note-editor-wrapper.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@

.note-editor-wrapper {
flex-grow: 1;
height: 100%;
width: 100%;
}
2 changes: 2 additions & 0 deletions packages/app-desktop/integration-tests/models/Sidebar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@ import { ElectronApplication, Locator, Page } from '@playwright/test';

export default class Sidebar {
public readonly container: Locator;
public readonly allNotes: Locator;

public constructor(page: Page, private mainScreen: MainScreen) {
this.container = page.locator('.rli-sideBar');
this.allNotes = this.container.getByText('All notes');
}

public async createNewFolder(title: string) {
Expand Down
9 changes: 9 additions & 0 deletions packages/app-desktop/integration-tests/util/setDarkMode.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { ElectronApplication } from '@playwright/test';

const setDarkMode = (app: ElectronApplication, darkMode: boolean) => {
return app.evaluate(({ nativeTheme }, darkMode) => {
nativeTheme.themeSource = darkMode ? 'dark' : 'light';
}, darkMode);
};

export default setDarkMode;
2 changes: 2 additions & 0 deletions packages/app-desktop/integration-tests/util/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { _electron as electron, Page, ElectronApplication, test as base } from '
import uuid from '@joplin/lib/uuid';
import createStartupArgs from './createStartupArgs';
import firstNonDevToolsWindow from './firstNonDevToolsWindow';
import setDarkMode from './setDarkMode';


type StartWithPluginsResult = { app: ElectronApplication; mainWindow: Page };
Expand Down Expand Up @@ -53,6 +54,7 @@ export const test = base.extend<JoplinFixtures>({
electronApp: async ({ profileDirectory }, use) => {
const startupArgs = createStartupArgs(profileDirectory);
const electronApp = await electron.launch({ args: startupArgs });
await setDarkMode(electronApp, false);

await use(electronApp);

Expand Down
52 changes: 52 additions & 0 deletions packages/app-desktop/integration-tests/wcag.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import { test, expect } from './util/test';
import MainScreen from './models/MainScreen';
import SettingsScreen from './models/SettingsScreen';
import AxeBuilder from '@axe-core/playwright';
import { Page } from '@playwright/test';

const createScanner = (page: Page) => {
return new AxeBuilder({ page })
.disableRules(['page-has-heading-one'])
.setLegacyMode(true);
};

const expectNoViolations = async (page: Page) => {
const results = await createScanner(page).analyze();
expect(results.violations).toEqual([]);
};

test.describe('wcag', () => {
for (const tabName of ['General', 'Plugins']) {
test(`should not detect significant issues in the settings screen ${tabName} tab`, async ({ electronApp, mainWindow }) => {
const mainScreen = new MainScreen(mainWindow);
await mainScreen.waitFor();

await mainScreen.openSettings(electronApp);

// Should be on the settings screen
const settingsScreen = new SettingsScreen(mainWindow);
await settingsScreen.waitFor();

const tabLocator = settingsScreen.getTabLocator(tabName);
await tabLocator.click();
await expect(tabLocator).toBeFocused();

await expectNoViolations(mainWindow);
});
}

test('should not detect significant issues in the main screen with an open note', async ({ mainWindow }) => {
const mainScreen = new MainScreen(mainWindow);
await mainScreen.waitFor();

// For now, activate all notes to make it active. When inactive, it causes a contrast warning.
// This seems to be allowed under WCAG 2.2 SC 1.4.3 under the "Incidental" exception.
await mainScreen.sidebar.allNotes.click();
await mainScreen.createNewNote('Test');

const results = await createScanner(mainWindow)
.analyze();
expect(results.violations).toEqual([]);
});
});

1 change: 1 addition & 0 deletions packages/app-desktop/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@
"homepage": "https://github.com/laurent22/joplin#readme",
"devDependencies": {
"7zip-bin": "5.2.0",
"@axe-core/playwright": "4.10.0",
"@electron/rebuild": "3.6.0",
"@joplin/default-plugins": "~3.1",
"@joplin/tools": "~3.1",
Expand Down
2 changes: 2 additions & 0 deletions packages/app-mobile/components/NoteEditor/NoteEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,8 @@ function NoteEditor(props: Props, ref: any) {
ignoreModifiers: false,

indentWithTabs: true,

editorLabel: _('Markdown editor'),
}), [props.themeId, props.readOnly]);

const injectedJavaScript = `
Expand Down
1 change: 1 addition & 0 deletions packages/editor/CodeMirror/configFromSettings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ const configFromSettings = (settings: EditorSettings) => {
autocapitalize: 'sentence',
autocorrect: settings.spellcheckEnabled ? 'true' : 'false',
spellcheck: settings.spellcheckEnabled ? 'true' : 'false',
'aria-label': settings.editorLabel,
}),
EditorState.readOnly.of(settings.readOnly),
indentUnit.of(settings.indentWithTabs ? '\t' : ' '),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ const createEditorSettings = (themeId: number) => {
themeData,

indentWithTabs: true,
editorLabel: 'Markdown editor',
};

return editorSettings;
Expand Down
2 changes: 2 additions & 0 deletions packages/editor/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,8 @@ export interface EditorSettings {
readOnly: boolean;

indentWithTabs: boolean;

editorLabel: string;
}

export type LogMessageCallback = (message: string)=> void;
Expand Down
4 changes: 2 additions & 2 deletions packages/lib/themes/light.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const theme: Theme = {
colorCorrect: 'green', // Opposite of colorError
colorWarn: 'rgb(228,86,0)',
colorWarnUrl: '#155BDA',
colorFaded: '#7C8B9E', // For less important text
colorFaded: '#627184', // For less important text
dividerColor: '#dddddd',
selectedColor: '#e5e5e5',
urlColor: '#155BDA',
Expand All @@ -32,7 +32,7 @@ const theme: Theme = {
// It's dark text over gray background.
backgroundColor3: '#F4F5F6',
backgroundColorHover3: '#CBDAF1',
color3: '#738598',
color3: '#627284',

// Color scheme "4" is used for secondary-style buttons. It makes a white
// button with blue text.
Expand Down
1 change: 1 addition & 0 deletions packages/tools/cspell/dictionary4.txt
Original file line number Diff line number Diff line change
Expand Up @@ -133,3 +133,4 @@ rcompare
tabindex
Backblaze
treeitem
WCAG
Loading

0 comments on commit 6c9b78b

Please sign in to comment.