From f07e4e9b5a5562d1d16ba2c2c976205376f2d2c5 Mon Sep 17 00:00:00 2001
From: Henry Heino <46334387+personalizedrefrigerator@users.noreply.github.com>
Date: Sat, 26 Oct 2024 13:06:09 -0700
Subject: [PATCH] Desktop: Accessibility: Add missing labels to the note
 attachments screen and master password dialog (#11231)

---
 .eslintignore                                 |  2 +
 .gitignore                                    |  2 +
 .../gui/MasterPasswordDialog/Dialog.tsx       | 39 ++++++-------
 .../PasswordInput/LabelledPasswordInput.tsx   | 56 +++++++++++++++++++
 .../gui/PasswordInput/PasswordInput.tsx       | 32 ++++++++---
 .../app-desktop/gui/PasswordInput/style.scss  | 22 +-------
 .../styles/labelled-password-input.scss       | 10 ++++
 .../PasswordInput/styles/password-input.scss  | 19 +++++++
 .../styles/password-status-icon.scss          | 13 +++++
 .../app-desktop/gui/PasswordInput/types.ts    |  6 ++
 packages/app-desktop/gui/ResourceScreen.tsx   | 29 ++++++----
 packages/app-desktop/main.scss                | 13 -----
 12 files changed, 170 insertions(+), 73 deletions(-)
 create mode 100644 packages/app-desktop/gui/PasswordInput/LabelledPasswordInput.tsx
 create mode 100644 packages/app-desktop/gui/PasswordInput/styles/labelled-password-input.scss
 create mode 100644 packages/app-desktop/gui/PasswordInput/styles/password-input.scss
 create mode 100644 packages/app-desktop/gui/PasswordInput/styles/password-status-icon.scss
 create mode 100644 packages/app-desktop/gui/PasswordInput/types.ts

diff --git a/.eslintignore b/.eslintignore
index ae4fbcc60..25c41fd03 100644
--- a/.eslintignore
+++ b/.eslintignore
@@ -386,7 +386,9 @@ packages/app-desktop/gui/NoteTextViewer.js
 packages/app-desktop/gui/NoteToolbar/NoteToolbar.js
 packages/app-desktop/gui/NotyfContext.js
 packages/app-desktop/gui/OneDriveLoginScreen.js
+packages/app-desktop/gui/PasswordInput/LabelledPasswordInput.js
 packages/app-desktop/gui/PasswordInput/PasswordInput.js
+packages/app-desktop/gui/PasswordInput/types.js
 packages/app-desktop/gui/PdfViewer.js
 packages/app-desktop/gui/PromptDialog.js
 packages/app-desktop/gui/ResizableLayout/MoveButtons.js
diff --git a/.gitignore b/.gitignore
index 36b0e0f8d..0cd35145c 100644
--- a/.gitignore
+++ b/.gitignore
@@ -363,7 +363,9 @@ packages/app-desktop/gui/NoteTextViewer.js
 packages/app-desktop/gui/NoteToolbar/NoteToolbar.js
 packages/app-desktop/gui/NotyfContext.js
 packages/app-desktop/gui/OneDriveLoginScreen.js
+packages/app-desktop/gui/PasswordInput/LabelledPasswordInput.js
 packages/app-desktop/gui/PasswordInput/PasswordInput.js
+packages/app-desktop/gui/PasswordInput/types.js
 packages/app-desktop/gui/PdfViewer.js
 packages/app-desktop/gui/PromptDialog.js
 packages/app-desktop/gui/ResizableLayout/MoveButtons.js
diff --git a/packages/app-desktop/gui/MasterPasswordDialog/Dialog.tsx b/packages/app-desktop/gui/MasterPasswordDialog/Dialog.tsx
index e23f9bf42..95ac26184 100644
--- a/packages/app-desktop/gui/MasterPasswordDialog/Dialog.tsx
+++ b/packages/app-desktop/gui/MasterPasswordDialog/Dialog.tsx
@@ -10,7 +10,7 @@ import { reg } from '@joplin/lib/registry';
 import EncryptionService from '@joplin/lib/services/e2ee/EncryptionService';
 import KvStore from '@joplin/lib/services/KvStore';
 import ShareService from '@joplin/lib/services/share/ShareService';
-import { PasswordInput } from '../PasswordInput/PasswordInput';
+import LabelledPasswordInput from '../PasswordInput/LabelledPasswordInput';
 
 interface Props {
 	themeId: number;
@@ -136,11 +136,6 @@ export default function(props: Props) {
 		setCurrentPasswordIsValid(isValid);
 	}, [currentPassword]);
 
-	function renderCurrentPasswordIcon() {
-		if (!currentPassword || status === MasterPasswordStatus.NotSet) return null;
-		return currentPasswordIsValid ? <i className="fas fa-check password-valid-icon"></i> : <i className="fas fa-times"></i>;
-	}
-
 	function renderPasswordForm() {
 		const renderCurrentPassword = () => {
 			if (!showCurrentPassword) return null;
@@ -151,14 +146,14 @@ export default function(props: Props) {
 			// having to reset the password (and lose access to any data that's
 			// been encrypted with it).
 
+			const showValidIcon = currentPassword && status !== MasterPasswordStatus.NotSet;
 			return (
-				<div className="form-input-group">
-					<label>{'Current password'}</label>
-					<div className="current-password-wrapper">
-						<PasswordInput value={currentPassword} onChange={onCurrentPasswordChange}/>
-						{renderCurrentPasswordIcon()}
-					</div>
-				</div>
+				<LabelledPasswordInput
+					labelText={_('Current password')}
+					value={currentPassword}
+					onChange={onCurrentPasswordChange}
+					valid={showValidIcon ? currentPasswordIsValid : undefined}
+				/>
 			);
 		};
 
@@ -175,15 +170,17 @@ export default function(props: Props) {
 				<div>
 					<div className="form">
 						{renderCurrentPassword()}
-						<div className="form-input-group">
-							<label>{enterPasswordLabel}</label>
-							<PasswordInput value={password1} onChange={onPasswordChange1}/>
-						</div>
+						<LabelledPasswordInput
+							labelText={enterPasswordLabel}
+							value={password1}
+							onChange={onPasswordChange1}
+						/>
 						{needToRepeatPassword && (
-							<div className="form-input-group">
-								<label>{'Re-enter password'}</label>
-								<PasswordInput value={password2} onChange={onPasswordChange2}/>
-							</div>
+							<LabelledPasswordInput
+								labelText={_('Re-enter password')}
+								value={password2}
+								onChange={onPasswordChange2}
+							/>
 						)}
 					</div>
 					<p className="bold">Please make sure you remember your password. For security reasons, it is not possible to recover it if it is lost.</p>
diff --git a/packages/app-desktop/gui/PasswordInput/LabelledPasswordInput.tsx b/packages/app-desktop/gui/PasswordInput/LabelledPasswordInput.tsx
new file mode 100644
index 000000000..d902f52df
--- /dev/null
+++ b/packages/app-desktop/gui/PasswordInput/LabelledPasswordInput.tsx
@@ -0,0 +1,56 @@
+import * as React from 'react';
+import PasswordInput from './PasswordInput';
+import { useId } from 'react';
+import { ChangeEventHandler } from './types';
+import { _ } from '@joplin/lib/locale';
+
+interface Props {
+	labelText: string;
+	value: string;
+	onChange: ChangeEventHandler;
+	valid?: boolean;
+}
+
+const LabelledPasswordInput: React.FC<Props> = props => {
+	const inputId = useId();
+	const statusIconId = useId();
+
+	const canRenderStatusIcon = (props.valid ?? null) !== null && props.value;
+	const renderStatusIcon = () => {
+		if (!canRenderStatusIcon) return null;
+		let title, classNames;
+		if (props.valid) {
+			title = _('Valid');
+			classNames = 'fas fa-check -valid';
+		} else {
+			title = _('Invalid');
+			classNames = 'fas fa-times -invalid';
+		}
+		return <i
+			className={`password-status-icon ${classNames}`}
+			id={statusIconId}
+
+			role='img'
+			aria-label={title}
+			title={title}
+
+			aria-live='polite'
+		></i>;
+	};
+
+	return <div className='labelled-password-input form-input-group'>
+		<label htmlFor={inputId}>{props.labelText}</label>
+		<div className='password'>
+			<PasswordInput
+				inputId={inputId}
+				aria-invalid={canRenderStatusIcon ? !props.valid : undefined}
+				aria-errormessage={canRenderStatusIcon ? statusIconId : undefined}
+				value={props.value}
+				onChange={props.onChange}
+			/>
+			{renderStatusIcon()}
+		</div>
+	</div>;
+};
+
+export default LabelledPasswordInput;
diff --git a/packages/app-desktop/gui/PasswordInput/PasswordInput.tsx b/packages/app-desktop/gui/PasswordInput/PasswordInput.tsx
index d25fa4179..fd6dfaeab 100644
--- a/packages/app-desktop/gui/PasswordInput/PasswordInput.tsx
+++ b/packages/app-desktop/gui/PasswordInput/PasswordInput.tsx
@@ -1,22 +1,24 @@
+import * as React from 'react';
 import { useState, useCallback } from 'react';
 import StyledInput from '../style/StyledInput';
-
-export interface ChangeEvent {
-	value: string;
-}
-
-type ChangeEventHandler = (event: ChangeEvent)=> void;
+import { _ } from '@joplin/lib/locale';
+import { ChangeEventHandler } from './types';
 
 interface Props {
 	value: string;
+	inputId: string;
 	onChange: ChangeEventHandler;
+
+	'aria-invalid'?: boolean;
+	'aria-errormessage'?: string;
 }
 
-export const PasswordInput = (props: Props) => {
+const PasswordInput = (props: Props) => {
 	const [showPassword, setShowPassword] = useState(false);
 
 	const inputType = showPassword ? 'text' : 'password';
 	const icon = showPassword ? 'far fa-eye-slash' : 'far fa-eye';
+	const title = showPassword ? _('Hide password') : _('Show password');
 
 	const onShowPassword = useCallback(() => {
 		setShowPassword(current => !current);
@@ -24,8 +26,20 @@ export const PasswordInput = (props: Props) => {
 
 	return (
 		<div className="password-input">
-			<StyledInput className="field" type={inputType} value={props.value} onChange={props.onChange}/>
-			<button onClick={onShowPassword} className="showpasswordbutton"><i className={icon}></i></button>
+			<StyledInput
+				id={props.inputId}
+				aria-errormessage={props['aria-errormessage']}
+				aria-invalid={props['aria-invalid']}
+				className="field"
+				type={inputType}
+				value={props.value}
+				onChange={props.onChange}
+			/>
+			<button onClick={onShowPassword} className="showpasswordbutton">
+				<i className={icon} role='img' aria-label={title} title={title}></i>
+			</button>
 		</div>
 	);
 };
+
+export default PasswordInput;
diff --git a/packages/app-desktop/gui/PasswordInput/style.scss b/packages/app-desktop/gui/PasswordInput/style.scss
index eba398237..95349b3a4 100644
--- a/packages/app-desktop/gui/PasswordInput/style.scss
+++ b/packages/app-desktop/gui/PasswordInput/style.scss
@@ -1,19 +1,3 @@
-.password-input {
-	display: flex;
-	position: relative;
-	flex: 1;
-
-	> .field {
-		display: flex;
-		flex: 1;
-		width: 100%;
-	}
-
-	> .showpasswordbutton {
-		position: absolute;
-		right: 5px;
-		top: 4px;
-		border: none;
-		background: none;
-	}
-}
+@use "styles/password-input.scss";
+@use "styles/labelled-password-input.scss";
+@use "styles/password-status-icon.scss";
\ No newline at end of file
diff --git a/packages/app-desktop/gui/PasswordInput/styles/labelled-password-input.scss b/packages/app-desktop/gui/PasswordInput/styles/labelled-password-input.scss
new file mode 100644
index 000000000..66cd3c55f
--- /dev/null
+++ b/packages/app-desktop/gui/PasswordInput/styles/labelled-password-input.scss
@@ -0,0 +1,10 @@
+.labelled-password-input {
+	display: flex;
+	flex-direction: column;
+
+	> .password {
+		display: flex;
+		flex-direction: row;
+		align-items: center;
+	}
+}
\ No newline at end of file
diff --git a/packages/app-desktop/gui/PasswordInput/styles/password-input.scss b/packages/app-desktop/gui/PasswordInput/styles/password-input.scss
new file mode 100644
index 000000000..eba398237
--- /dev/null
+++ b/packages/app-desktop/gui/PasswordInput/styles/password-input.scss
@@ -0,0 +1,19 @@
+.password-input {
+	display: flex;
+	position: relative;
+	flex: 1;
+
+	> .field {
+		display: flex;
+		flex: 1;
+		width: 100%;
+	}
+
+	> .showpasswordbutton {
+		position: absolute;
+		right: 5px;
+		top: 4px;
+		border: none;
+		background: none;
+	}
+}
diff --git a/packages/app-desktop/gui/PasswordInput/styles/password-status-icon.scss b/packages/app-desktop/gui/PasswordInput/styles/password-status-icon.scss
new file mode 100644
index 000000000..ced186ce3
--- /dev/null
+++ b/packages/app-desktop/gui/PasswordInput/styles/password-status-icon.scss
@@ -0,0 +1,13 @@
+
+.password-status-icon {
+	margin-left: 10px;
+
+	&.-valid {
+		color: var(--joplin-color-correct);
+	}
+
+	&.-invalid {
+		color: var(--joplin-color-error);
+		margin-left: 5px;
+	}
+}
\ No newline at end of file
diff --git a/packages/app-desktop/gui/PasswordInput/types.ts b/packages/app-desktop/gui/PasswordInput/types.ts
new file mode 100644
index 000000000..bb7a4da58
--- /dev/null
+++ b/packages/app-desktop/gui/PasswordInput/types.ts
@@ -0,0 +1,6 @@
+
+export interface ChangeEvent {
+	value: string;
+}
+
+export type ChangeEventHandler = (event: ChangeEvent)=> void;
diff --git a/packages/app-desktop/gui/ResourceScreen.tsx b/packages/app-desktop/gui/ResourceScreen.tsx
index 871078a63..ca2af7a9e 100644
--- a/packages/app-desktop/gui/ResourceScreen.tsx
+++ b/packages/app-desktop/gui/ResourceScreen.tsx
@@ -60,15 +60,6 @@ interface ActiveSorting {
 const ResourceTableComp = (props: ResourceTable) => {
 	const theme = themeStyle(props.themeId);
 
-	const sortOrderEngagedMarker = (s: SortingOrder) => {
-		return (
-			<a href="#"
-				style={{ color: theme.urlColor }}
-				onClick={() => props.onToggleSorting(s)}>{
-					(props.sorting.order === s && props.sorting.type === 'desc') ? '▾' : '▴'}</a>
-		);
-	};
-
 	const titleCellStyle = {
 		...theme.textStyle,
 		textOverflow: 'ellipsis',
@@ -96,12 +87,28 @@ const ResourceTableComp = (props: ResourceTable) => {
 		(resource: InnerResource) => !props.filter || resource.title?.includes(props.filter) || resource.id.includes(props.filter),
 	);
 
+	const renderSortableHeader = (title: string, order: SortingOrder) => {
+		const sortedDescending = props.sorting.order === order && props.sorting.type === 'desc';
+		const sortButtonLabel = sortedDescending ? _('Sort "%s" in ascending order', title) : _('Sort "%s" in descending order', title);
+		const reverseSortButton = (
+			<a
+				href="#"
+				style={{ color: theme.urlColor }}
+				onClick={() => props.onToggleSorting(order)}
+				aria-label={sortButtonLabel}
+				title={sortButtonLabel}
+				role='button'
+			>{sortedDescending ? '▾' : '▴'}</a>
+		);
+		return <th key={`header-${title}`} style={headerStyle}>{title} {reverseSortButton}</th>;
+	};
+
 	return (
 		<table style={{ width: '100%' }}>
 			<thead>
 				<tr>
-					<th style={headerStyle}>{_('Title')} {sortOrderEngagedMarker('name')}</th>
-					<th style={headerStyle}>{_('Size')} {sortOrderEngagedMarker('size')}</th>
+					{renderSortableHeader(_('Title'), 'name')}
+					{renderSortableHeader(_('Size'), 'size')}
 					<th style={headerStyle}>{_('ID')}</th>
 					<th style={headerStyle}>{_('Action')}</th>
 				</tr>
diff --git a/packages/app-desktop/main.scss b/packages/app-desktop/main.scss
index ed6e1d975..c862447e5 100644
--- a/packages/app-desktop/main.scss
+++ b/packages/app-desktop/main.scss
@@ -289,22 +289,9 @@ Component-specific classes
 	display: flex;
 	flex-direction: row;
 	align-items: center;
-
-	> .password-valid-icon {
-		margin-left: 10px;
-	}
 }
 
 // .master-password-dialog .current-password-wrapper input {
 // 	flex: 1;
 // 	margin-right: 10px;
 // }
-
-.master-password-dialog .fa-check {
-	color: var(--joplin-color-correct);
-}
-
-.master-password-dialog .fa-times {
-	color: var(--joplin-color-error);
-	margin-left: 5px;
-}