Desktop: Accessibility: Improve focus handling for plugin and prompt …

…dialogs (#10801)
laurent22 · Jul 31, 2024 · 596bcd8 · 596bcd8
1 parent ecc4f3e
commit 596bcd8
Show file tree

Hide file tree

Showing 21 changed files with 230 additions and 165 deletions.
diff --git a/.eslintignore b/.eslintignore
@@ -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

diff --git a/.gitignore b/.gitignore
@@ -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

diff --git a/packages/app-desktop/gui/Dialog.tsx b/packages/app-desktop/gui/Dialog.tsx
@@ -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,28 +19,36 @@ 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
 			event.preventDefault();
 		}
 	}, []);
 
+	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>
 	);

diff --git a/packages/app-desktop/gui/EditFolderDialog/Dialog.tsx b/packages/app-desktop/gui/EditFolderDialog/Dialog.tsx
@@ -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>
 	);
 }
diff --git a/packages/app-desktop/gui/MasterPasswordDialog/Dialog.tsx b/packages/app-desktop/gui/MasterPasswordDialog/Dialog.tsx
@@ -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>
 	);
 }
diff --git a/packages/app-desktop/gui/NoteContentPropertiesDialog.tsx b/packages/app-desktop/gui/NoteContentPropertiesDialog.tsx
@@ -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>
 	);
 }
diff --git a/packages/app-desktop/gui/NotePropertiesDialog.tsx b/packages/app-desktop/gui/NotePropertiesDialog.tsx
@@ -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>
 		);
 	}
 }

diff --git a/packages/app-desktop/gui/PromptDialog.tsx b/packages/app-desktop/gui/PromptDialog.tsx
@@ -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>
 		);
 	}
 }
diff --git a/packages/app-desktop/gui/Root.tsx b/packages/app-desktop/gui/Root.tsx
@@ -167,7 +167,7 @@ class RootComponent extends React.Component<Props, any> {
 			);
 		};
 
-		return <Dialog renderContent={renderContent}/>;
+		return <Dialog>{renderContent()}</Dialog>;
 	}
 
 	private modalDialogProps(): ModalDialogProps {

diff --git a/packages/app-desktop/gui/ShareFolderDialog/ShareFolderDialog.tsx b/packages/app-desktop/gui/ShareFolderDialog/ShareFolderDialog.tsx
@@ -407,7 +407,7 @@ function ShareFolderDialog(props: Props) {
 	}
 
 	return (
-		<Dialog renderContent={renderContent}/>
+		<Dialog>{renderContent()}</Dialog>
 	);
 }
 

diff --git a/packages/app-desktop/gui/ShareNoteDialog.tsx b/packages/app-desktop/gui/ShareNoteDialog.tsx
@@ -226,7 +226,7 @@ export function ShareNoteDialog(props: Props) {
 	};
 
 	return (
-		<Dialog renderContent={renderContent}/>
+		<Dialog>{renderContent()}</Dialog>
 	);
 }
 

diff --git a/packages/app-desktop/gui/SyncWizard/Dialog.tsx b/packages/app-desktop/gui/SyncWizard/Dialog.tsx
@@ -277,6 +277,6 @@ export default function(props: Props) {
 	}
 
 	return (
-		<Dialog onClose={closeDialog} renderContent={renderDialogWrapper}/>
+		<Dialog onCancel={closeDialog}>{renderDialogWrapper()}</Dialog>
 	);
 }
diff --git a/packages/app-desktop/gui/styles/dialog-modal-layer.scss b/packages/app-desktop/gui/styles/dialog-modal-layer.scss
@@ -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);
 	}
 }
diff --git a/packages/app-desktop/gui/styles/index.scss b/packages/app-desktop/gui/styles/index.scss
@@ -1,2 +1,4 @@
 
-@use './dialog-modal-layer.scss';
+@use './dialog-modal-layer.scss';
+@use './user-webview-dialog.scss';
+@use './prompt-dialog.scss';
diff --git a/packages/app-desktop/gui/styles/prompt-dialog.scss b/packages/app-desktop/gui/styles/prompt-dialog.scss
@@ -0,0 +1,7 @@
+
+.prompt-dialog {
+	> .content {
+		display: inline-block;
+		max-width: 450px;
+	}
+}
diff --git a/packages/app-desktop/gui/styles/user-webview-dialog.scss b/packages/app-desktop/gui/styles/user-webview-dialog.scss
@@ -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);
+	}
+}