feat: added download notif on both web and tauri versions#977
Open
Audatic07 wants to merge 5 commits intoCircuitVerse:mainfrom
Open
feat: added download notif on both web and tauri versions#977Audatic07 wants to merge 5 commits intoCircuitVerse:mainfrom
Audatic07 wants to merge 5 commits intoCircuitVerse:mainfrom
Conversation
✅ Deploy Preview for circuitverse ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
WalkthroughThe export flow was changed to be asynchronous and provide user feedback. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/DialogBox/ExportProject.vue (1)
69-79:⚠️ Potential issue | 🟠 Major
exportAsFilehas no error handling — failures leave the user with no feedback and the dialog stuck open.Both
generateSaveData(line 71) anddownloadFile(line 76) areawaited inside an async handler with notry/catch. If either rejects (e.g., Tauri FS permission denied, disk full, serialization error), the exception propagates silently out of the Vue event handler: the successshowMessageis never called, the dialog is never closed, and the user has no indication of what went wrong.🛡️ Proposed fix
const exportAsFile = async () => { - let fileName = escapeHtml(fileNameInput.value) || 'untitled' - const circuitData = await generateSaveData( - projectStore.getProjectName, - false - ) - fileName = `${fileName.replace(/[^a-z0-9]/gi, '_')}.cv` - const downloadDir = await downloadFile(fileName, circuitData) - showMessage(`"${fileName}" saved successfully to ${downloadDir}`) - SimulatorState.dialogBox.export_project_dialog = false + try { + let fileName = escapeHtml(fileNameInput.value) || 'untitled' + const circuitData = await generateSaveData( + projectStore.getProjectName, + false + ) + fileName = `${fileName.replace(/[^a-z0-9]/gi, '_')}.cv` + const downloadDir = await downloadFile(fileName, circuitData) + showMessage(`"${fileName}" saved successfully to ${downloadDir}`) + SimulatorState.dialogBox.export_project_dialog = false + } catch (error) { + showError(`Failed to save file: ${error}`) + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/DialogBox/ExportProject.vue` around lines 69 - 79, Wrap the body of exportAsFile in a try/catch: call generateSaveData and downloadFile inside the try; on success keep the existing showMessage and set SimulatorState.dialogBox.export_project_dialog = false; in catch, call showMessage (or a dedicated error logger) with a descriptive error containing the caught error message so the user sees what failed, and ensure the dialog is closed or re-enabled (set SimulatorState.dialogBox.export_project_dialog = false or leave it open based on UX decision) and optionally rethrow or return to stop further execution. Reference: exportAsFile, generateSaveData, downloadFile, showMessage, SimulatorState.dialogBox.export_project_dialog.
🧹 Nitpick comments (1)
src/components/DialogBox/ExportProject.vue (1)
44-45: Consider consolidating the two imports from the same module.
showMessageandescapeHtmlare both from'#/simulator/src/utils'but imported on separate lines.♻️ Proposed consolidation
-import { downloadFile, showMessage } from '#/simulator/src/utils' -import { escapeHtml } from '#/simulator/src/utils' +import { downloadFile, showMessage, escapeHtml } from '#/simulator/src/utils'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/DialogBox/ExportProject.vue` around lines 44 - 45, In ExportProject.vue consolidate the duplicate imports from '#/simulator/src/utils' into a single import statement; specifically import downloadFile, showMessage and escapeHtml together (remove the separate import lines) so the file uses one combined import for those symbols.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/DialogBox/ExportProject.vue`:
- Around line 76-77: The notification is being suppressed by showMessage's
dedupe (prevShowMessage) causing repeated exports with the same message to be
silent; in ExportProject.vue where you call downloadFile and then
showMessage(`"${fileName}" saved successfully to ${downloadDir}`), change the
call to ensure uniqueness — either append a timestamp/nonce to the message
payload (e.g., include Date.now() or a short UUID) or modify showMessage usage
to call a variant that resets prevShowMessage after displaying (update
utils.ts's showMessage or add showMessageImmediate) so repeated identical
exports (same fileName) still produce a visible notification; reference
downloadFile, showMessage and prevShowMessage when making the change.
---
Outside diff comments:
In `@src/components/DialogBox/ExportProject.vue`:
- Around line 69-79: Wrap the body of exportAsFile in a try/catch: call
generateSaveData and downloadFile inside the try; on success keep the existing
showMessage and set SimulatorState.dialogBox.export_project_dialog = false; in
catch, call showMessage (or a dedicated error logger) with a descriptive error
containing the caught error message so the user sees what failed, and ensure the
dialog is closed or re-enabled (set
SimulatorState.dialogBox.export_project_dialog = false or leave it open based on
UX decision) and optionally rethrow or return to stop further execution.
Reference: exportAsFile, generateSaveData, downloadFile, showMessage,
SimulatorState.dialogBox.export_project_dialog.
---
Duplicate comments:
In `@v1/src/components/DialogBox/ExportProject.vue`:
- Around line 44-45: Consolidate the two identical module imports from
'#/simulator/src/utils' in ExportProject.vue into a single import statement:
merge the named imports downloadFile, showMessage and escapeHtml so the file
only imports from '#/simulator/src/utils' once (referencing the existing import
occurrences for downloadFile, showMessage and escapeHtml to locate the
duplicate).
- Around line 69-79: Wrap the exportAsFile async function body in a try/catch:
inside try keep the existing steps (escapeHtml(fileNameInput.value), await
generateSaveData(...), sanitize fileName, await downloadFile(...),
showMessage(...), and set SimulatorState.dialogBox.export_project_dialog =
false); in catch log the error (console.error(err)) and surface a user-facing
message via showMessage including the error details (e.g. `Failed to save
"${fileName}": ${err.message}`) so failures are visible and avoid closing the
export dialog on failure. Reference symbols: exportAsFile, generateSaveData,
downloadFile, showMessage, SimulatorState.dialogBox.export_project_dialog,
fileNameInput.
- Around line 76-77: The call to showMessage after downloadFile is producing
duplicate notifications; update the export flow in ExportProject.vue so we only
emit a single notification: compute the message string using fileName and
downloadDir (from downloadFile(fileName, circuitData)), then guard the
showMessage call by checking a deduplication condition (e.g., compare against a
stored lastMessage or use a showMessageOnce helper) before invoking
showMessage(message). Ensure you reference the existing symbols downloadFile,
fileName, circuitData, downloadDir and showMessage so the check replaces the
direct showMessage(...) call.
In `@v1/src/simulator/src/utils.ts`:
- Around line 119-145: No code changes required — the Tauri v2 API usage in
downloadFile and downloadFileWeb is correct and matches the accepted pattern;
leave the functions downloadFile, downloadFileWeb and the desktop variant
downloadFileDesktop (and the isTauri check) as implemented and proceed with
merging this approved change.
- Around line 148-161: The downloadFileDesktop function writes an absolute path
in the Downloads directory (downloadFileDesktop, downloadDir, writeTextFile)
which requires an explicit Tauri filesystem permission; either update your Tauri
config/allowlist to permit writing to the Downloads path (add the
fs:allow-download-write or include the downloadsDirectory path in the fs
allowlist/scope) or change downloadFileDesktop to avoid absolute writes (e.g.,
use a user save dialog or write to an app-scoped directory returned by Tauri
that is already allowed). Make the change where downloadFileDesktop calls
downloadDir() and writeTextFile(...) so the runtime has the required fs
permission or the function uses a permitted/interactive save flow.
---
Nitpick comments:
In `@src/components/DialogBox/ExportProject.vue`:
- Around line 44-45: In ExportProject.vue consolidate the duplicate imports from
'#/simulator/src/utils' into a single import statement; specifically import
downloadFile, showMessage and escapeHtml together (remove the separate import
lines) so the file uses one combined import for those symbols.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/simulator/src/utils.ts (1)
125-134:⚠️ Potential issue | 🟡 MinorTypeScript type mismatch:
JSONindownloadFile's parameter is incompatible withdownloadFileWeb's narrower signature.
downloadFileacceptstext: string | number | boolean | JSON, butdownloadFileWebonly acceptstext: string | number | boolean. CallingdownloadFileWeb(filename, text)at line 132 passes a type that includesJSON, which is not assignable todownloadFileWeb's parameter — this is a TypeScript compile error.Note:
JSONin TypeScript refers to the globalJSONnamespace object (the one withparse/stringify), not a general "JSON-serializable value". If the intent is to allow arbitrary serializable data, a more appropriate type would bestring | number | boolean | objector a customJsonValuealias.🛠️ Proposed fix — align types across all three functions
export async function downloadFile( filename: string, - text: string | number | boolean | JSON, + text: string | number | boolean, ): Promise<string> { export function downloadFileWeb(filename: string, text: string | number | boolean): string { export async function downloadFileDesktop( filename: string, - text: string | number | boolean | JSON, + text: string | number | boolean, ): Promise<string> {If callers genuinely need to pass raw objects, use
text.toString()/JSON.stringify(text)at the call-site, or acceptunknownand narrow before writing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/simulator/src/utils.ts` around lines 125 - 134, The function downloadFile accepts text: string | number | boolean | JSON which is incorrect because JSON is the global namespace and makes the call to downloadFileWeb (which expects string|number|boolean) a type error; fix by aligning types—either change downloadFile's signature to string | number | boolean | object or a proper JsonValue alias (e.g., JsonValue = string|number|boolean|null|JsonObject|JsonArray) and then serialize objects before delegating, or narrow/serialize the incoming text in downloadFile (using JSON.stringify for objects) before calling downloadFileWeb or downloadFileDesktop; update the signatures of downloadFile, downloadFileWeb, and downloadFileDesktop (or add a local narrowing step in downloadFile) to use the same compatible type so the call downloadFileWeb(filename, text) type-checks.src/components/DialogBox/ExportProject.vue (1)
69-79:⚠️ Potential issue | 🟠 MajorNo error handling — Tauri file-write failures are silently swallowed.
If
downloadFilerejects (e.g.,writeTextFilefails due to permissions, disk full, or path error on desktop), the function throws pastshowMessageImmediateanddialog = false. Vue doesn't surface unhandled async rejections in click handlers as visible UI, so the user sees nothing and has no way to know the save failed. Since the Tauri notification is the only feedback mechanism on desktop (no browser download indicator), this leaves users completely in the dark on failure.🛡️ Proposed fix — wrap download in try/catch
const exportAsFile = async () => { let fileName = escapeHtml(fileNameInput.value) || 'untitled' const circuitData = await generateSaveData( projectStore.getProjectName, false ) fileName = `${fileName.replace(/[^a-z0-9]/gi, '_')}.cv` - const downloadDir = await downloadFile(fileName, circuitData) - showMessageImmediate(`"${fileName}" saved successfully to ${downloadDir}`) - SimulatorState.dialogBox.export_project_dialog = false + try { + const downloadDir = await downloadFile(fileName, circuitData) + showMessageImmediate(`"${fileName}" saved successfully to ${downloadDir}`) + SimulatorState.dialogBox.export_project_dialog = false + } catch (err) { + showMessageImmediate(`Failed to save "${fileName}": ${err instanceof Error ? err.message : String(err)}`) + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/DialogBox/ExportProject.vue` around lines 69 - 79, The exportAsFile flow lacks error handling around the Tauri file write; wrap the downloadFile call (and subsequent success actions: showMessageImmediate and setting SimulatorState.dialogBox.export_project_dialog = false) in a try/catch, calling downloadFile(fileName, circuitData) inside the try; on success keep current behavior, but in catch call showMessageImmediate with a clear failure message including the caught error, and do not close the dialog so the user can retry; reference exportAsFile, fileNameInput, generateSaveData, downloadFile, showMessageImmediate, and SimulatorState.dialogBox.export_project_dialog when making the change.
🧹 Nitpick comments (1)
src/components/DialogBox/ExportProject.vue (1)
44-45: Consolidate the two imports from the same module.♻️ Proposed refactor
-import { downloadFile, showMessageImmediate } from '#/simulator/src/utils' -import { escapeHtml } from '#/simulator/src/utils' +import { downloadFile, showMessageImmediate, escapeHtml } from '#/simulator/src/utils'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/DialogBox/ExportProject.vue` around lines 44 - 45, Two separate import statements import from '#/simulator/src/utils' (downloadFile, showMessageImmediate and escapeHtml) so consolidate them into a single import line; replace the two imports with one import that lists all named exports (downloadFile, showMessageImmediate, escapeHtml) so there are no duplicate imports in ExportProject.vue.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/components/DialogBox/ExportProject.vue`:
- Around line 69-79: The exportAsFile flow lacks error handling around the Tauri
file write; wrap the downloadFile call (and subsequent success actions:
showMessageImmediate and setting SimulatorState.dialogBox.export_project_dialog
= false) in a try/catch, calling downloadFile(fileName, circuitData) inside the
try; on success keep current behavior, but in catch call showMessageImmediate
with a clear failure message including the caught error, and do not close the
dialog so the user can retry; reference exportAsFile, fileNameInput,
generateSaveData, downloadFile, showMessageImmediate, and
SimulatorState.dialogBox.export_project_dialog when making the change.
In `@src/simulator/src/utils.ts`:
- Around line 125-134: The function downloadFile accepts text: string | number |
boolean | JSON which is incorrect because JSON is the global namespace and makes
the call to downloadFileWeb (which expects string|number|boolean) a type error;
fix by aligning types—either change downloadFile's signature to string | number
| boolean | object or a proper JsonValue alias (e.g., JsonValue =
string|number|boolean|null|JsonObject|JsonArray) and then serialize objects
before delegating, or narrow/serialize the incoming text in downloadFile (using
JSON.stringify for objects) before calling downloadFileWeb or
downloadFileDesktop; update the signatures of downloadFile, downloadFileWeb, and
downloadFileDesktop (or add a local narrowing step in downloadFile) to use the
same compatible type so the call downloadFileWeb(filename, text) type-checks.
---
Nitpick comments:
In `@src/components/DialogBox/ExportProject.vue`:
- Around line 44-45: Two separate import statements import from
'#/simulator/src/utils' (downloadFile, showMessageImmediate and escapeHtml) so
consolidate them into a single import line; replace the two imports with one
import that lists all named exports (downloadFile, showMessageImmediate,
escapeHtml) so there are no duplicate imports in ExportProject.vue.
Contributor
Author
|
@Nihal4777 CI is green, and Coderabbit suggestions are outside the diff. Please review :) |
Contributor
|
Tested locally and working fine. |
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
2 participants
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.
Fixes #975
Describe the changes you have made in this PR -
Added notification pop-up when using export as file, in both tauri and web versions. The tauri version lists the whole file path of the download.
Screenshots of the UI changes (If any) -
Code Understanding and AI Usage
Did you use AI assistance (ChatGPT, Claude, Copilot, etc.) to write any part of this code?
If you used AI assistance:
Explain your implementation approach:
Checklist before requesting a review
Note: Please check Allow edits from maintainers if you would like us to assist in the PR.
Summary by CodeRabbit