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

Desktop: Fix errors found by automated accessibility testing #11246

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
8d4a946
Desktop: Accessibility: Declare app locale with the HTML lang attribute
personalizedrefrigerator Oct 15, 2024
1b00c77
Merge branch 'pr/desktop/accessibility/declare-app-lang' into HEAD
personalizedrefrigerator Oct 23, 2024
6c9b78b
Desktop: Accessibility: Fix errors found by automated accessibility t…
personalizedrefrigerator Oct 23, 2024
75f112a
Merge commit 'a2d0908eb60d1569aea7803c254b2df2f2799526' into pr/deskt…
personalizedrefrigerator Oct 28, 2024
015adbf
Merge branch 'dev' into pr/desktop/automated-accessibility-testing
personalizedrefrigerator Oct 28, 2024
b197ee0
Merge branch 'dev' into pr/desktop/automated-accessibility-testing
laurent22 Oct 28, 2024
662b373
Improve test reliability
personalizedrefrigerator Oct 29, 2024
332dc4e
Merge remote-tracking branch 'refs/remotes/origin/pr/desktop/automate…
personalizedrefrigerator Oct 29, 2024
92dcb32
Merge branch 'dev' into pr/desktop/automated-accessibility-testing
laurent22 Oct 30, 2024
1c1b9d9
Merge remote-tracking branch 'upstream/dev' into pr/desktop/automated…
personalizedrefrigerator Oct 31, 2024
84a33de
Update yarn.lock post-merge
personalizedrefrigerator Oct 31, 2024
4a1bc2f
Merge branch 'dev' into pr/desktop/automated-accessibility-testing
personalizedrefrigerator Nov 4, 2024
ec6c1cb
Merge branch 'dev' into pr/desktop/automated-accessibility-testing
laurent22 Nov 4, 2024
399f33a
Merge branch 'dev' into pr/desktop/automated-accessibility-testing
personalizedrefrigerator Nov 4, 2024
44af494
Merge branch 'dev' into pr/desktop/automated-accessibility-testing
laurent22 Nov 5, 2024
d5f4af2
Merge remote-tracking branch 'upstream/dev' into pr/desktop/automated…
personalizedrefrigerator Nov 8, 2024
77c7308
Fix tests post merge
personalizedrefrigerator Nov 8, 2024
c1e65dc
Fixing tests
personalizedrefrigerator Nov 9, 2024
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
2 changes: 2 additions & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
Expand Up @@ -510,10 +510,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 @@ -487,10 +487,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
10 changes: 6 additions & 4 deletions packages/app-desktop/gui/MainScreen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -633,10 +633,12 @@ class MainScreenComponent extends React.Component<Props, State> {
},

editor: () => {
return <NoteEditor
windowId={defaultWindowId}
key={key}
/>;
return <div className='note-editor-wrapper' role='main' aria-label={_('Note')}>
<NoteEditor
windowId={defaultWindowId}
key={key}
/>
</div>;
},
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,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 @@ -116,6 +116,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 @@ -282,12 +282,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 @@ -178,7 +178,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 @@ -6,6 +6,7 @@ import { focus } from '@joplin/lib/utils/focusHandler';
import { ForwardedRef, forwardRef, RefObject, useContext, useEffect, useImperativeHandle, useMemo, useRef, useState } from 'react';
import { WindowIdContext } from './NewWindowOrIFrame';
import useDocument from './hooks/useDocument';
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 @@ -224,6 +225,7 @@ const NoteTextViewer = forwardRef((props: Props, ref: ForwardedRef<NoteViewerCon
style={viewerStyle}
allow='clipboard-write=(self) fullscreen=(self) autoplay=(self) local-fonts=(self) encrypted-media=(self)'
allowFullScreen={true}
aria-label={_('Note editor')}
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,4 +9,5 @@
@use './editor-toolbar.scss';
@use './user-webview-dialog-container.scss';
@use './dialog-anchor-node.scss';
@use './note-editor-wrapper.scss';
@use './text-input.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 @@ -61,6 +62,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
68 changes: 68 additions & 0 deletions packages/app-desktop/integration-tests/wcag.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
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);
};

// Fade-in transitions can cause color contrast issues if still running
// during a scan.
// See https://github.com/dequelabs/axe-core-npm/issues/952
const waitForAnimationsToEnd = (page: Page) => {
return page.locator('body').evaluate(element => {
const animationPromises = element
.getAnimations({ subtree: true })
.map(animation => animation.finished);
return Promise.all(animationPromises);
});
};

const expectNoViolations = async (page: Page) => {
await waitForAnimationsToEnd(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();

await mainScreen.createNewNote('Test');

// 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();

// Ensure that `:hover` styling is consistent between tests:
await mainScreen.noteEditor.noteTitleInput.hover();

await expectNoViolations(mainWindow);
});
});

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.2",
"@joplin/tools": "~3.2",
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 @@ -348,6 +348,8 @@ function NoteEditor(props: Props, ref: any) {
autocompleteMarkup: Setting.value('editor.autocompleteMarkup'),

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 @@ -56,6 +56,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 @@ -17,6 +17,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 @@ -168,6 +168,8 @@ export interface EditorSettings {
readOnly: boolean;

indentWithTabs: boolean;

editorLabel: string;
}

export type LogMessageCallback = (message: string)=> void;
Expand Down
Loading
Loading