coder/coder-desktop-macosPublic

NotificationsYou must be signed in to change notification settings
Fork3
Star16

feat: make workspace apps collapsible#143

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

ethanndickson merged 4 commits intomainfromethan/collapsible-apps

May 1, 2025

Merged

feat: make workspace apps collapsible#143

ethanndickson merged 4 commits intomainfromethan/collapsible-apps

May 1, 2025

Conversation

Copy link

Member

ethanndickson commentedApr 29, 2025•
edited
Loading

collapsibleapps.mov

This was referencedApr 29, 2025

feat: add workspace apps#136

Merged

feat: add progress messages when creating sync sessions#139

Merged

Copy link

MemberAuthor

ethanndickson commentedApr 29, 2025•
edited
Loading

This stack of pull requests is managed byGraphite. Learn more aboutstacking.

ethanndickson requested review fromdeansheather andCopilot

April 30, 2025 02:35

ethanndickson marked this pull request as ready for review

April 30, 2025 02:35

CopilotAI reviewed

Apr 30, 2025

View reviewed changes

Copy link

CopilotAI 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 introduces collapsible workspace apps in the VPN menu along with minor test updates and UI refinements. Key changes include updating view inspector tests, replacing deprecated hover handlers, and adding new UI components (collapsible view, icons, and animated chevron) for a better user experience.

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
Coder-Desktop/Coder-DesktopTests/AgentsTests.swift	Updated tests to use text-based view lookup instead of link-based lookup.
Coder-Desktop/Coder-Desktop/Views/VPN/WorkspaceAppIcon.swift	Replaced onHoverWithPointingHand with the standard onHover handler.
Coder-Desktop/Coder-Desktop/Views/VPN/VPNMenuItem.swift	Added a helper function to compute the primary host and refactored MenuItemView for collapsible functionality.
Coder-Desktop/Coder-Desktop/Views/VPN/Agents.swift	Updated to pass the expandedItem binding to MenuItemView for managing collapsible state.
Coder-Desktop/Coder-Desktop/VPN/MenuState.swift	Minor spelling fix in a comment.
Coder-Desktop/Coder-Desktop/Theme.swift	Introduced a new animation duration for collapsible transitions.

Coder-Desktop/Coder-Desktop/VPN/MenuState.swift OutdatedShow resolvedHide resolved

deansheather reviewed

Apr 30, 2025

View reviewed changes

Coder-Desktop/Coder-Desktop/Views/VPN/Agents.swift

		@@ -4,6 +4,7 @@ struct Agents<VPN: VPNService>: View {
		@EnvironmentObject var vpn: VPN
		@EnvironmentObject var state: AppState
		@State private var viewAll = false
		@State private var expandedItem: VPNMenuItem.ID?

Copy link

Member

deansheatherApr 30, 2025

Choose a reason for hiding this comment

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

It seems odd to me having individual collapsible chevrons for each item, but only one item can be expanded at a time. I think we should just allow all items to be expanded independently

Copy link

MemberAuthor

ethanndicksonApr 30, 2025•
edited
Loading

Choose a reason for hiding this comment

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

This is just a pattern I've seen in the past that I liked. I'm pretty sure designers do it to stop the user from cluttering up the UI, or making the page too long, which is why I've done it here too. A user doesn't really need to be able to view multiple at once, and having to go close them all yourself to tidy it up is annoying.

Copy link

Member

deansheather commentedApr 30, 2025

Also the first workspace in the list should have the apps visible by default. Most people only use one workspace so it'll make the experience better

ethanndickson force-pushed theethan/collapsible-apps branch froma8affe1 toaeca6dfCompare

April 30, 2025 07:08

Copy link

MemberAuthor

ethanndickson commentedApr 30, 2025•
edited
Loading

Here's the algorithm for how and when to expand the first item, wherevisibleItems is sorted, andhasToggledExpansion is also updated by any user interaction.

.onChange(of: visibleItems) {    // If no workspaces are online, we should expand the first one to come online    if visibleItems.filter({ $0.status != .off }).isEmpty {        hasToggledExpansion = false        return    }    if hasToggledExpansion {        return    }    expandedItem = visibleItems.first?.id    hasToggledExpansion = true}

Copy link

Member

matifali commentedApr 30, 2025

Also the first workspace in the list should have the apps visible by default. Most people only use one workspace so it'll make the experience better

What would it take to put the favorite workspaces on top?

Copy link

MemberAuthor

ethanndickson commentedApr 30, 2025

What would it take to put the favorite workspaces on top?

We actually get thefavorite bool when we fetch the workspace apps, but, with the way I've set up the UI code, getting it to reorder after that fetching is done would require juggling some code around. It'd be a separate PR for sure.

ethanndickson requested a review fromdeansheather

April 30, 2025 09:45

deansheather approved these changes

May 1, 2025

View reviewed changes

Copy link

MemberAuthor

ethanndickson commentedMay 1, 2025•
edited
Loading

Merge activity

Apr 30, 10:18 PM EDT: A user started a stack merge that includes this pull request viaGraphite.
Apr 30, 10:22 PM EDT:Graphite rebased this pull request as part of a merge.
Apr 30, 10:22 PM EDT:@ethanndickson merged this pull request withGraphite.