Improve error handling and UX for image uploads in RichTextEditor#3359
Open
SxxAq wants to merge 5 commits intofossasia:devfrom
Open
Improve error handling and UX for image uploads in RichTextEditor#3359SxxAq wants to merge 5 commits intofossasia:devfrom
SxxAq wants to merge 5 commits intofossasia:devfrom
Conversation
Contributor
Reviewer's guide (collapsed on small PRs)Reviewer's GuideReplaces blocking alert-based error handling for image uploads in RichTextEditor with reactive, inline error UI, adds proper lifecycle cleanup, and improves logging and styling for upload failures. Sequence diagram for updated image upload error handling in RichTextEditorsequenceDiagram
actor User
participant RichTextEditor
participant QuillEditor
participant FileInput
participant ApiService
participant I18n
participant Console
User->>RichTextEditor: Click image upload button
RichTextEditor->>QuillEditor: Open image upload handler
QuillEditor->>FileInput: Trigger file selection
User->>FileInput: Select image file
FileInput-->>QuillEditor: Return selected file
QuillEditor->>RichTextEditor: Provide selected file
RichTextEditor->>RichTextEditor: uploadError = null
RichTextEditor->>RichTextEditor: uploading = true
RichTextEditor->>ApiService: uploadFilePromise(file, fileName)
alt Upload succeeds with no error
ApiService-->>RichTextEditor: data (no error)
RichTextEditor->>QuillEditor: Insert uploaded image
RichTextEditor->>RichTextEditor: uploading = false
else Upload returns error in response
ApiService-->>RichTextEditor: data.error
RichTextEditor->>I18n: t(RichTextEditor:upload:error)
I18n-->>RichTextEditor: localized message
RichTextEditor->>RichTextEditor: uploadError = localized message
RichTextEditor->>Console: error(RichTextEditor image upload failed, data.error)
RichTextEditor->>RichTextEditor: uploading = false
else Upload request fails (network or other)
ApiService--xRichTextEditor: throw error
RichTextEditor->>I18n: t(RichTextEditor:upload:error)
I18n-->>RichTextEditor: localized message
RichTextEditor->>RichTextEditor: uploadError = localized message
RichTextEditor->>Console: error(RichTextEditor image upload failed, error)
RichTextEditor->>RichTextEditor: uploading = false
end
RichTextEditor-->>User: Inline error shown in toolbar if uploadError is set
Class diagram for updated RichTextEditor component state and dependenciesclassDiagram
class RichTextEditor {
+ref outline
+ref editor
+ref quill
+ref uploading
+ref uploadError
+onMounted()
+onBeforeUnmount()
+onTextchange(delta, oldContents, source)
+onSelectionchange(range, oldRange, source)
}
class QuillEditor {
+getContents()
+setContents(contents)
+updateContents(delta)
+getSelection(focus)
+on(eventName, handler)
+off(eventName, handler)
}
class ApiService {
+uploadFilePromise(file, fileName)
}
class I18n {
+t(key)
}
class ConsoleLogger {
+error(message, details)
}
RichTextEditor --> QuillEditor : uses
RichTextEditor --> ApiService : calls
RichTextEditor --> I18n : translates
RichTextEditor --> ConsoleLogger : logs
note for RichTextEditor "uploading controls bunt-progress-circular visibility; uploadError controls .upload-error inline message styling"
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The new
i18n.t('RichTextEditor:upload:error')key should be added toen.json(and any other relevant locale files) to avoid runtime fallback or missing-translation behavior for the upload error message. - Consider including additional context (e.g., the file name or upload endpoint) in the
console.error('RichTextEditor image upload failed', ...)logs to make debugging upload issues easier.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `i18n.t('RichTextEditor:upload:error')` key should be added to `en.json` (and any other relevant locale files) to avoid runtime fallback or missing-translation behavior for the upload error message.
- Consider including additional context (e.g., the file name or upload endpoint) in the `console.error('RichTextEditor image upload failed', ...)` logs to make debugging upload issues easier.
## Individual Comments
### Comment 1
<location path="app/eventyay/webapp/video/src/components/RichTextEditor.vue" line_range="104-110" />
<code_context>
+ uploadError.value = null
uploading.value = true
api.uploadFilePromise(file, file.name).then(data => {
if (data.error) {
- alert(`Upload error: ${data.error}`)
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider using `finally` to centralize `uploading.value = false` cleanup.
Because `uploading.value = false` is in both `then` and `catch`, any error thrown before those lines run could leave `uploading.value` stuck at `true` and the spinner never clears. Moving the cleanup into `.finally(() => { uploading.value = false })` avoids duplication and ensures the flag is always reset, even if future changes add intermediate failures.
Suggested implementation:
```
api.uploadFilePromise(file, file.name).then(data => {
if (data.error) {
uploadError.value = i18n.t('RichTextEditor:upload:error')
console.error('RichTextEditor image upload failed', data.error)
} else {
const range = instance.getSelection(true)
instance.updateContents(new Delta()
}
}).catch(error => {
```
You’ll also need to:
1. Locate the end of the `.catch(error => { ... })` block for this same promise chain.
2. Remove any existing `uploading.value = false` from inside the `catch` block.
3. Close the `catch` block and append a `finally` to centralize the cleanup, for example:
```js
api.uploadFilePromise(file, file.name).then(data => {
// existing success/error handling as above
}).catch(error => {
// existing error handling:
// uploadError.value = ...
// console.error(...)
}).finally(() => {
uploading.value = false
})
```
This ensures `uploading.value` is always reset, even if errors occur before your existing `then`/`catch` cleanup paths.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves the RichTextEditor image upload UX by replacing blocking alert() calls with an inline, translated error message and console error logging when uploads fail.
Changes:
- Added
uploadErrorstate to track and display image upload failures inline. - Replaced
alert()/console.logwith translated error messaging andconsole.errorlogging. - Added styling for the inline upload error message.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
app/eventyay/webapp/video/src/locales/en.json |
Adds a new translation key for the image upload failure message. |
app/eventyay/webapp/video/src/components/RichTextEditor.vue |
Tracks upload errors, shows an inline error message, and logs failures to the console. |
Member
|
@SxxAq Please Address the AI reviews |
Saksham-Sirohi
requested changes
Apr 26, 2026
Collaborator
Saksham-Sirohi
left a comment
Saksham-Sirohi
left a comment
There was a problem hiding this comment.
i dont think displaying this message inside the editor is a good idea
Contributor
Author
I put the error in the editor from the UX perspective as the error will be close to the action area. where would you prefer the error?
|
Contributor
We show errors and success messages mostly everywhere in the above, with a red worng sign along with a rectangular box and an error inside it, and the same goes for success messages. I suggested that same layout for success or error message would be good but lets hear from @Saksham-Sirohi ! |
Collaborator
|
hi @SxxAq you can use a popup dialog to inform within the textbox and allow edits further only after user has clicked ok or something |
Contributor
Author
|
@Saksham-Sirohi Implemented a popup error dialog in RichTextEditor and refactored it into a reusable ErrorDialog component, so we can reuse it for future error popups across the webapp. 
|
Comment on lines
+33
to
+39
| error-dialog( | ||
| v-if="showErrorDialog" | ||
| :title="$t('RichTextEditor:upload:error-title')" | ||
| :message="uploadErrorMessage" | ||
| :button-text="$t('RichTextEditor:upload:error-ok')" | ||
| @close="closeErrorDialog" | ||
| ) |
Comment on lines
+204
to
+207
| .upload-error | ||
| color: $clr-danger | ||
| font-size: 14px | ||
| margin-top: 8px |
Comment on lines
+13
to
+21
| <script setup> | ||
| import { ref } from 'vue' | ||
| import Prompt from 'components/Prompt' | ||
|
|
||
| const props = defineProps({ | ||
| title: { | ||
| type: String, | ||
| default: 'Error' | ||
| }, |
Saksham-Sirohi
requested changes
May 4, 2026
Collaborator
Saksham-Sirohi
left a comment
Saksham-Sirohi
left a comment
There was a problem hiding this comment.
please get rid of nook being displayed on left of the dialog box
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replaced blocking
alert()dialogs with proper error handling when image uploads fail in RichTextEditor. Errors now display inline in the toolbar with a user-friendly message.Changes:
Summary by Sourcery
Improve RichTextEditor image upload experience by surfacing non-blocking, localized error feedback and cleaning up editor event handling.
New Features:
Bug Fixes:
Enhancements: