Skip to content

chore: minor ui/ux changes #186

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

Merged
merged 2 commits into from
Jun 18, 2025
Merged

chore: minor ui/ux changes #186

merged 2 commits into from
Jun 18, 2025

Conversation

ethanndickson
Copy link
Member

@ethanndickson ethanndickson commented Jun 18, 2025

These changes were in response to feedback:

  • Adds tooltips on hover to the copy DNS button, and the open in browser button on the main tray menu.
  • Includes the download URL in the error message if the client receives an unexpected HTTP code when downloading.
  • Makes the file sync table controls a lil bigger (24px -> 28px):
    • Before:
    • After:

Copy link
Member Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@ethanndickson ethanndickson marked this pull request as ready for review June 18, 2025 01:57
Copy link

@Copilot Copilot AI left a 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 enhances UI/UX tooltips, improves error diagnostics for downloads, and standardizes sizing for file sync table controls.

  • Adds hover tooltips on the copy DNS and open-in-browser tray menu buttons.
  • Includes the download URL in DownloadError.unexpectedStatusCode messages.
  • Increases footer icon size from 24px to 30px via a new FooterIcon component and Theme.Size.tableFooterIconSize.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
VPNLib/Download.swift Updated DownloadError.unexpectedStatusCode to include URL and adjusted error resume call.
Views/VPN/VPNMenuItem.swift Added .help(...) modifiers to show tooltips on hover for copy and open buttons.
Views/FileSync/FileSyncConfig.swift Replaced fixed 24px frames with FooterIcon, increased size to 30px, and updated padding.
Theme.swift Added Theme.Size.tableFooterIconSize to centralize the new footer icon dimension.
Comments suppressed due to low confidence (4)

Coder-Desktop/VPNLib/Download.swift:149

  • Consider adding a unit test that triggers an unexpected status code and asserts that the error description includes both the HTTP code and the download URL.
    case unexpectedStatusCode(Int, url: String)

Coder-Desktop/VPNLib/Download.swift:149

  • [nitpick] It may be safer to store the URL as a URL type instead of String in the error, preserving type information and avoiding downstream parsing issues.
    case unexpectedStatusCode(Int, url: String)

Coder-Desktop/Coder-Desktop/Views/FileSync/FileSyncConfig.swift:149

  • The play/pause toggle logic appears inverted: when isResumable is true (i.e., the session is paused), you should show a play icon with tooltip "Resume", and vice versa for pausing.
                                FooterIcon(systemName: "play")

Coder-Desktop/Coder-Desktop/Views/FileSync/FileSyncConfig.swift:206

  • [nitpick] If FooterIcon is intended for reuse across multiple views, consider moving it into a shared UI components file or adding a doc comment to clarify its purpose.
struct FooterIcon: View {

Copy link
Member Author

ethanndickson commented Jun 18, 2025

Merge activity

  • Jun 18, 8:09 AM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Jun 18, 8:09 AM UTC: @ethanndickson merged this pull request with Graphite.

@ethanndickson ethanndickson merged commit 99d4e4d into main Jun 18, 2025
4 checks passed
@ethanndickson ethanndickson deleted the ethan/minor-ux branch June 18, 2025 08:09
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.

2 participants