Repository navigation
Keep iOS Mac avatar color slots stable during Mac switches #7858
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
Changes from all commits
d924896
c35d9df
a246305
ba2d43c
bb8cbd5
24ca50b
173ae95
83b468a
5a49608
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -243,7 +243,7 @@ public final class MobileShellComposite: MobileTerminalOutputSinking { | |
| var workspacesByMac: [String: MacWorkspaceState] = [:] { | ||
| didSet { recomputeDerivedWorkspaceState() } | ||
| } | ||
| private let workspaceAggregation = MobileWorkspaceAggregation() | ||
| let workspaceAggregation = MobileWorkspaceAggregation(); var stableMacColorSlots: [String: Int] = [:] // see MobileShellComposite+MacSwitchState.swift | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| /// The flat aggregated workspace list the UI renders. A materialized | ||
|
azooz2003-bit marked this conversation as resolved.
|
||
| /// derivation of ``workspacesByMac``: only ``recomputeDerivedWorkspaceState`` | ||
| /// assigns it, so it is never independently mutated. | ||
|
|
@@ -268,7 +268,7 @@ public final class MobileShellComposite: MobileTerminalOutputSinking { | |
| /// workspace avatars use), so the Computers screen can color each Mac's row to | ||
| /// match its workspaces. Keyed by `macDeviceID`. | ||
| public var machineColorIndex: [String: Int] { | ||
| workspaceAggregation.machineColorIndex(statesByMac: workspacesByMac) | ||
| stableMacColorSlots | ||
|
cursor[bot] marked this conversation as resolved.
azooz2003-bit marked this conversation as resolved.
|
||
| } | ||
|
|
||
| public var macConnectionStatuses: [String: MobileMacConnectionStatus] { | ||
|
|
@@ -1188,7 +1188,7 @@ public final class MobileShellComposite: MobileTerminalOutputSinking { | |
| workspaces: PreviewMobileHost.workspaces, | ||
| groups: [] | ||
| )] | ||
| selectedWorkspaceID = workspaces.first?.id | ||
| resetStableMacColorSlotsForSignOut(); selectedWorkspaceID = workspaces.first?.id | ||
| selectedTerminalID = workspaces.first?.terminals.first?.id | ||
| // Selection resets above are done; allow draft saving again so a | ||
| // subsequent sign-in restores drafts normally. | ||
|
|
@@ -1218,7 +1218,7 @@ public final class MobileShellComposite: MobileTerminalOutputSinking { | |
| // on the next foreground / Computers `.task` / pull-to-refresh. | ||
| teardownSecondaryMacSubscriptions() | ||
| let foregroundKey = foregroundMacKey | ||
| workspacesByMac = workspacesByMac.filter { $0.key == foregroundKey } | ||
| workspacesByMac = workspacesByMac.filter { $0.key == foregroundKey }; pruneStableMacColorSlots(keepingForegroundKey: foregroundKey) | ||
| // Restore memo: invalidate so the next read re-restores for the new | ||
| // (account, team) scope, and a suspended old-team restore can't resume. | ||
| // Invalidate the shared boundary synchronously first; actor cleanup is | ||
|
|
@@ -3565,7 +3565,7 @@ public final class MobileShellComposite: MobileTerminalOutputSinking { | |
| /// source of truth. Pure and cheap; the only place those two are assigned, | ||
| /// called on any ``workspacesByMac`` or foreground change. | ||
| private func recomputeDerivedWorkspaceState() { | ||
| let previousSelection = selectedWorkspaceID.flatMap { id in | ||
| updateStableMacColorSlots(); let previousSelection = selectedWorkspaceID.flatMap { id in | ||
| workspaces.first { $0.id == id } | ||
| } | ||
| let foregroundKey: String? | ||
|
|
@@ -3577,7 +3577,7 @@ public final class MobileShellComposite: MobileTerminalOutputSinking { | |
| foregroundKey = nil | ||
| } | ||
| var derived = workspaceAggregation.derivedWorkspaces( | ||
| statesByMac: workspacesByMac, foregroundMacDeviceID: foregroundKey) | ||
| statesByMac: workspacesByMac, foregroundMacDeviceID: foregroundKey, machineColorIndex: stableMacColorSlots) | ||
| // Stamp per-Mac user color/icon overrides from pairedMacs so every | ||
| // workspace avatar matches its computer's customization (same place the | ||
| // aggregation already assigned the automatic color index). | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| import CMUXMobileCore | ||
| import CmuxMobilePairedMac | ||
| import CmuxMobileRPC | ||
| import CmuxMobileShellModel | ||
| import Foundation | ||
| import Testing | ||
| @testable import CmuxMobileShell | ||
|
|
||
| /// Regression tests for `stableMacColorSlots` resets on the account/team | ||
| /// boundaries. Split from ``MobileShellCompositePreviewTests`` to keep that | ||
| /// file under the Swift file length budget. | ||
| @MainActor | ||
| @Suite struct MobileShellCompositeColorSlotResetTests { | ||
| @Test func signOutClearsStableMacColorSlots() { | ||
| let store = MobileShellComposite.preview() | ||
| store.signIn() | ||
| store.pairingCode = "debug" | ||
| store.connectPreviewHost() | ||
|
|
||
| // Seed a real (non-anonymous) Mac's color slot so sign-out has something | ||
| // to leak: without an explicit reset, `stableMacColorSlots` is | ||
| // additive-only, so the next account's slots would start numbering past | ||
| // this account's stale entry instead of at zero. | ||
| store.workspacesByMac["mac-previous-account"] = MacWorkspaceState( | ||
| macDeviceID: "mac-previous-account", | ||
| workspaces: [] | ||
| ) | ||
| #expect(store.machineColorIndex["mac-previous-account"] != nil) | ||
|
|
||
| store.signOut() | ||
|
|
||
| // The previous account's Mac→color assignment must not survive | ||
| // sign-out into the next account's session. | ||
| #expect(store.machineColorIndex.isEmpty) | ||
| } | ||
|
|
||
| @Test func teamSwitchDropsBackgroundMacColorSlotsButKeepsForeground() { | ||
| let store = MobileShellComposite.preview() | ||
| store.signIn() | ||
| store.pairingCode = "debug" | ||
| store.connectPreviewHost() | ||
| // A real (non-anonymous) foreground Mac, plus a background Mac that | ||
| // belongs to the team being switched away from. | ||
| store.foregroundMacDeviceID = "mac-foreground" | ||
| store.workspacesByMac["mac-foreground"] = MacWorkspaceState( | ||
| macDeviceID: "mac-foreground", | ||
| workspaces: [] | ||
| ) | ||
| store.workspacesByMac["mac-old-team"] = MacWorkspaceState( | ||
| macDeviceID: "mac-old-team", | ||
| workspaces: [] | ||
| ) | ||
| let foregroundIndexBefore = store.machineColorIndex["mac-foreground"] | ||
| #expect(foregroundIndexBefore != nil) | ||
| #expect(store.machineColorIndex["mac-old-team"] != nil) | ||
|
|
||
| store.currentTeamDidChange() | ||
|
|
||
| // The old team's background Mac must not survive the team switch | ||
| // (additive-only slots would otherwise leak it into the new team's | ||
| // numbering forever), while the foreground Mac's own slot is left | ||
| // intact so its color doesn't flash/change across the switch. | ||
| #expect(store.machineColorIndex["mac-old-team"] == nil) | ||
| #expect(store.machineColorIndex["mac-foreground"] == foregroundIndexBefore) | ||
| } | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.