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

Render processing view for stalled uploads #1102

Merged
merged 4 commits into from
Oct 9, 2024

Conversation

radical-ube
Copy link
Contributor

@radical-ube radical-ube commented Sep 26, 2024

resolves #1094

This PR solves a "hidden timeout" issue users were experiencing, by rendering a "still processing" message for "stalled" uploads. The solution i chose to solve for this was to opt for rendering the _processing partial instead of the _error partial for the upload_stalled? user conditional flow. I did this because i saw that the only difference between upload_processing? and upload_stalled? was the retryable condition. Therefore, a "stalled" upload should still be processing. A side benefit to using the _processing partial is that it continually polls the server, whereas the _error partial does not; if a "stalled" upload eventually completed, the _error view wouldn't refresh on its own to a _complete view, whereas the _processing view does.

Within the _processing partial, i added the retry button that is conditionally present in the _error partial, as well as a small (conditional) message underneath the field that notifies the user if the processing is taking longer than expected. This way, we keep the functionality for a user to choose to retry.

@cavis for functional review
@brandonhundt for language review

Copy link
Member

@cavis cavis left a comment

Choose a reason for hiding this comment

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

Makes good sense to me!

Searching through the code, it looks like Transcripts and Images both use basically identical retry logic to this. Can we port this to those places as well?

@@ -649,6 +649,7 @@ en:
title: MP3 Not Provided
processing:
hint: Processing
still_processing: Processing is taking longer than expected. Thank you for waiting.
Copy link
Member

Choose a reason for hiding this comment

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

Nice, I do like how this isn't highlighted in red now. Looks less angry:

image

I might prompt them that it's cool to click the retry button though... "Processing is taking longer than expected - you can continue waiting or retry"

Copy link
Contributor

Choose a reason for hiding this comment

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

FWIW: I prefer the term "patience" over "waiting", ie: "thank you for your patience"... Throw in a llittle "Ope" and we might even get the user to chuckle in another wise frustrating moment.

I suggest:
"Ope, file processing is taking longer than expected - you can continue waiting or retry"

@brandonhundt brandonhundt requested review from cavis and removed request for brandonhundt October 2, 2024 18:58
Copy link
Member

@cavis cavis left a comment

Choose a reason for hiding this comment

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

:+3: nice. The spinner being active really conveys that processing stalled, and it's not as angry as landing in error state.

@radical-ube radical-ube merged commit d39a96c into main Oct 9, 2024
3 checks passed
@radical-ube radical-ube deleted the feat/display_media_timeout_errors branch October 9, 2024 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Media processing errors vs timeouts
3 participants