mirror of
https://github.com/netbirdio/netbird.git
synced 2026-05-16 05:39:56 +00:00
[client/ui] Optimistic Connecting on profile switch, status row disabled
Three UX fixes for the tray's profile-switch flow:
* Optimistic Connecting paint when switching from Connected/Connecting.
Previously the daemon's Down step emitted Idle before the new
profile's Up emitted Connecting, leaving the tray flashing
"Disconnected" for the duration of the Down. switchProfile now sets a
flag and synthesizes a Connecting paint at click time; applyStatus
suppresses the transient Idle and the stale Connected updates that
arrive during the old profile's teardown, clearing the flag only when
the new profile's flow surfaces (Connecting, NeedsLogin, LoginFailed,
SessionExpired, DaemonUnavailable, or a 30s safety timeout).
* Disconnect during an in-flight switch now actually disconnects. The
switchCancel context cancels the ProfileSwitcher.SwitchActive
goroutine so its queued Up RPC never fires, and the
switchInProgress flag is cleared so the daemon's Idle push paints
through immediately. Without this, the user's Disconnect click was
followed seconds later by the switcher's Up bringing the new
profile back online.
* The first menu row is informational only. SetEnabled(false) is
re-applied to t.statusItem (initial build, applyStatus, and the
optimistic paint) and the openRoute("/login") OnClick handler is
dropped — every actionable transition flows through the
Connect/Disconnect entries below.
The switchProfile and applyStatus godocs carry the full
prevStatus → suppressed-events / final-state transition tables so
future readers don't have to rebuild the policy from the code.
This commit is contained in:
@@ -134,6 +134,15 @@ type Tray struct {
|
||||
activeUsername string
|
||||
switchCancel context.CancelFunc
|
||||
|
||||
// switchInProgress / switchInProgressUntil drive the optimistic
|
||||
// Connecting feedback when the user clicks a profile while we are
|
||||
// Connected or Connecting. See switchProfile and applyStatus —
|
||||
// applyStatus suppresses the transient daemon Idle that the Down
|
||||
// step emits, keeping the tray on Connecting until the new
|
||||
// profile's Up actually starts.
|
||||
switchInProgress bool
|
||||
switchInProgressUntil time.Time
|
||||
|
||||
// profileLoadMu serializes loadProfiles so the daemon-status-driven
|
||||
// refresh in applyStatus cannot race with the ApplicationStarted seed
|
||||
// or the post-switchProfile reload — both manipulate profileSubmenu and
|
||||
@@ -196,14 +205,13 @@ func (t *Tray) ShowWindow() {
|
||||
func (t *Tray) buildMenu() *application.Menu {
|
||||
menu := application.NewMenu()
|
||||
|
||||
// statusItem shows the daemon's current status. Kept enabled so the
|
||||
// label renders in the normal foreground colour (NSMenuItem greys
|
||||
// disabled items, which makes the live status read like inactive
|
||||
// chrome), but no OnClick handler is wired — clicking is a no-op.
|
||||
// The Connect entry below drives every actionable transition,
|
||||
// statusItem shows the daemon's current status. Disabled (and no
|
||||
// OnClick handler) so clicks are no-ops — the row is informational
|
||||
// only. The Connect entry below drives every actionable transition,
|
||||
// including the SSO re-auth flow for NeedsLogin/SessionExpired
|
||||
// (the daemon's Up RPC returns NeedsSSOLogin when applicable).
|
||||
t.statusItem = menu.Add(menuStatusDisconnected).
|
||||
SetEnabled(false).
|
||||
SetBitmap(iconMenuDotIdle)
|
||||
|
||||
menu.AddSeparator()
|
||||
@@ -309,8 +317,20 @@ func (t *Tray) handleConnect() {
|
||||
}()
|
||||
}
|
||||
|
||||
// handleDisconnect aborts any in-flight profile switch before sending
|
||||
// Down — otherwise the switcher's queued Up would re-establish the
|
||||
// connection right after the Disconnect, making the click look like a
|
||||
// no-op. Also clears the optimistic-Connecting guard so subsequent
|
||||
// daemon Idle pushes paint through to the tray immediately.
|
||||
func (t *Tray) handleDisconnect() {
|
||||
t.downItem.SetEnabled(false)
|
||||
t.mu.Lock()
|
||||
if t.switchCancel != nil {
|
||||
t.switchCancel()
|
||||
t.switchCancel = nil
|
||||
}
|
||||
t.switchInProgress = false
|
||||
t.mu.Unlock()
|
||||
go func() {
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
defer cancel()
|
||||
@@ -472,8 +492,50 @@ func (t *Tray) onUpdateProgress(ev *application.CustomEvent) {
|
||||
// (connected, hasUpdate, status label) changed — the daemon emits
|
||||
// rapid SubscribeStatus bursts during health probes that would
|
||||
// otherwise spam Shell_NotifyIcon and the log.
|
||||
//
|
||||
// Optimistic profile-switch suppression: when switchProfile fired the
|
||||
// optimistic Connecting paint (see its table), switchInProgress is set
|
||||
// and the daemon-emitted Connected (stale, from old profile teardown
|
||||
// where the daemon keeps pushing as peer count drops) and Idle (from
|
||||
// the Down step) events are swallowed here so the user doesn't see a
|
||||
// Disconnected/Connected blink before the new profile's flow begins.
|
||||
// Clears the flag and lets reality take over on:
|
||||
// - Connecting (the new profile's Up has officially started),
|
||||
// - NeedsLogin / LoginFailed / SessionExpired / DaemonUnavailable
|
||||
// (the daemon won't start Up — that's the new profile's terminal
|
||||
// state, surface it immediately), or
|
||||
// - the 30s safety timeout (Up wedged or never emitted anything).
|
||||
func (t *Tray) applyStatus(st services.Status) {
|
||||
t.mu.Lock()
|
||||
if t.switchInProgress {
|
||||
switch {
|
||||
case time.Now().After(t.switchInProgressUntil):
|
||||
// Safety net: the daemon never emitted a follow-up status
|
||||
// after Down (Up failed silently, RPC wedged, etc.). Drop
|
||||
// the optimistic guard and render whatever arrives next.
|
||||
t.switchInProgress = false
|
||||
case strings.EqualFold(st.Status, services.StatusConnecting),
|
||||
strings.EqualFold(st.Status, services.StatusNeedsLogin),
|
||||
strings.EqualFold(st.Status, services.StatusLoginFailed),
|
||||
strings.EqualFold(st.Status, services.StatusSessionExpired),
|
||||
strings.EqualFold(st.Status, services.StatusDaemonUnavailable):
|
||||
// New profile's flow has officially started: Up has begun
|
||||
// (Connecting) or the daemon refused to start Up at all
|
||||
// (NeedsLogin/LoginFailed/SessionExpired/DaemonUnavailable).
|
||||
// Clear the guard and let applyStatus paint normally — the
|
||||
// user should see the new state without further delay.
|
||||
t.switchInProgress = false
|
||||
default:
|
||||
// Connected (stale leftover from the old profile's teardown
|
||||
// — the daemon emits extra status updates while peer count
|
||||
// drops during cleanup) or Idle (transient between Down and
|
||||
// Up). Suppress so the tray stays on the optimistic
|
||||
// Connecting until the new profile's flow actually emits a
|
||||
// post-switch status.
|
||||
t.mu.Unlock()
|
||||
return
|
||||
}
|
||||
}
|
||||
connected := strings.EqualFold(st.Status, services.StatusConnected)
|
||||
iconChanged := connected != t.connected || st.Status != t.lastStatus
|
||||
// Detect the transition into SessionExpired: the daemon emits the
|
||||
@@ -500,10 +562,9 @@ func (t *Tray) applyStatus(st services.Status) {
|
||||
daemonUnavailable := strings.EqualFold(st.Status, services.StatusDaemonUnavailable)
|
||||
connecting := strings.EqualFold(st.Status, services.StatusConnecting)
|
||||
if t.statusItem != nil {
|
||||
// Label-only: enabled so the text isn't greyed out, but no
|
||||
// click handler is bound (see buildMenu). Swap the displayed
|
||||
// text so the user sees a familiar phrase instead of the raw
|
||||
// daemon enum.
|
||||
// Label-only: kept disabled (informational row). Swap the
|
||||
// displayed text so the user sees a familiar phrase instead
|
||||
// of the raw daemon enum.
|
||||
label := st.Status
|
||||
switch {
|
||||
case daemonUnavailable:
|
||||
@@ -512,7 +573,7 @@ func (t *Tray) applyStatus(st services.Status) {
|
||||
label = menuStatusDisconnected
|
||||
}
|
||||
t.statusItem.SetLabel(label)
|
||||
t.statusItem.SetEnabled(true)
|
||||
t.statusItem.SetEnabled(false)
|
||||
t.applyStatusIndicator(st.Status)
|
||||
}
|
||||
if t.upItem != nil {
|
||||
@@ -826,6 +887,39 @@ func (t *Tray) loadProfiles() {
|
||||
// switchProfile cancels any in-flight profile switch, then starts a new one.
|
||||
// Cancelling the previous context aborts its in-flight gRPC calls (Down/Up)
|
||||
// so rapid clicks always converge to the last selected profile.
|
||||
//
|
||||
// Optimistic-feedback transition table (driven by t.lastStatus at click time;
|
||||
// see applyStatus for the suppression rules):
|
||||
//
|
||||
// ┌──────────────────────┬──────────────────────┬──────────────────────────┬────────────────────┐
|
||||
// │ prevStatus │ ProfileSwitcher call │ Optimistic tray label │ Suppressed events │
|
||||
// │ │ │ shown immediately │ until new flow │
|
||||
// ├──────────────────────┼──────────────────────┼──────────────────────────┼────────────────────┤
|
||||
// │ Connected │ Switch + Down + Up │ Connecting (spinner) │ Connected, Idle │
|
||||
// │ Connecting │ Switch + Down + Up │ Connecting (unchanged) │ Connected, Idle │
|
||||
// │ Idle │ Switch only │ (no change) │ — │
|
||||
// │ NeedsLogin │ Switch + Down │ (no change) │ — │
|
||||
// │ LoginFailed │ Switch + Down │ (no change) │ — │
|
||||
// │ SessionExpired │ Switch + Down │ (no change) │ — │
|
||||
// │ DaemonUnavailable │ (RPC fails) │ (no change; toast error) │ — │
|
||||
// └──────────────────────┴──────────────────────┴──────────────────────────┴────────────────────┘
|
||||
//
|
||||
// Only Connected/Connecting need optimistic feedback because they're the
|
||||
// only prevStatus values where the switcher follows up with Up — i.e. the
|
||||
// only ones where the daemon emits stale Connected updates during Down
|
||||
// (peer count drops as the engine tears down) and then Idle, before the
|
||||
// new profile's Connecting/Connected. Both the stale Connected and the
|
||||
// transient Idle are suppressed by applyStatus until a status that
|
||||
// indicates the new flow has begun (Connecting, or any of the "Up won't
|
||||
// run" states: NeedsLogin / LoginFailed / SessionExpired /
|
||||
// DaemonUnavailable). The other branches either don't drive Down/Up at
|
||||
// all (Idle) or stop after Down (NeedsLogin / LoginFailed /
|
||||
// SessionExpired), and the resulting Idle is the correct terminal
|
||||
// state, so no suppression is needed.
|
||||
//
|
||||
// A 30s safety timeout clears the in-progress flag if the daemon never
|
||||
// emits a follow-up event — without it, a stuck Down/Up could leave the
|
||||
// tray frozen on Connecting forever.
|
||||
func (t *Tray) switchProfile(name string) {
|
||||
t.mu.Lock()
|
||||
if t.switchCancel != nil {
|
||||
@@ -833,8 +927,18 @@ func (t *Tray) switchProfile(name string) {
|
||||
}
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
t.switchCancel = cancel
|
||||
needsOptimistic := strings.EqualFold(t.lastStatus, services.StatusConnected) ||
|
||||
strings.EqualFold(t.lastStatus, services.StatusConnecting)
|
||||
if needsOptimistic {
|
||||
t.switchInProgress = true
|
||||
t.switchInProgressUntil = time.Now().Add(30 * time.Second)
|
||||
}
|
||||
t.mu.Unlock()
|
||||
|
||||
if needsOptimistic {
|
||||
t.applyOptimisticConnecting()
|
||||
}
|
||||
|
||||
go func() {
|
||||
username, err := t.svc.Profiles.Username()
|
||||
if err != nil {
|
||||
@@ -856,6 +960,35 @@ func (t *Tray) switchProfile(name string) {
|
||||
}()
|
||||
}
|
||||
|
||||
// applyOptimisticConnecting paints the tray as if the daemon had just
|
||||
// pushed status=Connecting, without waiting for the daemon to actually
|
||||
// get there. Touches the status cache (so the iconChanged comparison in
|
||||
// the next real applyStatus call lines up) and the same item bits that
|
||||
// applyStatus would set for Connecting. Doesn't touch exitNodes,
|
||||
// daemonVersion, or sessionExpired — none of those should change as a
|
||||
// side effect of a profile switch.
|
||||
func (t *Tray) applyOptimisticConnecting() {
|
||||
t.mu.Lock()
|
||||
t.connected = false
|
||||
t.lastStatus = services.StatusConnecting
|
||||
t.mu.Unlock()
|
||||
|
||||
t.applyIcon()
|
||||
if t.statusItem != nil {
|
||||
t.statusItem.SetLabel(services.StatusConnecting)
|
||||
t.statusItem.SetEnabled(false)
|
||||
t.applyStatusIndicator(services.StatusConnecting)
|
||||
}
|
||||
if t.upItem != nil {
|
||||
t.upItem.SetHidden(true)
|
||||
t.upItem.SetEnabled(false)
|
||||
}
|
||||
if t.downItem != nil {
|
||||
t.downItem.SetHidden(false)
|
||||
t.downItem.SetEnabled(true)
|
||||
}
|
||||
}
|
||||
|
||||
// notify wraps the Wails notification service with the tray's standard
|
||||
// id-prefix scheme and swallows errors (notifications are best-effort).
|
||||
func (t *Tray) notify(title, body, id string) {
|
||||
|
||||
Reference in New Issue
Block a user