-
Notifications
You must be signed in to change notification settings - Fork 1
UI improvements + workflow updates #33
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR improves the UI by adding ETA and connection mode displays, enhances logging in the UI, and updates versioning and workflows to support protocol changes. It also refactors some deprecated code and adjusts the release workflows accordingly.
- Added ETA and transfer mode signals to the Send and Receive pages.
- Updated logging messages and connection type emissions.
- Updated version constants and workflow triggers.
Reviewed Changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| qs-gui/src/Pages/Send.tsx | Added transfer mode signal and enhanced logging for connection events. |
| qs-gui/src/Pages/Receive.tsx | Added transfer mode signal; similar connection type parsing logic as in Send.tsx. |
| qs-gui/src/Pages/Main.tsx | Replaced createEffect with onCleanup for file drop listener removal. |
| qs-gui/src/Pages/AcceptFiles.tsx | Added currentSpeedBps prop for file transfer ETA display. |
| qs-gui/src/Components/FileTransferCard.tsx | Added remaining time calculation using currentSpeedBps. |
| qs-gui/src-tauri/src/lib.rs | Updated emission of connection type information. |
| qs-core/* | Replaced QS_VERSION with QS_PROTO_VERSION and removed unused version helpers. |
| qs-cli/* | Updated version constants and added command aliases. |
| Cargo.toml and .github workflows | Updated dependency versions and revised release workflow triggers. |
Comments suppressed due to low confidence (1)
.github/workflows/release-ui.yml:4
- [nitpick] The trigger for the UI release workflow has changed from a push on 'master' to a release published event; please verify that this change aligns with the desired deployment process for the UI.
release:
| (connType: Event<string>) => { | ||
| console.log("here") | ||
| setStore("currentState", SendState.WaitingForFileAccept) | ||
| if (connType.payload.startsWith("direct")) { |
Copilot
AI
May 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The connection type parsing logic (checking for 'direct', 'mixed', 'relay') is duplicated here and in Receive.tsx; consider extracting it to a shared helper to improve maintainability.
| if (connType.payload.startsWith("direct")) { | ||
| setTransferMode("direct") | ||
| } else if (connType.payload.startsWith("mixed")) { | ||
| setTransferMode("mixed") | ||
| } else if (connType.payload.startsWith("relay")) { | ||
| setTransferMode("relay") | ||
| } |
Copilot
AI
May 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] This duplicate connection type parsing logic appears similar to that in Send.tsx; extracting it to a common helper function would reduce code repetition.
| if (connType.payload.startsWith("direct")) { | |
| setTransferMode("direct") | |
| } else if (connType.payload.startsWith("mixed")) { | |
| setTransferMode("mixed") | |
| } else if (connType.payload.startsWith("relay")) { | |
| setTransferMode("relay") | |
| } | |
| const parsedType = parseConnectionType(connType.payload) | |
| setTransferMode(parsedType) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you create this parseConnectionType in utils.ts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot won't reply to this thread.
gg
| }) | ||
| setStore("currentState", null) | ||
| }} | ||
| transferMode={transferMode()!} |
Copilot
AI
May 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The use of a non-null assertion for transferMode here differs from Send.tsx where it is passed without force unwrapping; consider handling nullable transferMode consistently in both components.
closes #32
closes #27