From 30c9054b2318c2d39f09c65d67774a783e9f11ca Mon Sep 17 00:00:00 2001
From: Michael Kohler <me@michaelkohler.info>
Date: Sat, 12 Feb 2022 02:20:53 +0100
Subject: [PATCH] fix: better differentiation for loading and error states
 (fixes #594)

---
 web/css/root.css                              |  62 ++++++++--
 web/css/spinner-button.css                    |  27 +----
 web/src/components/add/submit-form.test.tsx   |   7 +-
 web/src/components/add/submit-form.tsx        | 112 +++++++++++-------
 web/src/components/error.tsx                  |  25 +++-
 web/src/components/pages/add.tsx              |   8 +-
 .../components/pages/my-sentences-list.tsx    |  16 +--
 .../pages/rejected-sentences-list.tsx         |  16 +--
 web/src/components/pages/review.tsx           |  43 +++----
 web/src/components/pages/stats.tsx            |  27 +++--
 .../profile/add-language-section.tsx          |   3 +-
 .../profile/personal-language-info.tsx        |   2 +-
 web/src/components/profile/settings.tsx       |   7 +-
 web/src/components/success.tsx                |  33 ++++++
 14 files changed, 241 insertions(+), 147 deletions(-)
 create mode 100644 web/src/components/success.tsx

diff --git a/web/css/root.css b/web/css/root.css
index a1128ba1..097dca0d 100644
--- a/web/css/root.css
+++ b/web/css/root.css
@@ -10,11 +10,13 @@
   --deactive-color: #d7d7db;
   --grey-color: #d7d7db;
   --dark-grey-color: #5e5c5b;
-  --error-color: #ff4f5e;
+  --error-font-color: #ff4f5e;
   --error-border-color: #ff7070;
   --error-background-color: #ffbfc5;
   --warning-border-color: #ffff00;
   --warning-background-color: #ffffb9;
+  --success-border-color: #46ff55;
+  --success-background-color: #71ffc4;
   --review-selected-color: #000000;
   --review-unselected-color: #ffffff;
   --base-font-size: 19px;
@@ -192,13 +194,6 @@ form button:not(.standalone) {
   font-weight: 600;
 }
 
-.form-error {
-  line-height: 1rem;
-  color: var(--error-color);
-  text-align: center;
-  font-weight: 600;
-}
-
 .loading-text {
   display: inline-block;
   font-size: 80%;
@@ -213,13 +208,23 @@ form button:not(.standalone) {
 }
 
 .error-message {
-  color: red;
+  color: var(--error-font-color);
+  font-size: 0.9rem;
+  margin-top: 0.5rem;
+  margin-bottom: 0.5rem;
 }
 
 .small {
   font-size: 0.7rem;
 }
 
+.success-box {
+  margin: 1.5rem 0;
+  padding: 1rem;
+  border: 2px solid var(--success-border-color);
+  background-color: var(--success-background-color);
+}
+
 .error-box {
   margin: 1.5rem 0;
   padding: 1rem;
@@ -239,3 +244,42 @@ form button:not(.standalone) {
   flex-direction: row;
   align-items: center;
 }
+
+.loading-container {
+  position: relative;
+}
+
+.spinning:before {
+  content: '';
+  width: 0px;
+  height: 0px;
+  border-radius: 50%;
+  right: 6px;
+  top: 50%;
+  position: absolute;
+  border-right: 3px solid #2795ae;
+  animation: rotate360 0.5s infinite linear, exist 0.1s forwards ease;
+}
+
+.loading-container .spinning:before {
+  right: unset;
+  left: 0px;
+}
+
+.loading-container p {
+  margin-left: 30px;
+}
+
+@keyframes rotate360 {
+  100% {
+    transform: rotate(360deg);
+  }
+}
+
+@keyframes exist {
+  100% {
+    width: 15px;
+    height: 15px;
+    margin: -8px 5px 0 0;
+  }
+}
diff --git a/web/css/spinner-button.css b/web/css/spinner-button.css
index 40793e74..0f18da8c 100644
--- a/web/css/spinner-button.css
+++ b/web/css/spinner-button.css
@@ -1,34 +1,9 @@
 .spinnerButton {
   transition: padding-right 0.3s ease;
+  position: relative;
 }
 
 .spinnerButton.spinning {
   padding-right: 40px;
   cursor: not-allowed;
 }
-
-.spinnerButton.spinning:before {
-  content: '';
-  width: 0px;
-  height: 0px;
-  border-radius: 50%;
-  right: 6px;
-  top: 50%;
-  position: absolute;
-  border-right: 3px solid #2795ae;
-  animation: rotate360 0.5s infinite linear, exist 0.1s forwards ease;
-}
-
-@keyframes rotate360 {
-  100% {
-    transform: rotate(360deg);
-  }
-}
-
-@keyframes exist {
-  100% {
-    width: 15px;
-    height: 15px;
-    margin: -8px 5px 0 0;
-  }
-}
diff --git a/web/src/components/add/submit-form.test.tsx b/web/src/components/add/submit-form.test.tsx
index 81135dd5..3f2397f5 100644
--- a/web/src/components/add/submit-form.test.tsx
+++ b/web/src/components/add/submit-form.test.tsx
@@ -35,12 +35,11 @@ test('should render submit button', async () => {
   expect(screen.getByText('Submit')).toBeTruthy();
 });
 
-test('should render message', async () => {
-  const message = 'Hi';
+test('should render success', async () => {
   await renderWithLocalization(
-    <SubmitForm languages={languages} message={message} onSubmit={onSubmit} />
+    <SubmitForm languages={languages} duplicates={0} onSubmit={onSubmit} />
   );
-  expect(screen.getByText(message)).toBeTruthy();
+  expect(screen.queryByText(/Submitted sentences./)).toBeTruthy();
 });
 
 test('should render error', async () => {
diff --git a/web/src/components/add/submit-form.tsx b/web/src/components/add/submit-form.tsx
index 246517ec..61895b8d 100644
--- a/web/src/components/add/submit-form.tsx
+++ b/web/src/components/add/submit-form.tsx
@@ -1,6 +1,6 @@
-import React, { useState } from 'react';
+import React, { useEffect, useState } from 'react';
 import { Link } from 'react-router-dom';
-import { Localized, useLocalization } from '@fluent/react';
+import { Localized } from '@fluent/react';
 
 import { useLocaleUrl } from '../../urls';
 import type { Language, SubmissionFailures } from '../../types';
@@ -8,20 +8,12 @@ import Error from '../error';
 import LanguageSelector from '../language-selector';
 import Sentence from '../sentence';
 import SubmitButton from '../submit-button';
+import Success from '../success';
 import { Prompt } from '../prompt';
 
 const SPLIT_ON = '\n';
 const TRANSLATION_KEY_PREFIX = 'TRANSLATION_KEY:';
 
-function parseSentences(sentenceText: string): string[] {
-  const sentences = sentenceText
-    .split(SPLIT_ON)
-    .map((s) => s.trim())
-    .filter(Boolean);
-  const dedupedSentences = Array.from(new Set(sentences));
-  return dedupedSentences;
-}
-
 type SubmissionData = {
   sentences: string[];
   language: string;
@@ -31,7 +23,7 @@ type SubmissionData = {
 type Props = {
   languages: Language[];
   onSubmit: (data: SubmissionData) => void;
-  message?: string;
+  duplicates?: number;
   error?: string;
   sentenceSubmissionFailures?: SubmissionFailures;
   languageFetchFailure?: boolean;
@@ -43,24 +35,36 @@ type FormFields = {
   confirmed: boolean;
 };
 
+type FormErrorDescription = {
+  language: boolean;
+  sentences: boolean;
+  source: boolean;
+  confirmed: boolean;
+};
+
 export default function SubmitForm({
   languages,
   onSubmit,
-  message,
+  duplicates,
   error,
   sentenceSubmissionFailures,
   languageFetchFailure = false,
 }: Props) {
-  const firstLanguage = languages.length === 1 && languages[0];
-  const [formError, setError] = useState('');
+  const [formErrors, setFormErrors] = useState<FormErrorDescription | Record<string, never>>({});
   const [formFields, setFormFields] = useState<FormFields>({
     sentenceText: '',
     source: '',
     confirmed: false,
   });
-  const [language, setLanguage] = useState(firstLanguage ? firstLanguage.id : '');
+  const [language, setLanguage] = useState('');
   const localizedHowToUrl = useLocaleUrl('/how-to');
 
+  useEffect(() => {
+    if (languages.length === 1 && languages[0]) {
+      setLanguage(languages[0].id);
+    }
+  }, [languages]);
+
   const handleInputChange = (
     event: React.FormEvent<HTMLInputElement> | React.ChangeEvent<HTMLTextAreaElement>
   ) => {
@@ -77,30 +81,18 @@ export default function SubmitForm({
     setLanguage(language);
   };
 
-  const { l10n } = useLocalization();
-
   const validateForm = () => {
-    if (!language) {
-      setError(l10n.getString('sc-submit-err-select-lang'));
-      return false;
-    }
-
-    if (!formFields.sentenceText) {
-      setError(l10n.getString('sc-submit-err-add-sentences'));
-      return false;
-    }
-
-    if (!formFields.source) {
-      setError(l10n.getString('sc-submit-err-add-source'));
-      return false;
-    }
+    const errors = {
+      language: !language,
+      sentences: !formFields.sentenceText,
+      source: !formFields.source,
+      confirmed: !formFields.confirmed,
+    };
 
-    if (!formFields.confirmed) {
-      setError(l10n.getString('sc-submit-err-confirm-pd'));
-      return false;
-    }
+    setFormErrors(errors);
 
-    return true;
+    const hasErrors = Object.values(errors).some((errorState) => errorState);
+    return !hasErrors;
   };
 
   const onSentencesSubmit = (event: React.FormEvent<HTMLFormElement>) => {
@@ -129,16 +121,23 @@ export default function SubmitForm({
         </Localized>
 
         {languageFetchFailure && <Error translationKey="sc-languages-fetch-error" />}
-
-        {message && <section className="form-message">{message}</section>}
-        {formError && <section className="form-error">{formError}</section>}
-        {error && <section className="form-error">{error}</section>}
+        {typeof duplicates !== 'undefined' && (
+          <Success translationKey="sc-add-result" vars={{ duplicates }} />
+        )}
+        {error && <Error>{error}</Error>}
 
         <section>
           <Localized id="sc-submit-select-language" attrs={{ labelText: true }}>
             <LanguageSelector languages={languages} labelText="" onChange={onLanguageSelect} />
           </Localized>
+
+          {formErrors.language && (
+            <Localized id="sc-submit-err-select-lang">
+              <p className="error-message"></p>
+            </Localized>
+          )}
         </section>
+
         <section>
           <Localized
             id="sc-submit-add-sentences"
@@ -154,6 +153,13 @@ export default function SubmitForm({
           >
             <label htmlFor="sentences-input"></label>
           </Localized>
+
+          {formErrors.sentences && (
+            <Localized id="sc-submit-err-add-sentences">
+              <p className="error-message"></p>
+            </Localized>
+          )}
+
           <Localized id="sc-submit-ph-one-per-line" attrs={{ placeholder: true }}>
             <textarea
               id="sentences-input"
@@ -164,6 +170,7 @@ export default function SubmitForm({
             />
           </Localized>
         </section>
+
         <section>
           <Localized
             id="sc-submit-from-where"
@@ -179,6 +186,13 @@ export default function SubmitForm({
           >
             <label htmlFor="source-input"></label>
           </Localized>
+
+          {formErrors.source && (
+            <Localized id="sc-submit-err-add-source">
+              <p className="error-message"></p>
+            </Localized>
+          )}
+
           <Localized id="sc-submit-ph-read-how-to" attrs={{ placeholder: true }}>
             <input
               id="source-input"
@@ -189,7 +203,14 @@ export default function SubmitForm({
             />
           </Localized>
         </section>
+
         <section>
+          {formErrors.confirmed && (
+            <Localized id="sc-submit-err-confirm-pd">
+              <p className="error-message"></p>
+            </Localized>
+          )}
+
           <input id="agree" type="checkbox" name="confirmed" onChange={handleInputChange} />
           <Localized
             id="sc-submit-confirm"
@@ -247,3 +268,12 @@ export default function SubmitForm({
     </React.Fragment>
   );
 }
+
+function parseSentences(sentenceText: string): string[] {
+  const sentences = sentenceText
+    .split(SPLIT_ON)
+    .map((s) => s.trim())
+    .filter(Boolean);
+  const dedupedSentences = Array.from(new Set(sentences));
+  return dedupedSentences;
+}
diff --git a/web/src/components/error.tsx b/web/src/components/error.tsx
index b7256f6f..077eeeed 100644
--- a/web/src/components/error.tsx
+++ b/web/src/components/error.tsx
@@ -1,10 +1,23 @@
 import React from 'react';
 import { Localized } from '@fluent/react';
 
-export default function Footer({ translationKey }: { translationKey: string }) {
-  return (
-    <Localized id={translationKey}>
-      <div className="error-box"></div>
-    </Localized>
-  );
+type Props = {
+  translationKey?: string;
+  children?: React.ReactNode;
+};
+
+export default function Error({ translationKey, children }: Props) {
+  if (translationKey) {
+    return (
+      <Localized id={translationKey}>
+        <div className="error-box"></div>
+      </Localized>
+    );
+  }
+
+  if (children) {
+    return <div className="error-box">{children}</div>;
+  }
+
+  return null;
 }
diff --git a/web/src/components/pages/add.tsx b/web/src/components/pages/add.tsx
index 6d1415ab..0b482bf7 100644
--- a/web/src/components/pages/add.tsx
+++ b/web/src/components/pages/add.tsx
@@ -21,7 +21,7 @@ export default function Add() {
   const dispatch = useDispatch();
   const [language, setLanguage] = useState<string>('');
   const [source, setSource] = useState<string>('');
-  const [message, setMessage] = useState<string>('');
+  const [duplicates, setDuplicates] = useState<number | undefined>();
   const [error, setError] = useState<string>('');
   const [submitted, setSubmitted] = useState<string[]>([]);
   const [unreviewed, setUnreviewed] = useState<string[]>([]);
@@ -44,7 +44,7 @@ export default function Add() {
   const resetState = () => {
     setLanguage('');
     setSource('');
-    setMessage('');
+    setDuplicates(undefined);
     setError('');
     setSubmitted([]);
     setUnreviewed([]);
@@ -91,7 +91,7 @@ export default function Add() {
       }
 
       resetState();
-      setMessage(l10n.getString('sc-add-result', { duplicates: duplicates }));
+      setDuplicates(duplicates);
       setError(
         errors && errors.length > 0
           ? l10n.getString('sc-add-err-failed', { sentences: errors.length })
@@ -157,7 +157,7 @@ export default function Add() {
   return (
     <SubmitForm
       onSubmit={onSubmit}
-      message={message}
+      duplicates={duplicates}
       error={error}
       languages={availableLanguages}
       sentenceSubmissionFailures={sentenceSubmissionFailures}
diff --git a/web/src/components/pages/my-sentences-list.tsx b/web/src/components/pages/my-sentences-list.tsx
index 32771893..21367de6 100644
--- a/web/src/components/pages/my-sentences-list.tsx
+++ b/web/src/components/pages/my-sentences-list.tsx
@@ -5,6 +5,7 @@ import '../../../css/sentences-list.css';
 
 import { sendRequest } from '../../backend';
 import type { SentenceRecord } from '../../types';
+import Error from '../error';
 import Sentence from '../sentence';
 import SpinnerButton from '../spinner-button';
 
@@ -85,16 +86,15 @@ export default function MySentencesList() {
       </Localized>
 
       {sentencesLoading && (
-        <Localized id="sc-my-loading">
-          <p></p>
-        </Localized>
+        <div className="loading-container">
+          <span className="spinning" />
+          <Localized id="sc-my-loading">
+            <p></p>
+          </Localized>
+        </div>
       )}
 
-      {error && (
-        <Localized id="sc-my-err-fetching">
-          <p></p>
-        </Localized>
-      )}
+      {error && <Error translationKey="sc-my-err-fetching" />}
 
       {hasNoSentences && !sentencesLoading && (
         <Localized id="sc-my-no-sentences">
diff --git a/web/src/components/pages/rejected-sentences-list.tsx b/web/src/components/pages/rejected-sentences-list.tsx
index 9741cbc5..3370172c 100644
--- a/web/src/components/pages/rejected-sentences-list.tsx
+++ b/web/src/components/pages/rejected-sentences-list.tsx
@@ -3,6 +3,7 @@ import { Localized, useLocalization } from '@fluent/react';
 
 import { sendRequest } from '../../backend';
 import type { SentenceRecord } from '../../types';
+import Error from '../error';
 import Sentence from '../sentence';
 
 export type RejectedSentences = Record<string, SentenceRecord[]>;
@@ -37,16 +38,15 @@ export default function RejectedSentencesList() {
       </Localized>
 
       {sentencesLoading && (
-        <Localized id="sc-rejected-loading">
-          <p></p>
-        </Localized>
+        <div className="loading-container">
+          <span className="spinning" />
+          <Localized id="sc-rejected-loading">
+            <p></p>
+          </Localized>
+        </div>
       )}
 
-      {error && (
-        <Localized id="sc-rejected-err-fetching">
-          <p></p>
-        </Localized>
-      )}
+      {error && <Error translationKey="sc-rejected-err-fetching" />}
 
       {hasNoSentences && !sentencesLoading && !error && (
         <Localized id="sc-rejected-none-found">
diff --git a/web/src/components/pages/review.tsx b/web/src/components/pages/review.tsx
index 9c840f10..1432d778 100644
--- a/web/src/components/pages/review.tsx
+++ b/web/src/components/pages/review.tsx
@@ -103,29 +103,21 @@ export default function Review() {
 
   return (
     <div>
-      <section>
-        <Localized id="sc-review-title">
-          <h1></h1>
-        </Localized>
-
-        {fetchFailure && <Error translationKey="sc-languages-fetch-error" />}
+      <Localized id="sc-review-title">
+        <h1></h1>
+      </Localized>
 
-        <div className="row">
-          <LanguageSelector
-            labelText=""
-            languages={languages}
-            selected={language}
-            onChange={onSelectLanguage}
-          />
-          <ReviewCriteria />
-        </div>
-      </section>
+      {fetchFailure && <Error translationKey="sc-languages-fetch-error" />}
 
-      {sentencesLoading && (
-        <Localized id="sc-review-loading">
-          <p></p>
-        </Localized>
-      )}
+      <div className="row">
+        <LanguageSelector
+          labelText=""
+          languages={languages}
+          selected={language}
+          onChange={onSelectLanguage}
+        />
+        <ReviewCriteria />
+      </div>
 
       {!language && (
         <Localized id="sc-review-select-language">
@@ -133,6 +125,15 @@ export default function Review() {
         </Localized>
       )}
 
+      {sentencesLoading && (
+        <div className="loading-container">
+          <span className="spinning" />
+          <Localized id="sc-review-loading">
+            <p></p>
+          </Localized>
+        </div>
+      )}
+
       {hasNoSentences && (
         <Localized
           id="sc-review-no-sentences"
diff --git a/web/src/components/pages/stats.tsx b/web/src/components/pages/stats.tsx
index 384b641e..ce508d10 100644
--- a/web/src/components/pages/stats.tsx
+++ b/web/src/components/pages/stats.tsx
@@ -21,9 +21,11 @@ export default function Stats() {
   const [hasError, setHasError] = useState(false);
   const [loading, setLoading] = useState(false);
   const [language, setLanguage] = useState<string>();
-  const { languages, pendingLanguages, fetchFailure } = useSelector(
-    (state: RootState) => state.languages
-  );
+  const {
+    languages,
+    pendingLanguages,
+    fetchFailure: languageFetchFailure,
+  } = useSelector((state: RootState) => state.languages);
 
   const isLoading = loading || pendingLanguages;
 
@@ -61,7 +63,7 @@ export default function Stats() {
         <h1></h1>
       </Localized>
 
-      {fetchFailure && <Error translationKey="sc-languages-fetch-error" />}
+      {languageFetchFailure && <Error translationKey="sc-languages-fetch-error" />}
 
       <div className="row">
         <LanguageSelector
@@ -72,16 +74,15 @@ export default function Stats() {
         />
       </div>
 
-      {isLoading && (
-        <Localized id="sc-stats-updating">
-          <p></p>
-        </Localized>
-      )}
+      {hasError && <Error translationKey="sc-stats-error" />}
 
-      {!isLoading && hasError && (
-        <Localized id="sc-stats-error">
-          <p></p>
-        </Localized>
+      {isLoading && (
+        <div className="loading-container">
+          <span className="spinning" />
+          <Localized id="sc-stats-updating">
+            <p></p>
+          </Localized>
+        </div>
       )}
 
       {!isLoading && !hasError && (
diff --git a/web/src/components/profile/add-language-section.tsx b/web/src/components/profile/add-language-section.tsx
index 74a527d7..4eb532c2 100644
--- a/web/src/components/profile/add-language-section.tsx
+++ b/web/src/components/profile/add-language-section.tsx
@@ -5,6 +5,7 @@ import { Localized, useLocalization } from '@fluent/react';
 import { addLanguage } from '../../actions/languages';
 import type { RootState } from '../../types';
 
+import Error from '../error';
 import LanguageSelector from '../language-selector';
 
 export default function AddLanguage() {
@@ -36,7 +37,7 @@ export default function AddLanguage() {
 
   return (
     <section>
-      {error && <p className="error-message">{error}</p>}
+      {error && <Error>{error}</Error>}
 
       <LanguageSelector
         selected={language}
diff --git a/web/src/components/profile/personal-language-info.tsx b/web/src/components/profile/personal-language-info.tsx
index 80654bdf..9dc113dc 100644
--- a/web/src/components/profile/personal-language-info.tsx
+++ b/web/src/components/profile/personal-language-info.tsx
@@ -31,7 +31,7 @@ export default function PersonalLanguageInfo() {
 
   return (
     <section>
-      {error && <p className="error-message">{error}</p>}
+      {error && <Error>{error}</Error>}
 
       {fetchFailure && <Error translationKey="sc-languages-fetch-error" />}
 
diff --git a/web/src/components/profile/settings.tsx b/web/src/components/profile/settings.tsx
index 623efd49..64eb3ebd 100644
--- a/web/src/components/profile/settings.tsx
+++ b/web/src/components/profile/settings.tsx
@@ -8,6 +8,7 @@ import { resetSkippedSentences } from '../../actions/sentences';
 import { DEFAULT_LOCALE } from '../../l10n';
 import type { RootState } from '../../types';
 
+import Error from '../error';
 import LanguageSelector from '../language-selector';
 
 const CV_TRANSLATED_LOCALES: string[] = cvTranslatedLocales;
@@ -41,11 +42,7 @@ export default function Settings() {
         <h2></h2>
       </Localized>
 
-      {showErrorMessage && (
-        <Localized id="sc-settings-failed">
-          <p className="form-error"></p>
-        </Localized>
-      )}
+      {showErrorMessage && <Error translationKey="sc-settings-failed" />}
 
       <section>
         <section className="settings-section">
diff --git a/web/src/components/success.tsx b/web/src/components/success.tsx
new file mode 100644
index 00000000..1ba16984
--- /dev/null
+++ b/web/src/components/success.tsx
@@ -0,0 +1,33 @@
+import React from 'react';
+import { Localized } from '@fluent/react';
+
+type Props = {
+  children?: React.ReactNode;
+  translationKey?: string;
+  vars?: Record<string, any>; // eslint-disable-line @typescript-eslint/no-explicit-any
+};
+
+export default function Success({ translationKey, vars, children }: Props) {
+  const hasVars = vars && Object.keys(vars).length > 0;
+  if (translationKey && !hasVars) {
+    return (
+      <Localized id={translationKey}>
+        <div className="success-box"></div>
+      </Localized>
+    );
+  }
+
+  if (translationKey && hasVars) {
+    return (
+      <Localized id={translationKey} vars={vars}>
+        <div className="success-box"></div>
+      </Localized>
+    );
+  }
+
+  if (children) {
+    return <div className="success-box">{children}</div>;
+  }
+
+  return null;
+}