From 6add4ef0318b2a4a28428c982ec07194030fc61c Mon Sep 17 00:00:00 2001
From: Merlijn Vos <merlijn@soverin.net>
Date: Mon, 15 Nov 2021 10:15:52 +0000
Subject: [PATCH] Status bar error state improvements (#3299)

- Show error state of status bar if there is only _one_ failed file instead of _all_ failed files. Used to be stuck in the uploading state when a file failed.
- Add "x of x files uploaded" below "Upload failed" for extra context.
- Improve the error details button styling in the statusbar
- Improve the error details button styling in the file info card. Now placed next to the file name.
- Set status bar state to complete if the user manually removes the failed files.
---
 .../src/components/FileItem/FileInfo/index.js | 17 +++---
 .../components/FileItem/FileInfo/index.scss   |  9 +++
 .../src/components/FileItem/index.scss        | 11 ++--
 packages/@uppy/status-bar/src/Components.js   | 55 +++++++++----------
 packages/@uppy/status-bar/src/StatusBar.js    | 19 ++++---
 packages/@uppy/status-bar/src/index.js        |  6 +-
 packages/@uppy/status-bar/src/locale.js       |  1 +
 packages/@uppy/status-bar/src/style.scss      |  5 ++
 8 files changed, 72 insertions(+), 51 deletions(-)

diff --git a/packages/@uppy/dashboard/src/components/FileItem/FileInfo/index.js b/packages/@uppy/dashboard/src/components/FileItem/FileInfo/index.js
index c9c974b286..b7d08afd05 100644
--- a/packages/@uppy/dashboard/src/components/FileItem/FileInfo/index.js
+++ b/packages/@uppy/dashboard/src/components/FileItem/FileInfo/index.js
@@ -77,7 +77,7 @@ const ErrorButton = ({ file, onClick }) => {
   if (file.error) {
     return (
       <button
-        className="uppy-Dashboard-Item-errorDetails"
+        className="uppy-u-reset uppy-Dashboard-Item-errorDetails"
         aria-label={file.error}
         data-microtip-position="bottom"
         data-microtip-size="medium"
@@ -92,22 +92,25 @@ const ErrorButton = ({ file, onClick }) => {
 }
 
 module.exports = function FileInfo (props) {
+  const { file } = props
   return (
     <div
       className="uppy-Dashboard-Item-fileInfo"
-      data-uppy-file-source={props.file.source}
+      data-uppy-file-source={file.source}
     >
-      {renderFileName(props)}
-      <div className="uppy-Dashboard-Item-status">
-        {renderAuthor(props)}
-        {renderFileSize(props)}
-        {ReSelectButton(props)}
+      <div className="uppy-Dashboard-Item-fileName">
+        {renderFileName(props)}
         <ErrorButton
           file={props.file}
           // eslint-disable-next-line no-alert
           onClick={() => alert(props.file.error)} // TODO: move to a custom alert implementation
         />
       </div>
+      <div className="uppy-Dashboard-Item-status">
+        {renderAuthor(props)}
+        {renderFileSize(props)}
+        {ReSelectButton(props)}
+      </div>
       <MetaErrorMessage
         file={props.file}
         i18n={props.i18n}
diff --git a/packages/@uppy/dashboard/src/components/FileItem/FileInfo/index.scss b/packages/@uppy/dashboard/src/components/FileItem/FileInfo/index.scss
index 4f7dab37fa..c3ccdfb653 100644
--- a/packages/@uppy/dashboard/src/components/FileItem/FileInfo/index.scss
+++ b/packages/@uppy/dashboard/src/components/FileItem/FileInfo/index.scss
@@ -17,6 +17,15 @@
   }
 }
 
+.uppy-Dashboard-Item-fileName {
+  display: flex;
+  align-items: baseline;
+
+  button {
+    margin-left: 5px;
+  }
+}
+
 .uppy-Dashboard-Item-author {
   color: $gray-600;
   vertical-align: bottom;
diff --git a/packages/@uppy/dashboard/src/components/FileItem/index.scss b/packages/@uppy/dashboard/src/components/FileItem/index.scss
index ccfd34d6f5..7472d82584 100644
--- a/packages/@uppy/dashboard/src/components/FileItem/index.scss
+++ b/packages/@uppy/dashboard/src/components/FileItem/index.scss
@@ -143,21 +143,22 @@
 
 .uppy-Dashboard-Item-errorDetails {
   position: relative;
-  top: -1px;
+  top: 0;
   display: inline-block;
-  width: 12px;
-  height: 12px;
+  width: 13px;
+  height: 13px;
   color: $white;
   font-weight: 600;
-  font-size: 8px;
+  font-size: 10px;
   line-height: 12px;
   text-align: center;
   vertical-align: middle;
   background-color: $gray-500;
   border-radius: 50%;
+  border: none;
   cursor: help;
   appearance: none;
-  inset-inline-start: 6px;
+  inset-inline-start: 2px;
 }
 
 .uppy-Dashboard-Item-errorDetails::after {
diff --git a/packages/@uppy/status-bar/src/Components.js b/packages/@uppy/status-bar/src/Components.js
index 8eb5765a3c..428e2edf5d 100644
--- a/packages/@uppy/status-bar/src/Components.js
+++ b/packages/@uppy/status-bar/src/Components.js
@@ -246,7 +246,7 @@ function ProgressDetails (props) {
   )
 }
 
-function UnknownProgressDetails (props) {
+function FileUploadCount (props) {
   const { i18n, complete, numUploads } = props
 
   return (
@@ -327,7 +327,7 @@ function ProgressBarUploading (props) {
         )
       }
       return (
-        <UnknownProgressDetails
+        <FileUploadCount
           i18n={i18n}
           complete={complete}
           numUploads={numUploads}
@@ -388,7 +388,7 @@ function ProgressBarComplete (props) {
 }
 
 function ProgressBarError (props) {
-  const { error, i18n } = props
+  const { error, i18n, complete, numUploads } = props
 
   function displayErrorAlert () {
     const errorMessage = `${i18n('uploadFailed')} \n\n ${error}`
@@ -397,36 +397,35 @@ function ProgressBarError (props) {
   }
 
   return (
-    <div
-      className="uppy-StatusBar-content"
-      role="alert"
-      title={i18n('uploadFailed')}
-    >
+    <div className="uppy-StatusBar-content" title={i18n('uploadFailed')}>
+      <svg
+        aria-hidden="true"
+        focusable="false"
+        className="uppy-StatusBar-statusIndicator uppy-c-icon"
+        width="11"
+        height="11"
+        viewBox="0 0 11 11"
+      >
+        <path d="M4.278 5.5L0 1.222 1.222 0 5.5 4.278 9.778 0 11 1.222 6.722 5.5 11 9.778 9.778 11 5.5 6.722 1.222 11 0 9.778z" />
+      </svg>
       <div className="uppy-StatusBar-status">
         <div className="uppy-StatusBar-statusPrimary">
-          <svg
-            aria-hidden="true"
-            focusable="false"
-            className="uppy-StatusBar-statusIndicator uppy-c-icon"
-            width="11"
-            height="11"
-            viewBox="0 0 11 11"
-          >
-            <path d="M4.278 5.5L0 1.222 1.222 0 5.5 4.278 9.778 0 11 1.222 6.722 5.5 11 9.778 9.778 11 5.5 6.722 1.222 11 0 9.778z" />
-          </svg>
           {i18n('uploadFailed')}
+
+          <button
+            className="uppy-u-reset uppy-StatusBar-details"
+            aria-label={i18n('showErrorDetails')}
+            data-microtip-position="top-right"
+            data-microtip-size="medium"
+            onClick={displayErrorAlert}
+            type="button"
+          >
+            ?
+          </button>
         </div>
+
+        <FileUploadCount i18n={i18n} complete={complete} numUploads={numUploads} />
       </div>
-      <button
-        className="uppy-StatusBar-details"
-        aria-label={error}
-        data-microtip-position="top-right"
-        data-microtip-size="medium"
-        onClick={displayErrorAlert}
-        type="button"
-      >
-        ?
-      </button>
     </div>
   )
 }
diff --git a/packages/@uppy/status-bar/src/StatusBar.js b/packages/@uppy/status-bar/src/StatusBar.js
index 7f52cbeee6..b5ab955e81 100644
--- a/packages/@uppy/status-bar/src/StatusBar.js
+++ b/packages/@uppy/status-bar/src/StatusBar.js
@@ -139,7 +139,7 @@ function StatusBar (props) {
     && !hidePauseResumeButton
     && uploadState === STATE_UPLOADING
 
-  const showRetryBtn = error && !hideRetryButton
+  const showRetryBtn = error && !isAllComplete && !hideRetryButton
 
   const showDoneBtn = doneButtonHandler && uploadState === STATE_COMPLETE
 
@@ -171,15 +171,18 @@ function StatusBar (props) {
         switch (uploadState) {
           case STATE_PREPROCESSING:
           case STATE_POSTPROCESSING:
-            return (
-              <ProgressBarProcessing
-                progress={calculateProcessingProgress(files)}
-              />
-            )
+            return <ProgressBarProcessing progress={calculateProcessingProgress(files)} />
           case STATE_COMPLETE:
             return <ProgressBarComplete i18n={i18n} />
           case STATE_ERROR:
-            return <ProgressBarError error={error} i18n={i18n} />
+            return (
+              <ProgressBarError
+                error={error}
+                i18n={i18n}
+                numUploads={numUploads}
+                complete={complete}
+              />
+            )
           case STATE_UPLOADING:
             return (
               <ProgressBarUploading
@@ -205,7 +208,7 @@ function StatusBar (props) {
       })()}
 
       <div className="uppy-StatusBar-actions">
-        {(recoveredState || showUploadBtn) ? (
+        {recoveredState || showUploadBtn ? (
           <UploadBtn
             newFiles={newFiles}
             isUploadStarted={isUploadStarted}
diff --git a/packages/@uppy/status-bar/src/index.js b/packages/@uppy/status-bar/src/index.js
index b714657beb..f7b9abce33 100644
--- a/packages/@uppy/status-bar/src/index.js
+++ b/packages/@uppy/status-bar/src/index.js
@@ -101,7 +101,7 @@ module.exports = class StatusBar extends UIPlugin {
     return StatusBarUI({
       error,
       uploadState: getUploadingState(
-        isAllErrored,
+        error,
         isAllComplete,
         recoveredState,
         state.files || {},
@@ -180,8 +180,8 @@ function getTotalETA (files) {
   return Math.round((totalBytesRemaining / totalSpeed) * 10) / 10
 }
 
-function getUploadingState (isAllErrored, isAllComplete, recoveredState, files) {
-  if (isAllErrored) {
+function getUploadingState (error, isAllComplete, recoveredState, files) {
+  if (error && !isAllComplete) {
     return statusBarStates.STATE_ERROR
   }
 
diff --git a/packages/@uppy/status-bar/src/locale.js b/packages/@uppy/status-bar/src/locale.js
index 7da3099a3c..5ec26ece33 100644
--- a/packages/@uppy/status-bar/src/locale.js
+++ b/packages/@uppy/status-bar/src/locale.js
@@ -44,5 +44,6 @@ module.exports = {
       0: '%{smart_count} more file added',
       1: '%{smart_count} more files added',
     },
+    showErrorDetails: 'Show error details',
   },
 }
diff --git a/packages/@uppy/status-bar/src/style.scss b/packages/@uppy/status-bar/src/style.scss
index eea2c756e8..aece805b3b 100644
--- a/packages/@uppy/status-bar/src/style.scss
+++ b/packages/@uppy/status-bar/src/style.scss
@@ -129,6 +129,11 @@
 .uppy-StatusBar-statusPrimary {
   font-weight: 500;
   line-height: 1;
+  display: flex;
+
+  button.uppy-StatusBar-details {
+    margin-left: 5px;
+  }
 
   [data-uppy-theme="dark"] & {
     color: $gray-200;