1
0
mirror of https://github.com/laurent22/joplin.git synced 2024-12-21 09:38:01 +02:00

Desktop: Accessibility: Improve focus handling for plugin and prompt dialogs (#10801)

This commit is contained in:
Henry Heino 2024-07-31 06:10:58 -07:00 committed by GitHub
parent ecc4f3e22a
commit 596bcd8d8b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
21 changed files with 230 additions and 165 deletions

View File

@ -451,7 +451,9 @@ packages/app-desktop/gui/utils/convertToScreenCoordinates.js
packages/app-desktop/gui/utils/dragAndDrop.js
packages/app-desktop/gui/utils/loadScript.js
packages/app-desktop/gulpfile.js
packages/app-desktop/integration-tests/goToAnything.spec.js
packages/app-desktop/integration-tests/main.spec.js
packages/app-desktop/integration-tests/models/GoToAnything.js
packages/app-desktop/integration-tests/models/MainScreen.js
packages/app-desktop/integration-tests/models/NoteEditorScreen.js
packages/app-desktop/integration-tests/models/SettingsScreen.js
@ -469,6 +471,7 @@ packages/app-desktop/integration-tests/util/test.js
packages/app-desktop/integration-tests/util/waitForNextOpenPath.js
packages/app-desktop/playwright.config.js
packages/app-desktop/plugins/GotoAnything.js
packages/app-desktop/services/autoUpdater/AutoUpdaterService.js
packages/app-desktop/services/bridge.js
packages/app-desktop/services/commands/stateToWhenClauseContext.js
packages/app-desktop/services/commands/types.js

3
.gitignore vendored
View File

@ -430,7 +430,9 @@ packages/app-desktop/gui/utils/convertToScreenCoordinates.js
packages/app-desktop/gui/utils/dragAndDrop.js
packages/app-desktop/gui/utils/loadScript.js
packages/app-desktop/gulpfile.js
packages/app-desktop/integration-tests/goToAnything.spec.js
packages/app-desktop/integration-tests/main.spec.js
packages/app-desktop/integration-tests/models/GoToAnything.js
packages/app-desktop/integration-tests/models/MainScreen.js
packages/app-desktop/integration-tests/models/NoteEditorScreen.js
packages/app-desktop/integration-tests/models/SettingsScreen.js
@ -448,6 +450,7 @@ packages/app-desktop/integration-tests/util/test.js
packages/app-desktop/integration-tests/util/waitForNextOpenPath.js
packages/app-desktop/playwright.config.js
packages/app-desktop/plugins/GotoAnything.js
packages/app-desktop/services/autoUpdater/AutoUpdaterService.js
packages/app-desktop/services/bridge.js
packages/app-desktop/services/commands/stateToWhenClauseContext.js
packages/app-desktop/services/commands/types.js

View File

@ -1,10 +1,11 @@
import * as React from 'react';
import { ReactElement, ReactEventHandler, useCallback, useEffect, useRef, useState } from 'react';
import { MouseEventHandler, ReactEventHandler, ReactNode, useCallback, useEffect, useRef, useState } from 'react';
interface Props {
renderContent: ()=> ReactElement;
className?: string;
onClose?: ()=> void;
onCancel?: ()=> void;
contentStyle?: React.CSSProperties;
children: ReactNode;
}
export default function Dialog(props: Props) {
@ -18,12 +19,12 @@ export default function Dialog(props: Props) {
dialogElement.showModal();
}, [dialogElement]);
const onCloseRef = useRef(props.onClose);
onCloseRef.current = props.onClose;
const onCancelRef = useRef(props.onCancel);
onCancelRef.current = props.onCancel;
const onCancel: ReactEventHandler<HTMLDialogElement> = useCallback((event) => {
const canCancel = !!onCloseRef.current;
if (canCancel) {
const canCancel = !!onCancelRef.current;
if (!canCancel) {
// Prevents [Escape] from closing the dialog. In many places, this is handled
// elsewhere.
// See https://stackoverflow.com/a/61021326
@ -31,15 +32,23 @@ export default function Dialog(props: Props) {
}
}, []);
const onContainerClick: MouseEventHandler<HTMLDialogElement> = useCallback((event) => {
const onCancel = onCancelRef.current;
if (event.target === dialogElement && onCancel) {
onCancel();
}
}, [dialogElement]);
return (
<dialog
ref={setDialogRef}
className={`dialog-modal-layer ${props.className}`}
onClose={props.onClose}
onClose={props.onCancel}
onCancel={onCancel}
onClick={onContainerClick}
>
<div className='content'>
{props.renderContent()}
<div className='content' style={props.contentStyle}>
{props.children}
</div>
</dialog>
);

View File

@ -179,6 +179,6 @@ export default function(props: Props) {
}
return (
<Dialog onClose={onClose} className="master-password-dialog" renderContent={renderDialogWrapper}/>
<Dialog onCancel={onClose} className="master-password-dialog">{renderDialogWrapper()}</Dialog>
);
}

View File

@ -240,6 +240,6 @@ export default function(props: Props) {
}
return (
<Dialog onClose={onClose} className="master-password-dialog" renderContent={renderDialogWrapper}/>
<Dialog onCancel={onClose} className="master-password-dialog">{renderDialogWrapper()}</Dialog>
);
}

View File

@ -5,13 +5,13 @@ import DialogButtonRow from './DialogButtonRow';
const { themeStyle } = require('@joplin/lib/theme');
const Countable = require('@joplin/lib/countable/Countable');
import markupLanguageUtils from '../utils/markupLanguageUtils';
import Dialog from './Dialog';
interface NoteContentPropertiesDialogProps {
themeId: number;
text: string;
markupLanguage: number;
// eslint-disable-next-line @typescript-eslint/ban-types -- Old code before rule was applied
onClose: Function;
onClose: ()=> void;
}
interface TextPropertiesMap {
@ -159,22 +159,20 @@ export default function NoteContentPropertiesDialog(props: NoteContentProperties
const readTimeLabel = _('Read time: %s min', formatReadTime(strippedReadTime));
return (
<div style={theme.dialogModalLayer}>
<div style={theme.dialogBox}>
<div style={dialogBoxHeadingStyle}>{_('Statistics')}</div>
<table>
<thead>
{tableHeader}
</thead>
<tbody>
{tableBodyComps}
</tbody>
</table>
<div style={{ ...labelCompStyle, marginTop: 10 }}>
{readTimeLabel}
</div>
<DialogButtonRow themeId={props.themeId} onClick={buttonRow_click} okButtonShow={false} cancelButtonLabel={_('Close')}/>
<Dialog onCancel={props.onClose}>
<div style={dialogBoxHeadingStyle}>{_('Statistics')}</div>
<table>
<thead>
{tableHeader}
</thead>
<tbody>
{tableBodyComps}
</tbody>
</table>
<div style={{ ...labelCompStyle, marginTop: 10 }}>
{readTimeLabel}
</div>
</div>
<DialogButtonRow themeId={props.themeId} onClick={buttonRow_click} okButtonShow={false} cancelButtonLabel={_('Close')}/>
</Dialog>
);
}

View File

@ -8,14 +8,14 @@ import bridge from '../services/bridge';
import shim from '@joplin/lib/shim';
import { NoteEntity } from '@joplin/lib/services/database/types';
import { focus } from '@joplin/lib/utils/focusHandler';
import Dialog from './Dialog';
const Datetime = require('react-datetime').default;
const { clipboard } = require('electron');
const formatcoords = require('formatcoords');
interface Props {
noteId: string;
// eslint-disable-next-line @typescript-eslint/ban-types -- Old code before rule was applied
onClose: Function;
onClose: ()=> void;
// eslint-disable-next-line @typescript-eslint/ban-types -- Old code before rule was applied
onRevisionLinkClick: Function;
themeId: number;
@ -447,15 +447,13 @@ class NotePropertiesDialog extends React.Component<Props, State> {
}
return (
<div style={theme.dialogModalLayer}>
<div style={theme.dialogBox}>
<div style={theme.dialogTitle} id='note-properties-dialog-title'>{_('Note properties')}</div>
<div role='table' aria-labelledby='note-properties-dialog-title'>
{noteComps}
</div>
<DialogButtonRow themeId={this.props.themeId} okButtonShow={!this.isReadOnly()} okButtonRef={this.okButton} onClick={this.buttonRow_click}/>
<Dialog onCancel={this.props.onClose}>
<div style={theme.dialogTitle} id='note-properties-dialog-title'>{_('Note properties')}</div>
<div role='table' aria-labelledby='note-properties-dialog-title'>
{noteComps}
</div>
</div>
<DialogButtonRow themeId={this.props.themeId} okButtonShow={!this.isReadOnly()} okButtonRef={this.okButton} onClick={this.buttonRow_click}/>
</Dialog>
);
}
}

View File

@ -7,6 +7,8 @@ import CreatableSelect from 'react-select/creatable';
import Select from 'react-select';
import makeAnimated from 'react-select/animated';
import { focus } from '@joplin/lib/utils/focusHandler';
import Dialog from './Dialog';
interface Props {
themeId: number;
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
@ -90,31 +92,6 @@ export default class PromptDialog extends React.Component<Props, any> {
this.styles_ = {};
const paddingTop = 20;
this.styles_.modalLayer = {
zIndex: 9999,
position: 'absolute',
top: 0,
left: 0,
width: width,
height: height,
boxSizing: 'border-box',
backgroundColor: 'rgba(0,0,0,0.6)',
display: visible ? 'flex' : 'none',
alignItems: 'flex-start',
justifyContent: 'center',
paddingTop: `${paddingTop}px`,
};
this.styles_.promptDialog = {
backgroundColor: theme.backgroundColor,
padding: 16,
display: 'inline-block',
maxWidth: width * 0.5,
boxShadow: '6px 6px 20px rgba(0,0,0,0.5)',
};
this.styles_.button = {
minWidth: theme.buttonMinWidth,
minHeight: theme.buttonMinHeight,
@ -218,6 +195,8 @@ export default class PromptDialog extends React.Component<Props, any> {
}
public render() {
if (!this.state.visible) return null;
const style = this.props.style;
const theme = themeStyle(this.props.themeId);
const buttonTypes = this.props.buttons ? this.props.buttons : ['ok', 'cancel'];
@ -325,16 +304,14 @@ export default class PromptDialog extends React.Component<Props, any> {
}
return (
<div className="modal-layer" style={styles.modalLayer}>
<div className="modal-dialog" style={styles.promptDialog}>
<label style={styles.label}>{this.props.label ? this.props.label : ''}</label>
<div style={{ display: 'inline-block', color: 'black', backgroundColor: theme.backgroundColor }}>
{inputComp}
{descComp}
</div>
<div style={{ textAlign: 'right', marginTop: 10 }}>{buttonComps}</div>
<Dialog className='prompt-dialog'>
<label style={styles.label}>{this.props.label ? this.props.label : ''}</label>
<div style={{ display: 'inline-block', color: 'black', backgroundColor: theme.backgroundColor }}>
{inputComp}
{descComp}
</div>
</div>
<div style={{ textAlign: 'right', marginTop: 10 }}>{buttonComps}</div>
</Dialog>
);
}
}

View File

@ -167,7 +167,7 @@ class RootComponent extends React.Component<Props, any> {
);
};
return <Dialog renderContent={renderContent}/>;
return <Dialog>{renderContent()}</Dialog>;
}
private modalDialogProps(): ModalDialogProps {

View File

@ -407,7 +407,7 @@ function ShareFolderDialog(props: Props) {
}
return (
<Dialog renderContent={renderContent}/>
<Dialog>{renderContent()}</Dialog>
);
}

View File

@ -226,7 +226,7 @@ export function ShareNoteDialog(props: Props) {
};
return (
<Dialog renderContent={renderContent}/>
<Dialog>{renderContent()}</Dialog>
);
}

View File

@ -277,6 +277,6 @@ export default function(props: Props) {
}
return (
<Dialog onClose={closeDialog} renderContent={renderDialogWrapper}/>
<Dialog onCancel={closeDialog}>{renderDialogWrapper()}</Dialog>
);
}

View File

@ -13,6 +13,7 @@
> .content {
background-color: var(--joplin-background-color);
color: var(--joplin-color);
padding: 16px;
box-shadow: 6px 6px 20px rgba(0,0,0,0.5);
margin: 20px;
@ -23,6 +24,6 @@
}
&::backdrop {
background-color: rgba(0,0,0,0.6);
background-color: rgba(0,0,0,0.5);
}
}

View File

@ -1,2 +1,4 @@
@use './dialog-modal-layer.scss';
@use './dialog-modal-layer.scss';
@use './user-webview-dialog.scss';
@use './prompt-dialog.scss';

View File

@ -0,0 +1,7 @@
.prompt-dialog {
> .content {
display: inline-block;
max-width: 450px;
}
}

View File

@ -0,0 +1,25 @@
.user-webview-dialog {
// Overflow should be handled within WebView dialogs (rather than externally).
// Allowing external overflow can create space for a scrollbar in some cases.
overflow: unset;
align-items: center;
--content-width: 90vw;
--content-height: 90vh;
&.-fit {
--content-width: auto;
--content-height: auto;
}
> .content {
display: flex;
flex-direction: column;
background-color: var(--joplin-background-color);
padding: var(--joplin-main-padding);
border-radius: 4px;
box-shadow: 0 6px 10px #00000077;
width: var(--content-width);
height: var(--content-height);
}
}

View File

@ -0,0 +1,52 @@
import { test, expect } from './util/test';
import MainScreen from './models/MainScreen';
test.describe('goToAnything', () => {
test('clicking outside of go to anything should close it', async ({ electronApp, mainWindow }) => {
const mainScreen = new MainScreen(mainWindow);
await mainScreen.noteEditor.waitFor();
const goToAnything = mainScreen.goToAnything;
await goToAnything.open(electronApp);
await goToAnything.expectToBeOpen();
// Click outside of the dialog
await goToAnything.containerLocator.click({ position: { x: 0, y: 0 } });
await goToAnything.expectToBeClosed();
});
test('pressing escape in go to anything should close it ', async ({ electronApp, mainWindow }) => {
const mainScreen = new MainScreen(mainWindow);
const goToAnything = mainScreen.goToAnything;
// Pressing Escape to close the dialog should work even if opened multiple times in a row.
for (let i = 0; i < 3; i++) {
await goToAnything.open(electronApp);
await goToAnything.expectToBeOpen();
await goToAnything.inputLocator.press('Escape');
await goToAnything.expectToBeClosed();
}
});
test('should be possible to show the set tags dialog from goToAnything', async ({ electronApp, mainWindow }) => {
const mainScreen = new MainScreen(mainWindow);
await mainScreen.createNewNote('Test note');
const goToAnything = mainScreen.goToAnything;
await goToAnything.open(electronApp);
await goToAnything.inputLocator.fill(':setTags');
// Should show a matching command
await expect(goToAnything.containerLocator.getByText('Tags (setTags)')).toBeAttached();
await mainWindow.keyboard.press('Enter');
await goToAnything.expectToBeClosed();
// Should show the "set tags" dialog
const setTagsLabel = mainWindow.getByText('Add or remove tags:');
await expect(setTagsLabel).toBeVisible();
});
});

View File

@ -0,0 +1,36 @@
import { ElectronApplication, expect, Locator, Page } from '@playwright/test';
import MainScreen from './MainScreen';
import activateMainMenuItem from '../util/activateMainMenuItem';
export default class GoToAnything {
public readonly containerLocator: Locator;
public readonly inputLocator: Locator;
public constructor(page: Page, private readonly mainScreen: MainScreen) {
this.containerLocator = page.locator('.go-to-anything-dialog[open]');
this.inputLocator = this.containerLocator.getByRole('textbox');
}
public async open(electronApp: ElectronApplication) {
await this.mainScreen.waitFor();
if (!await activateMainMenuItem(electronApp, 'Goto Anything...')) {
throw new Error('Menu item for opening Goto Anything not found');
}
return this.waitFor();
}
public async waitFor() {
await this.containerLocator.waitFor();
}
public async expectToBeClosed() {
await expect(this.containerLocator).not.toBeAttached();
}
public async expectToBeOpen() {
await expect(this.containerLocator).toBeAttached();
}
}

View File

@ -2,6 +2,7 @@ import { Page, Locator, ElectronApplication } from '@playwright/test';
import NoteEditorScreen from './NoteEditorScreen';
import activateMainMenuItem from '../util/activateMainMenuItem';
import Sidebar from './Sidebar';
import GoToAnything from './GoToAnything';
export default class MainScreen {
public readonly newNoteButton: Locator;
@ -9,6 +10,7 @@ export default class MainScreen {
public readonly sidebar: Sidebar;
public readonly dialog: Locator;
public readonly noteEditor: NoteEditorScreen;
public readonly goToAnything: GoToAnything;
public constructor(private page: Page) {
this.newNoteButton = page.locator('.new-note-button');
@ -16,6 +18,7 @@ export default class MainScreen {
this.sidebar = new Sidebar(page, this);
this.dialog = page.locator('.dialog-root');
this.noteEditor = new NoteEditorScreen(page);
this.goToAnything = new GoToAnything(page, this);
}
public async waitFor() {

View File

@ -22,6 +22,7 @@ import Logger from '@joplin/utils/Logger';
import { MarkupLanguage, MarkupToHtml } from '@joplin/renderer';
import Resource from '@joplin/lib/models/Resource';
import { NoteEntity, ResourceEntity } from '@joplin/lib/services/database/types';
import Dialog from '../gui/Dialog';
const logger = Logger.create('GotoAnything');
@ -116,7 +117,7 @@ class GotoAnything {
}
class Dialog extends React.PureComponent<Props, State> {
class DialogComponent extends React.PureComponent<Props, State> {
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
private styles_: any;
@ -153,10 +154,8 @@ class Dialog extends React.PureComponent<Props, State> {
this.inputRef = React.createRef();
this.itemListRef = React.createRef();
this.onKeyDown = this.onKeyDown.bind(this);
this.input_onChange = this.input_onChange.bind(this);
this.input_onKeyDown = this.input_onKeyDown.bind(this);
this.modalLayer_onClick = this.modalLayer_onClick.bind(this);
this.renderItem = this.renderItem.bind(this);
this.listItem_onClick = this.listItem_onClick.bind(this);
this.helpButton_onClick = this.helpButton_onClick.bind(this);
@ -226,8 +225,6 @@ class Dialog extends React.PureComponent<Props, State> {
}
public componentDidMount() {
document.addEventListener('keydown', this.onKeyDown);
this.props.dispatch({
type: 'VISIBLE_DIALOGS_ADD',
name: 'gotoAnything',
@ -236,7 +233,6 @@ class Dialog extends React.PureComponent<Props, State> {
public componentWillUnmount() {
if (this.listUpdateIID_) shim.clearTimeout(this.listUpdateIID_);
document.removeEventListener('keydown', this.onKeyDown);
this.props.dispatch({
type: 'VISIBLE_DIALOGS_REMOVE',
@ -244,27 +240,13 @@ class Dialog extends React.PureComponent<Props, State> {
});
}
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
public onKeyDown(event: any) {
if (event.keyCode === 27) { // ESCAPE
this.props.dispatch({
pluginName: PLUGIN_NAME,
type: 'PLUGINLEGACY_DIALOG_SET',
open: false,
});
}
}
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
private modalLayer_onClick(event: any) {
if (event.currentTarget === event.target) {
this.props.dispatch({
pluginName: PLUGIN_NAME,
type: 'PLUGINLEGACY_DIALOG_SET',
open: false,
});
}
}
private modalLayer_onDismiss = () => {
this.props.dispatch({
pluginName: PLUGIN_NAME,
type: 'PLUGINLEGACY_DIALOG_SET',
open: false,
});
};
private helpButton_onClick() {
this.setState({ showHelp: !this.state.showHelp });
@ -646,21 +628,18 @@ class Dialog extends React.PureComponent<Props, State> {
}
public render() {
const theme = themeStyle(this.props.themeId);
const style = this.style();
const helpComp = !this.state.showHelp ? null : <div className="help-text" style={style.help}>{_('Type a note title or part of its content to jump to it. Or type # followed by a tag name, or @ followed by a notebook name. Or type : to search for commands.')}</div>;
return (
<div className="modal-layer" onClick={this.modalLayer_onClick} style={theme.dialogModalLayer}>
<div className="modal-dialog" style={style.dialogBox}>
{helpComp}
<div style={style.inputHelpWrapper}>
<input autoFocus type="text" style={style.input} ref={this.inputRef} value={this.state.query} onChange={this.input_onChange} onKeyDown={this.input_onKeyDown} />
<HelpButton onClick={this.helpButton_onClick} />
</div>
{this.renderList()}
<Dialog className="go-to-anything-dialog" onCancel={this.modalLayer_onDismiss} contentStyle={style.dialogBox}>
{helpComp}
<div style={style.inputHelpWrapper}>
<input autoFocus type="text" style={style.input} ref={this.inputRef} value={this.state.query} onChange={this.input_onChange} onKeyDown={this.input_onKeyDown} />
<HelpButton onClick={this.helpButton_onClick} />
</div>
</div>
{this.renderList()}
</Dialog>
);
}
@ -675,7 +654,7 @@ const mapStateToProps = (state: AppState) => {
};
};
GotoAnything.Dialog = connect(mapStateToProps)(Dialog);
GotoAnything.Dialog = connect(mapStateToProps)(DialogComponent);
GotoAnything.manifest = {

View File

@ -6,40 +6,14 @@ import WebviewController from '@joplin/lib/services/plugins/WebviewController';
import UserWebview, { Props as UserWebviewProps } from './UserWebview';
import UserWebviewDialogButtonBar from './UserWebviewDialogButtonBar';
import { focus } from '@joplin/lib/utils/focusHandler';
import Dialog from '../../gui/Dialog';
const styled = require('styled-components').default;
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
type StyleProps = any;
interface Props extends UserWebviewProps {
buttons: ButtonSpec[];
fitToContent: boolean;
}
const StyledRoot = styled.div`
display: flex;
flex: 1;
padding: 0;
margin: 0;
width: 100%;
height: 100%;
background-color: rgba(0,0,0,0.5);
box-sizing: border-box;
justify-content: center;
align-items: center;
`;
const Dialog = styled.div`
display: flex;
flex-direction: column;
background-color: ${(props: StyleProps) => props.theme.backgroundColor};
padding: ${(props: StyleProps) => `${props.theme.mainPadding}px`};
border-radius: 4px;
box-shadow: 0 6px 10px #00000077;
width: ${(props: StyleProps) => props.fitToContent ? 'auto' : '90vw'};
height: ${(props: StyleProps) => props.fitToContent ? 'auto' : '80vh'};
`;
const UserWebViewWrapper = styled.div`
display: flex;
flex: 1;
@ -109,25 +83,23 @@ export default function UserWebviewDialog(props: Props) {
}, []);
return (
<StyledRoot>
<Dialog fitToContent={props.fitToContent}>
<UserWebViewWrapper>
<UserWebview
ref={webviewRef}
html={props.html}
scripts={props.scripts}
pluginId={props.pluginId}
viewId={props.viewId}
themeId={props.themeId}
borderBottom={false}
fitToContent={props.fitToContent}
onSubmit={onSubmit}
onDismiss={onDismiss}
onReady={onReady}
/>
</UserWebViewWrapper>
<UserWebviewDialogButtonBar buttons={buttons}/>
</Dialog>
</StyledRoot>
<Dialog className={`user-webview-dialog ${props.fitToContent ? '-fit' : ''}`}>
<UserWebViewWrapper>
<UserWebview
ref={webviewRef}
html={props.html}
scripts={props.scripts}
pluginId={props.pluginId}
viewId={props.viewId}
themeId={props.themeId}
borderBottom={false}
fitToContent={props.fitToContent}
onSubmit={onSubmit}
onDismiss={onDismiss}
onReady={onReady}
/>
</UserWebViewWrapper>
<UserWebviewDialogButtonBar buttons={buttons}/>
</Dialog>
);
}