Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Status bar error state improvements #3299

Merged
merged 8 commits into from Nov 15, 2021
Merged

Status bar error state improvements #3299

merged 8 commits into from Nov 15, 2021

Conversation

Murderlon
Copy link
Member

@Murderlon Murderlon commented Nov 4, 2021

Fixes #3252

  • 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.

Before

Capture d’écran 2021-11-03 à 15 39 46

After

Capture d’écran 2021-11-04 à 13 59 32

Questions

  • Instead of the ? button next to the file name with the error in an alert, maybe we can re-use the design of Required meta fields UI #3285 (see design screenshot in that PR)?

@Murderlon Murderlon added the Status Bar Issues relating to the @uppy/status-bar plugin label Nov 4, 2021
examples/dev/Dashboard.html Outdated Show resolved Hide resolved
@arturi
Copy link
Contributor

arturi commented Nov 4, 2021

Nice work! The idea about re-using the failed meta field UI is valid, but I fear the error messages we hide behind ? would be too long. Maybe we show part and then underlined ....

@Murderlon
Copy link
Member Author

Murderlon commented Nov 8, 2021

Nice work! The idea about re-using the failed meta field UI is valid, but I fear the error messages we hide behind ? would be too long. Maybe we show part and then underlined ....

I don't think we should ever truncate important information. So in that case I think we should stick this with and check later whether it makes sense to switch.

* main:
  @uppy/aws-s3: use `Promise.allSettled` insead of custom utils (#3079)
  Convert sets to arrays prior to using spread operator to enable transpilation to ES5 (#3297)
  Add showRecordingLength to webcam types (#3303)
  Required meta fields UI (#3285)
  Update BACKLOG.md
  Release uppy@2.2.2
  @uppy/provider-views@2.0.5
@arturi
Copy link
Contributor

arturi commented Nov 8, 2021

It’s true though that it’s hard to tell which file has failed, the error message with red background would help that a lot:

/cc @nqst

@Murderlon
Copy link
Member Author

It’s true though that it’s hard to tell which file has failed, the error message with red background would help that a lot:

I think it's pretty clear for now with the green checked icon versus the darker overlay with retry icon.

@nqst
Copy link
Contributor

nqst commented Nov 9, 2021

I agree it could be even more clear. One idea is to add a bright background behind the Retry button. Here's a rough sketch:

image

Some notes:

  • I slightly grayed out the failed file details section.
  • One more idea is that we can dim the failed file thumbnail as well (using a white overlay instead of a black one) — so that it looks more "problematic" compared to the uploaded file.

@arturi
Copy link
Contributor

arturi commented Nov 9, 2021

I like it! Will take a note to implement in some follow up, outside of this PR, thanks @nqst.

@Murderlon Murderlon requested a review from arturi November 9, 2021 11:38
arturi and others added 3 commits November 9, 2021 20:16
* main:
  Refactor locale scripts & generate types and docs (#3276)
  Remove references of incorrect `options` argument for `companion.socket` (#3307)
  Upgrade linting to 2.0.0-0 (#3280)
  Release uppy@2.1.2, @uppy/robodog@2.1.3
  @uppy/core@2.1.2
  meta: update version references in docs
  Release @uppy/robodog@2.1.2
  @uppy/unsplash@2.0.3
@Murderlon
Copy link
Member Author

@arturi can I merge this?

this.failed = true
setTimeout(() => this.emitError(new Error('test')), 1000)
return Promise.reject(new Error('test'))
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a deliberate change or was meant to be removed after testing?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooh good catch this was taking along from a different branch.

@arturi
Copy link
Contributor

arturi commented Nov 12, 2021

Also keep getting this, because of missing showErrorDetails:

Screenshot 2021-11-12 at 00 51 54

@Murderlon
Copy link
Member Author

Turns out a really messed up the merge. Now double checked to see if it worked and it does.

@arturi arturi self-requested a review November 12, 2021 21:04
@Murderlon Murderlon merged commit 6add4ef into main Nov 15, 2021
@Murderlon Murderlon deleted the status-error-state branch November 15, 2021 10:15
Murderlon added a commit that referenced this pull request Nov 15, 2021
* main:
  website: update documents that were out of date (#3317)
  Status bar error state improvements (#3299)
  Fix typo in `docs/drag-drop.md` (#3319)
  website: Update /support and docs about Transloadit-hosted Companion (#3243)
@netdown
Copy link

netdown commented Nov 24, 2021

@Murderlon The status message is still stuck in my case with 2.2.3.
image

@Murderlon
Copy link
Member Author

@netdown do you have a reproducible example by chance?

@netdown
Copy link

netdown commented Nov 24, 2021

@Murderlon Sure, you can check at https://dev.fotologus.hu/en/component/photoorder/order2. Just click the orange button to select files, then the Uppy UI will appear.

@netdown
Copy link

netdown commented Nov 28, 2021

@Murderlon The problem seems to be that the uploadComplete flag is always false. Either uppy does not update it or the status bar incorrectly relys on it. Its actually fine with the following:
uppy.on('complete', (result) => { result.failed.forEach(failedFile => { uppy.setFileState(failedFile.id, { progress: {...failedFile.progress, uploadComplete: true } }); }); })

@github-actions github-actions bot mentioned this pull request Dec 7, 2021
github-actions bot added a commit that referenced this pull request Dec 7, 2021
| Package                   | Version | Package                   | Version |
| ------------------------- | ------- | ------------------------- | ------- |
| @uppy/angular             |   0.2.6 | @uppy/locales             |   2.0.4 |
| @uppy/audio               |   0.2.0 | @uppy/onedrive            |   2.0.5 |
| @uppy/aws-s3              |   2.0.6 | @uppy/provider-views      |   2.0.6 |
| @uppy/aws-s3-multipart    |   2.2.0 | @uppy/react               |   2.1.2 |
| @uppy/box                 |   1.0.5 | @uppy/screen-capture      |   2.0.5 |
| @uppy/companion           |   3.1.2 | @uppy/status-bar          |   2.1.2 |
| @uppy/companion-client    |   2.0.4 | @uppy/store-default       |   2.0.3 |
| @uppy/core                |   2.1.3 | @uppy/thumbnail-generator |   2.0.6 |
| @uppy/dashboard           |   2.1.2 | @uppy/transloadit         |   2.0.5 |
| @uppy/drag-drop           |   2.0.5 | @uppy/tus                 |   2.1.2 |
| @uppy/dropbox             |   2.0.5 | @uppy/url                 |   2.0.5 |
| @uppy/facebook            |   2.0.5 | @uppy/utils               |   4.0.4 |
| @uppy/file-input          |   2.0.5 | @uppy/webcam              |   2.0.5 |
| @uppy/golden-retriever    |   2.0.6 | @uppy/xhr-upload          |   2.0.6 |
| @uppy/google-drive        |   2.0.5 | @uppy/zoom                |   1.0.5 |
| @uppy/image-editor        |   1.1.0 | @uppy/robodog             |   2.1.4 |
| @uppy/informer            |   2.0.5 | uppy                      |   2.3.0 |
| @uppy/instagram           |   2.0.5 |                           |         |

- meta: add release automations (Antoine du Hamel / #3304)
- @uppy/dashboard: Save meta fields when opening the image editor (Merlijn Vos / #3339)
- @uppy/aws-s3-multipart: Drop `lockedCandidatesForBatch` and mark chunks as busy when preparing (Yegor Yarko / #3342)
- @uppy/webcam: fix broken links in `webcam.md` (Antoine du Hamel / #3346)
- @uppy/audio: new @uppy/audio plugin for recording with microphone (Artur Paikin / #2976)
- build: force use of `@babel/plugin-proposal-optional-chaining` (Antoine du Hamel / #3335)
- @uppy/companion: fix deploy Yarn version (Antoine du Hamel / #3327)
- @uppy/companion: upgrade aws-sdk (Mikael Finstad / #3334)
- @uppy/core: disable loose transpilation for legacy bundle (Antoine du Hamel / #3329)
- @uppy/angular: examples: update `angular-example` to Angular v13 (Antoine du Hamel / #3325)
- meta: Update BACKLOG.md (Artur Paikin, Merlijn Vos)
- meta: Add disableLocalFiles to options summary (Steve Barker / #3323)
- meta: Create SECURITY.md (Ziding Zhang / #3052)
- @uppy/image-editor: Pass croppedCanvasOptions to getCroppedCanvas (Mohamed Boudra / #3320)
- meta: finish `master`->`main` job (Mikael Finstad / #3315)
- website: update documents that were out of date (Antoine du Hamel / #3317)
- @uppy/status-bar: Status bar error state improvements (Merlijn Vos / #3299)
- doc: Fix typo in `docs/drag-drop.md` (Ash Allen / #3319)
- website: Update /support and docs about Transloadit-hosted Companion (Artur Paikin / #3243)
- @uppy/aws-s3,@uppy/box,@uppy/core,@uppy/dashboard,@uppy/drag-drop,@uppy/dropbox,@uppy/facebook,@uppy/file-input,@uppy/google-drive,@uppy/image-editor,@uppy/instagram,@uppy/locales,@uppy/onedrive,@uppy/screen-capture,@uppy/status-bar,@uppy/thumbnail-generator,@uppy/transloadit,@uppy/url,@uppy/webcam,@uppy/xhr-upload,@uppy/zoom: Refactor locale scripts & generate types and docs (Merlijn Vos / #3276)
- @uppy/companion: Remove references of incorrect `options` argument for `companion.socket` (Mikael Finstad / #3307)
- @uppy/companion: Upgrade linting to 2.0.0-0 (Kevin van Zonneveld / #3280)
vymao pushed a commit to vymao/uppy that referenced this pull request Mar 29, 2022
* main:
  website: update documents that were out of date (transloadit#3317)
  Status bar error state improvements (transloadit#3299)
  Fix typo in `docs/drag-drop.md` (transloadit#3319)
  website: Update /support and docs about Transloadit-hosted Companion (transloadit#3243)
HeavenFox pushed a commit to docsend/uppy that referenced this pull request Jun 27, 2023
| Package                   | Version | Package                   | Version |
| ------------------------- | ------- | ------------------------- | ------- |
| @uppy/angular             |   0.2.6 | @uppy/locales             |   2.0.4 |
| @uppy/audio               |   0.2.0 | @uppy/onedrive            |   2.0.5 |
| @uppy/aws-s3              |   2.0.6 | @uppy/provider-views      |   2.0.6 |
| @uppy/aws-s3-multipart    |   2.2.0 | @uppy/react               |   2.1.2 |
| @uppy/box                 |   1.0.5 | @uppy/screen-capture      |   2.0.5 |
| @uppy/companion           |   3.1.2 | @uppy/status-bar          |   2.1.2 |
| @uppy/companion-client    |   2.0.4 | @uppy/store-default       |   2.0.3 |
| @uppy/core                |   2.1.3 | @uppy/thumbnail-generator |   2.0.6 |
| @uppy/dashboard           |   2.1.2 | @uppy/transloadit         |   2.0.5 |
| @uppy/drag-drop           |   2.0.5 | @uppy/tus                 |   2.1.2 |
| @uppy/dropbox             |   2.0.5 | @uppy/url                 |   2.0.5 |
| @uppy/facebook            |   2.0.5 | @uppy/utils               |   4.0.4 |
| @uppy/file-input          |   2.0.5 | @uppy/webcam              |   2.0.5 |
| @uppy/golden-retriever    |   2.0.6 | @uppy/xhr-upload          |   2.0.6 |
| @uppy/google-drive        |   2.0.5 | @uppy/zoom                |   1.0.5 |
| @uppy/image-editor        |   1.1.0 | @uppy/robodog             |   2.1.4 |
| @uppy/informer            |   2.0.5 | uppy                      |   2.3.0 |
| @uppy/instagram           |   2.0.5 |                           |         |

- meta: add release automations (Antoine du Hamel / transloadit#3304)
- @uppy/dashboard: Save meta fields when opening the image editor (Merlijn Vos / transloadit#3339)
- @uppy/aws-s3-multipart: Drop `lockedCandidatesForBatch` and mark chunks as busy when preparing (Yegor Yarko / transloadit#3342)
- @uppy/webcam: fix broken links in `webcam.md` (Antoine du Hamel / transloadit#3346)
- @uppy/audio: new @uppy/audio plugin for recording with microphone (Artur Paikin / transloadit#2976)
- build: force use of `@babel/plugin-proposal-optional-chaining` (Antoine du Hamel / transloadit#3335)
- @uppy/companion: fix deploy Yarn version (Antoine du Hamel / transloadit#3327)
- @uppy/companion: upgrade aws-sdk (Mikael Finstad / transloadit#3334)
- @uppy/core: disable loose transpilation for legacy bundle (Antoine du Hamel / transloadit#3329)
- @uppy/angular: examples: update `angular-example` to Angular v13 (Antoine du Hamel / transloadit#3325)
- meta: Update BACKLOG.md (Artur Paikin, Merlijn Vos)
- meta: Add disableLocalFiles to options summary (Steve Barker / transloadit#3323)
- meta: Create SECURITY.md (Ziding Zhang / transloadit#3052)
- @uppy/image-editor: Pass croppedCanvasOptions to getCroppedCanvas (Mohamed Boudra / transloadit#3320)
- meta: finish `master`->`main` job (Mikael Finstad / transloadit#3315)
- website: update documents that were out of date (Antoine du Hamel / transloadit#3317)
- @uppy/status-bar: Status bar error state improvements (Merlijn Vos / transloadit#3299)
- doc: Fix typo in `docs/drag-drop.md` (Ash Allen / transloadit#3319)
- website: Update /support and docs about Transloadit-hosted Companion (Artur Paikin / transloadit#3243)
- @uppy/aws-s3,@uppy/box,@uppy/core,@uppy/dashboard,@uppy/drag-drop,@uppy/dropbox,@uppy/facebook,@uppy/file-input,@uppy/google-drive,@uppy/image-editor,@uppy/instagram,@uppy/locales,@uppy/onedrive,@uppy/screen-capture,@uppy/status-bar,@uppy/thumbnail-generator,@uppy/transloadit,@uppy/url,@uppy/webcam,@uppy/xhr-upload,@uppy/zoom: Refactor locale scripts & generate types and docs (Merlijn Vos / transloadit#3276)
- @uppy/companion: Remove references of incorrect `options` argument for `companion.socket` (Mikael Finstad / transloadit#3307)
- @uppy/companion: Upgrade linting to 2.0.0-0 (Kevin van Zonneveld / transloadit#3280)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status Bar Issues relating to the @uppy/status-bar plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dashboard uploading state gets stuck if one of the uploads fail
4 participants