From 578a4e0cf520c781af932f37be6fada706a3fe3c Mon Sep 17 00:00:00 2001
From: tobi <31960611+tsmethurst@users.noreply.github.com>
Date: Tue, 7 May 2024 19:48:12 +0200
Subject: [PATCH] [bugfix] Reset emoji fields on upload error (#2905)

---
 web/source/settings/components/error.tsx      | 91 +++++++++++------
 .../settings/components/form/inputs.tsx       | 28 +++---
 .../components/form/mutation-button.tsx       |  4 +-
 web/source/settings/lib/form/file.tsx         | 37 +++----
 web/source/settings/style.css                 | 69 ++++++++-----
 .../views/admin/emoji/local/new-emoji.tsx     | 99 ++++++++++++-------
 6 files changed, 206 insertions(+), 122 deletions(-)

diff --git a/web/source/settings/components/error.tsx b/web/source/settings/components/error.tsx
index 15c3bccd4..a2b4772dc 100644
--- a/web/source/settings/components/error.tsx
+++ b/web/source/settings/components/error.tsx
@@ -17,7 +17,9 @@
 	along with this program.  If not, see <http://www.gnu.org/licenses/>.
 */
 
-import React from "react";
+import { SerializedError } from "@reduxjs/toolkit";
+import { FetchBaseQueryError } from "@reduxjs/toolkit/query";
+import React, { ReactNode } from "react";
 
 function ErrorFallback({ error, resetErrorBoundary }) {
 	return (
@@ -44,39 +46,70 @@ function ErrorFallback({ error, resetErrorBoundary }) {
 	);
 }
 
-function Error({ error }) {
-	/* eslint-disable-next-line no-console */
-	console.error("Rendering error:", error);
-	let message;
+interface GtsError {
+	/**
+	 * Error message returned from the API.
+	 */
+	error: string;
 
-	if (error.data != undefined) { // RTK Query error with data
-		if (error.status) {
-			message = (<>
-				<b>{error.status}:</b> {error.data.error}
-				{error.data.error_description &&
-					<p>
-						{error.data.error_description}
-					</p>
-				}
-			</>);
-		} else {
-			message = error.data.error;
-		}
-	} else if (error.name != undefined || error.type != undefined) { // JS error
-		message = (<>
-			<b>{error.type && error.name}:</b> {error.message}
-		</>);
-	} else if (error.status && typeof error.error == "string") {
-		message = (<>
-			<b>{error.status}:</b> {error.error}
-		</>);
+	/**
+	 * For OAuth errors: description of the error.
+	 */
+	error_description?: string;
+}
+
+interface ErrorProps {
+	error: FetchBaseQueryError | SerializedError | Error | undefined;
+	
+	/**
+	 * Optional function to clear the error.
+	 * If provided, rendered error will have
+	 * a "dismiss" button.
+	 */
+	reset?: () => void;
+}
+
+function Error({ error, reset }: ErrorProps) {
+	if (error === undefined) {
+		return null;
+	}
+	
+	/* eslint-disable-next-line no-console */
+	console.error("caught error: ", error);
+	
+	let message: ReactNode;
+	if ("status" in error) {
+		// RTK Query error with data.
+		const gtsError = error.data as GtsError;
+		const errMsg = gtsError.error_description ?? gtsError.error;
+		message = <>Code {error.status} {errMsg}</>;
 	} else {
-		message = error.message ?? error;
+		// SerializedError or Error.
+		const errMsg = error.message ?? JSON.stringify(error);
+		message = (
+			<>{error.name && `${error.name}: `}{errMsg}</>
+		);
+	}
+
+	let className = "error";
+	if (reset) {
+		className += " with-dismiss";
 	}
 
 	return (
-		<div className="error">
-			{message}
+		<div className={className}>
+			<span>{message}</span>
+			{ reset && 
+				<span 
+					className="dismiss"
+					onClick={reset}
+					role="button"
+					tabIndex={0}
+				>
+					<span>Dismiss</span>
+					<i className="fa fa-fw fa-close" aria-hidden="true" />
+				</span>
+			}
 		</div>
 	);
 }
diff --git a/web/source/settings/components/form/inputs.tsx b/web/source/settings/components/form/inputs.tsx
index f82937fc1..c68095d95 100644
--- a/web/source/settings/components/form/inputs.tsx
+++ b/web/source/settings/components/form/inputs.tsx
@@ -29,6 +29,7 @@ import type {
 	RadioFormInputHook,
 	TextFormInputHook,
 } from "../../lib/form/types";
+import { nanoid } from "nanoid";
 
 export interface TextInputProps extends React.DetailedHTMLProps<
 	React.InputHTMLAttributes<HTMLInputElement>,
@@ -92,22 +93,25 @@ export interface FileInputProps extends React.DetailedHTMLProps<
 
 export function FileInput({ label, field, ...props }: FileInputProps) {
 	const { onChange, ref, infoComponent } = field;
+	const id = nanoid();
 
 	return (
 		<div className="form-field file">
-			<label>
-				<div className="label">{label}</div>
-				<div className="file-input button">Browse</div>
-				{infoComponent}
-				{/* <a onClick={removeFile("header")}>remove</a> */}
-				<input
-					type="file"
-					className="hidden"
-					onChange={onChange}
-					ref={ref ? ref as RefObject<HTMLInputElement> : undefined}
-					{...props}
-				/>
+			<label className="label-label" htmlFor={id}>
+				{label}
 			</label>
+			<label className="label-button" htmlFor={id}>
+				<div className="file-input button">Browse</div>
+			</label>
+			<input
+				id={id}
+				type="file"
+				className="hidden"
+				onChange={onChange}
+				ref={ref ? ref as RefObject<HTMLInputElement> : undefined}
+				{...props}
+			/>
+			{infoComponent}
 		</div>
 	);
 }
diff --git a/web/source/settings/components/form/mutation-button.tsx b/web/source/settings/components/form/mutation-button.tsx
index 1e6d8c968..5d831cd24 100644
--- a/web/source/settings/components/form/mutation-button.tsx
+++ b/web/source/settings/components/form/mutation-button.tsx
@@ -51,9 +51,9 @@ export default function MutationButton({
 	}
 
 	return (
-		<div className={wrapperClassName}>
+		<div className={wrapperClassName ? wrapperClassName : "mutation-button"}>
 			{(showError && targetsThisButton && result.error) &&
-				<Error error={result.error} />
+				<Error error={result.error} reset={result.reset} />
 			}
 			<button
 				type="submit"
diff --git a/web/source/settings/lib/form/file.tsx b/web/source/settings/lib/form/file.tsx
index 944d77ae1..cf9407827 100644
--- a/web/source/settings/lib/form/file.tsx
+++ b/web/source/settings/lib/form/file.tsx
@@ -27,6 +27,7 @@ import type {
 	HookOpts,
 	FileFormInputHook,
 } from "./types";
+import { Error as ErrorC } from "../../components/error";
 
 const _default = undefined;
 export default function useFileInput(
@@ -41,6 +42,15 @@ export default function useFileInput(
 	const [imageURL, setImageURL] = useState<string>();
 	const [info, setInfo] = useState<React.JSX.Element>();
 
+	function reset() {
+		if (imageURL) {
+			URL.revokeObjectURL(imageURL);
+		}
+		setImageURL(undefined);
+		setFile(undefined);
+		setInfo(undefined);
+	}
+
 	function onChange(e: React.ChangeEvent<HTMLInputElement>) {
 		const files = e.target.files;
 		if (!files) {
@@ -59,25 +69,18 @@ export default function useFileInput(
 			setImageURL(URL.createObjectURL(file));
 		}
 
-		let size = prettierBytes(file.size);
+		const sizePrettier = prettierBytes(file.size);
 		if (maxSize && file.size > maxSize) {
-			size = <span className="error-text">{size}</span>;
+			const maxSizePrettier = prettierBytes(maxSize);
+			setInfo(
+				<ErrorC
+					error={new Error(`file size ${sizePrettier} is larger than max size ${maxSizePrettier}`)}
+					reset={(reset)}
+				/>
+			);
+		} else {
+			setInfo(<>{file.name} ({sizePrettier})</>);
 		}
-
-		setInfo(
-			<>
-				{file.name} ({size})
-			</>
-		);
-	}
-
-	function reset() {
-		if (imageURL) {
-			URL.revokeObjectURL(imageURL);
-		}
-		setImageURL(undefined);
-		setFile(undefined);
-		setInfo(undefined);
 	}
 
 	const infoComponent = (
diff --git a/web/source/settings/style.css b/web/source/settings/style.css
index b73978e33..b63c47701 100644
--- a/web/source/settings/style.css
+++ b/web/source/settings/style.css
@@ -257,33 +257,37 @@ input, select, textarea {
 		overflow: auto;
 		margin: 0;
 	}
+
+	&.with-dismiss {
+		display: flex;
+		gap: 1rem;
+		justify-content: space-between;
+		align-items: center;
+		align-items: center;
+		flex-wrap: wrap;
+		align-items: center;
+		flex-wrap: wrap;
+
+		.dismiss {
+			display: flex;
+			flex-shrink: 0;
+			align-items: center;
+			align-self: stretch;
+			gap: 0.25rem;
+		}
+	}
+}
+
+.mutation-button {
+	display: flex;
+	flex-direction: column;
+	gap: 1rem;
 }
 
 .hidden {
 	display: none;
 }
 
-.messagebutton, .messagebutton > div {
-	display: flex;
-	align-items: center;
-	flex-wrap: wrap;
-
-	div.padded {
-		margin-left: 1rem;
-	}
-
-	button, .button {
-		white-space: nowrap;
-		margin-right: 1rem;
-	}
-}
-
-.messagebutton > div {
-	button, .button {
-		margin-top: 1rem;
-	}
-}
-
 .notImplemented {
 	border: 2px solid rgb(70, 79, 88);
 	background: repeating-linear-gradient(
@@ -500,12 +504,29 @@ form {
 	font-weight: bold;
 }
 
-.form-field.file label {
+.form-field.file {
 	display: grid;
 	grid-template-columns: auto 1fr;
+	grid-template-rows: auto auto;
+	grid-template-areas:
+		"label-label  label-label"
+		"label-button file-info"
+	;
+	
+	.label-label {
+		grid-area: label-label;
+	}
 
-	.label {
-		grid-column: 1 / span 2;
+	.label-button {
+		grid-area: label-button;
+	}
+
+	.form-info {
+		grid-area: file-info;
+		.error {
+			padding: 0.1rem;
+  			line-height: 1.4rem;
+		}
 	}
 }
 
diff --git a/web/source/settings/views/admin/emoji/local/new-emoji.tsx b/web/source/settings/views/admin/emoji/local/new-emoji.tsx
index 8ff8236a7..20f45f372 100644
--- a/web/source/settings/views/admin/emoji/local/new-emoji.tsx
+++ b/web/source/settings/views/admin/emoji/local/new-emoji.tsx
@@ -17,7 +17,7 @@
 	along with this program.  If not, see <http://www.gnu.org/licenses/>.
 */
 
-import React, { useMemo, useEffect } from "react";
+import React, { useMemo, useEffect, ReactNode } from "react";
 import { useFileInput, useComboBoxInput } from "../../../../lib/form";
 import useShortcode from "./use-shortcode";
 import useFormSubmit from "../../../../lib/form/submit";
@@ -27,52 +27,74 @@ import FakeToot from "../../../../components/fake-toot";
 import MutationButton from "../../../../components/form/mutation-button";
 import { useAddEmojiMutation } from "../../../../lib/query/admin/custom-emoji";
 import { useInstanceV1Query } from "../../../../lib/query/gts-api";
+import prettierBytes from "prettier-bytes";
 
 export default function NewEmojiForm() {
-	const shortcode = useShortcode();
-
 	const { data: instance } = useInstanceV1Query();
 	const emojiMaxSize = useMemo(() => {
 		return instance?.configuration?.emojis?.emoji_size_limit ?? 50 * 1024;
 	}, [instance]);
 
-	const image = useFileInput("image", {
-		withPreview: true,
-		maxSize: emojiMaxSize
-	});
+	const prettierMaxSize = useMemo(() => {
+		return prettierBytes(emojiMaxSize);
+	}, [emojiMaxSize]);
 
-	const category = useComboBoxInput("category");
+	const form = {
+		shortcode: useShortcode(),
+		image: useFileInput("image", {
+			withPreview: true,
+			maxSize: emojiMaxSize
+		}),
+		category: useComboBoxInput("category"),
+	};
 
-	const [submitForm, result] = useFormSubmit({
-		shortcode, image, category
-	}, useAddEmojiMutation());
+	const [submitForm, result] = useFormSubmit(
+		form,
+		useAddEmojiMutation(),
+		{
+			changedOnly: false,
+			// On submission, reset form values
+			// no matter what the result was.
+			onFinish: (_res) => {
+				form.shortcode.reset();
+				form.image.reset();
+				form.category.reset();
+			}
+		},
+	);
 
 	useEffect(() => {
-		if (shortcode.value === undefined || shortcode.value.length == 0) {
-			if (image.value != undefined) {
-				let [name, _ext] = image.value.name.split(".");
-				shortcode.setter(name);
-			}
+		// If shortcode has not been entered yet, but an image file
+		// has been submitted, suggest a shortcode based on filename.
+		if (
+			(form.shortcode.value === undefined || form.shortcode.value.length === 0) &&
+			form.image.value !== undefined
+		) {
+			let [name, _ext] = form.image.value.name.split(".");
+			form.shortcode.setter(name);
 		}
 
-		/* We explicitly don't want to have 'shortcode' as a dependency here
-			 because we only want to change the shortcode to the filename if the field is empty
-			 at the moment the file is selected, not some time after when the field is emptied
-		*/
-		/* eslint-disable-next-line react-hooks/exhaustive-deps */
-	}, [image.value]);
+		// We explicitly don't want to have 'shortcode' as a
+		// dependency here because we only want to change the
+		// shortcode to the filename if the field is empty at
+		// the moment the file is selected, not some time after
+		// when the field is emptied.
+		//
+		// eslint-disable-next-line react-hooks/exhaustive-deps
+	}, [form.image.value]);
 
-	let emojiOrShortcode;
-
-	if (image.previewValue != undefined) {
-		emojiOrShortcode = <img
-			className="emoji"
-			src={image.previewValue}
-			title={`:${shortcode.value}:`}
-			alt={shortcode.value}
-		/>;
-	} else if (shortcode.value !== undefined && shortcode.value.length > 0) {
-		emojiOrShortcode = `:${shortcode.value}:`;
+	let emojiOrShortcode: ReactNode;
+	if (form.image.previewValue !== undefined) {
+		emojiOrShortcode = (
+			<img
+				className="emoji"
+				src={form.image.previewValue}
+				title={`:${form.shortcode.value}:`}
+				alt={form.shortcode.value}
+			/>
+		);
+	} else if (form.shortcode.value !== undefined && form.shortcode.value.length > 0) {
+		emojiOrShortcode = `:${form.shortcode.value}:`;
 	} else {
 		emojiOrShortcode = `:your_emoji_here:`;
 	}
@@ -87,22 +109,23 @@ export default function NewEmojiForm() {
 
 			<form onSubmit={submitForm} className="form-flex">
 				<FileInput
-					field={image}
+					field={form.image}
+					label={`Image file: png, gif, or static webp; max size ${prettierMaxSize}`}
 					accept="image/png,image/gif,image/webp"
 				/>
 
 				<TextInput
-					field={shortcode}
+					field={form.shortcode}
 					label="Shortcode, must be unique among the instance's local emoji"
+					{...{pattern: "^\\w{2,30}$"}}
 				/>
 
 				<CategorySelect
-					field={category}
-					children={[]}
+					field={form.category}
 				/>
 
 				<MutationButton
-					disabled={image.previewValue === undefined}
+					disabled={form.image.previewValue === undefined || form.shortcode.value?.length === 0}
 					label="Upload emoji"
 					result={result}
 				/>