diff --git a/AGENTS.md b/AGENTS.md index f4875162..dea3d113 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -200,7 +200,7 @@ scrollbars, date formatting, case folding, or ID formatting: it already exists. | Selected message or reply in the focused pane (channel, TS, text, author, panel) | `App.selectedMessageContext()` (`internal/ui/app.go`) | | Switch the active workspace from the UI | `App.switchWorkspace(teamID)` (`internal/ui/app.go`). Always use it: it saves a theme a cycle left pending, so a switch back to that workspace reads the new theme | | Save a per-workspace theme | `settings.SaveTheme(teamID, name, scope)` with the workspace on screen (`a.activeTeamID`), never the backend's active one: a switch moves that first | -| Set the user's own presence or DND from the UI (optimistic cache + status bar, then the service) | `(*App).applyPresenceAction(action, snoozeMinutes)` (`internal/ui/presence_action.go`); the presence menu, custom snooze and the `/away` family use it | +| Set the user's own presence, DND, or custom status from the UI (presence/DND optimistic; custom status forwards to the service and waits for the real event — see `presenceController.CustomStatus`) | `(*App).applyPresenceAction(action, snoozeMinutes)` (`internal/ui/presence_action.go`); the presence menu, custom snooze and the `/away` family use it | | Switch to a channel, joining it first if the user is not a member | `(*App).openChannel(channelfinder.ChannelResult)` (`internal/ui/open_channel.go`); the channel finder and `/open` use it | | Resolve a command's channel argument (`#name` or a `<#C…\|name>` mention) to a channel, joined or not | `(*App).findChannelByID` (finder rows, by ID) and `matchChannelByName` (case- and accent-insensitive, channels and private channels only) in `internal/ui/slash_builtins.go`; a name the finder does not hold goes to the server through `ChannelService.SearchRemote` and comes back as `channelLookupMsg` (reduced in `reducer_slash_command.go`), because the finder's rows are only the user's own conversations. `/open` and `/join` use it | | Open (or find) a DM / group DM and switch to it, optionally pre-filling its draft | `(*App).openConversation(userIDs, msgPrefill)` (`internal/ui/open_conversation.go`); the new-message picker and `/msg` use it. Request IDs come from a counter that never resets; a `/msg` pre-fill whose open fails, is cancelled or is superseded is given back by `abandonMsgPrefill`, and `/msg` refuses while `openingConversation()` | diff --git a/cmd/slk/main.go b/cmd/slk/main.go index 2a38fab6..1f000193 100644 --- a/cmd/slk/main.go +++ b/cmd/slk/main.go @@ -662,6 +662,14 @@ func run() error { } app.SetSettingsService(core.NewSettingsService(saveTheme, saveSidebarWidth)) + // endOfDayUnix returns 23:59:59 local time on now's date, as a Unix + // timestamp, so a WFH status auto-clears at the end of the day + // instead of lingering indefinitely. + endOfDayUnix := func(now time.Time) int64 { + y, m, d := now.Date() + return time.Date(y, m, d, 23, 59, 59, 0, now.Location()).Unix() + } + // Wire presence/DND status setter. Resolves the active team ID // through the router at invocation so the closure always targets // the currently-active workspace context. @@ -681,6 +689,12 @@ func run() error { err = wctx.Client.SetUserPresence(ctx, "away") case presencemenu.ActionSnooze: _, err = wctx.Client.SetSnooze(ctx, snoozeMinutes) + case presencemenu.ActionSetWFH: + err = wctx.Client.SetCustomStatus(ctx, "Working from home", ":house:", endOfDayUnix(time.Now())) + case presencemenu.ActionSetVacation: + err = wctx.Client.SetCustomStatus(ctx, "On vacation", ":palm_tree:", 0) + case presencemenu.ActionClearStatus: + err = wctx.Client.SetCustomStatus(ctx, "", "", 0) case presencemenu.ActionEndDND: // End any active manual snooze AND any active scheduled // DND session. Either may be a no-op depending on the diff --git a/docs/superpowers/specs/2026-10-22-workspace-status-presets-design.md b/docs/superpowers/specs/2026-10-22-workspace-status-presets-design.md new file mode 100644 index 00000000..22b8a5af --- /dev/null +++ b/docs/superpowers/specs/2026-10-22-workspace-status-presets-design.md @@ -0,0 +1,157 @@ +# Workspace status presets — design + +Issue: https://github.com/gammons/slk/issues/356 (approved) +PR: https://github.com/gammons/slk/pull/360 +Author: SieuJS + +## Problem + +The `Ctrl+S` status menu (`internal/ui/presencemenu`) only set presence +(Active/Away) and DND snooze. Slack's actual custom status (free text + +emoji + expiration, `users.profile.set`) was never exposed from `slk`. + +Custom status in Slack is scoped **per workspace** — your profile, +including status, belongs to each workspace membership independently, +same as presence/DND already are in `slk` (see `presenceController` in +`internal/ui/presence.go`). + +## What shipped + +Two preset rows — **"Working from home"** (🏠) and **"On vacation"** +(🌴) — plus a **"Clear status"** row (shown only once a status is set) +in the `Ctrl+S` menu. Presence (Active/Away) never clears an existing +custom status; they're independent, matching real Slack. The status +bar shows a segment (e.g. `🏠 Working from home`) when a status is +active, and the menu shows a `✓` on the matching row when reopened. + +## Architecture: self-status reads from the peer-status store + +This is the one decision worth recording, because it wasn't the first +draft — PR review (two independent passes) caught that the initial +approach was wrong. + +**First draft:** a second, client-only cache — `workspaceStatus. +StatusText`/`StatusEmoji` in `presenceController`, written only when +`Apply()` ran a WFH/Vacation/Clear action from the menu. + +**Why that was wrong**, per review: `slk` already has a complete store +for exactly this data — `presenceController.peers` (`peerstatus. +Status`), used today to show *other people's* custom status in the +sidebar, DM headers, and channel finder. It's seeded at connect/switch +from `cachedPeerStatuses`/`ListUserStatuses` (which has no self-filter +on the `users` cache table), kept live via `UserStatusChangeMsg` (fed +by `user_change`/`user_status_changed`, which also carry no self-filter +— `cmd/slk/peer_status.go`'s `OnUserStatusChange`), and already expired +on a schedule via `peerStatusTickMsg`. The authenticated user's own +status flows through this exact pipeline; the first draft just never +read it back out for `a.currentUserID`. + +Keeping a second, write-only-by-`Apply` cache meant: +- The status vanished on restart (nothing re-read it from the peer + store or re-fetched it from Slack). +- An edit made from the real Slack client was never reflected. +- A failed `users.profile.set` call still displayed as if it had + succeeded, because the cache was written optimistically and nothing + ever corrected it (presence self-corrects via the next + `StatusChangeMsg`; the custom-status cache had no equivalent). +- WFH's end-of-day expiration was never locally honored — no deadline + field existed on the self-only cache at all. + +**Final architecture:** `presenceController.CustomStatus(userID)` reads +`p.peers[userID]` directly (`HasStatus`/`StatusGlyph` on `peerstatus. +Status` already handle expiry and shortcode→glyph). `Apply()` is now a +no-op for `ActionSetWFH`/`ActionSetVacation`/`ActionClearStatus` — they +only forward to `presenceSvc.SetStatus`, which calls the real API +(`Client.SetCustomStatus`, wrapping `slack-go`'s +`SetUserCustomStatusContext`). The status bar and the menu's checkmark +update only once the real `UserStatusChangeMsg` echo arrives (via +`applyPeer`) — not optimistically. This fixes all four problems above +at once: restart persistence, external edits, failed-request display, +and local expiry, all for free, with less code than the first draft +had. + +One ordering constraint this implies: whichever code seeds the active +workspace's custom-status display (workspace switch, `InitialActive` +activation) must call `a.presence.SetPeers(...)` (which seeds +`p.peers` for the new workspace) *before* calling `a.statusbar. +SetCustomStatus(a.presence.CustomStatus(a.currentUserID))` — reading +`CustomStatus` first would see the previous workspace's (or no) +peers. + +## Decisions + +1. **Presets, not a free-text editor.** Free-text/custom-emoji entry + is out of scope for this pass. +2. **Expiration:** WFH auto-clears server-side at end of day (local + time, 23:59:59, `cmd/slk/main.go`'s `endOfDayUnix`). Vacation has no + expiration — cleared manually. Both now also clear/display correctly + client-side once Slack's event reflects the change, per the + architecture above. +3. **Presence and custom status are independent**, matching real + Slack — picking Active/Away never touches the other. +4. **"Clear status"** only appears once a status is actually set. + +## Known test-fragility traps (useful if this menu grows further) + +- `internal/ui/mode_presence_menu_test.go` hardcodes the unfiltered row + count in `presenceMenuAllRows` and walks the cursor with a fixed + number of `j`/down presses in several cases. Both need updating for + any future row count change. +- The same file's "a printable key filters the menu" case filters by + the literal rune `"a"` — "On vacation" substring-matches via + "vacation", so any future row whose label contains "a" changes the + expected count there too. +- `internal/ui/golden_dialogs_test.go` / `testdata/golden/ + dialog_presence_menu.ansi` needs regenerating with `go test + ./internal/ui/ -run TestGolden_Dialogs -update` whenever the row + count changes. + +## Out of scope + +- Free-text / custom-emoji status entry. +- Syncing status across workspaces — each workspace's status is + independent, intentionally. + +## Review history + +Three rounds of automated review (Greptile, plus a bot posting as +@gammons — no human maintainer review yet as of this writing) on +PR #360 found: + +- The self-status architecture issue above (fixed by this redesign). +- The optimistic-display-on-failure issue (fixed as a side effect of + the same redesign). +- Emoji-only statuses (no text) were dropped everywhere — the status + bar and the menu's Clear-status row both gated visibility on status + text alone, discarding the emoji. Fixed: visibility now checks + text-or-emoji. +- Status-bar width overflow for long statuses. The redesign above made + this reachable: `slk` now shows *any* status set from the real Slack + client, not just this feature's own two short fixed presets, and + Slack allows up to 100 characters. Fixed: the custom-status segment + now degrades (full text → emoji + truncated text + ellipsis → emoji + alone → dropped) to stay within the status bar's exact-width budget, + rather than assuming a fixed short length. +- That the `Ctrl+S` menu can now exceed a short terminal's height with + no scrolling — a pre-existing limitation of the menu, not new to this + change, just easier to hit. Left as a follow-up, not fixed here. + +A follow-up local review (`codex-batch-review`, scoped to just the +emoji-only/truncation fix commit) caught three more real issues in that +fix itself, all fixed with regression tests: + +- The truncation budget math double-counted a separator when the + status segment landed between two other segments (e.g. an unread + badge before it, the connection indicator after it) — it always + needs exactly one *more* separator than the other segments' own + joined width already accounts for, not one per side. +- The truncation loop hand-rolled rune-by-rune width summation instead + of reusing `muesli/reflow/truncate.StringWithTail` — already the + established width-aware truncator elsewhere in this codebase + (`internal/ui/presencemenu`, `internal/ui/thread/breadcrumb.go`, + and a dozen other files). Rune-by-rune summation can mismeasure + combined grapheme clusters (variation selectors, ZWJ sequences) in + status text and let a segment exceed its budget. +- When the budget allowed the emoji but not even one character of + text, the result was a bare "emoji …" with nothing actually + truncated — now falls back to emoji alone instead. diff --git a/internal/core/types.go b/internal/core/types.go index 2f684835..e14d5528 100644 --- a/internal/core/types.go +++ b/internal/core/types.go @@ -144,6 +144,9 @@ const ( PresenceSnooze // SnoozeMinutes is set PresenceCustomSnooze // opens the custom-snooze input; never reaches the engine PresenceEndDND + PresenceSetWFH // custom status preset: "Working from home" + PresenceSetVacation // custom status preset: "On vacation" + PresenceClearStatus // clears the profile status text/emoji; leaves presence/DND alone ) // ThemeScope identifies whether a theme selection should be saved to diff --git a/internal/slack/client.go b/internal/slack/client.go index bd6ad44c..5602ba5f 100644 --- a/internal/slack/client.go +++ b/internal/slack/client.go @@ -50,6 +50,7 @@ type SlackAPI interface { JoinConversation(channelID string) (*slack.Channel, string, []string, error) SetUserPresenceContext(ctx context.Context, presence string) error GetUserPresenceContext(ctx context.Context, user string) (*slack.UserPresence, error) + SetUserCustomStatusContext(ctx context.Context, statusText, statusEmoji string, statusExpiration int64) error SetSnoozeContext(ctx context.Context, minutes int) (*slack.DNDStatus, error) EndSnoozeContext(ctx context.Context) (*slack.DNDStatus, error) EndDNDContext(ctx context.Context) error @@ -1313,6 +1314,15 @@ func (c *Client) GetUserPresence(ctx context.Context, userID string) (*slack.Use return p, nil } +// SetCustomStatus sets the authenticated user's profile status text, +// emoji, and expiration (Unix seconds; 0 means it never auto-clears). +func (c *Client) SetCustomStatus(ctx context.Context, statusText, statusEmoji string, statusExpiration int64) error { + if err := c.api.SetUserCustomStatusContext(ctx, statusText, statusEmoji, statusExpiration); err != nil { + return fmt.Errorf("setting custom status: %w", err) + } + return nil +} + // SetSnooze enables Do-Not-Disturb for `minutes` minutes. func (c *Client) SetSnooze(ctx context.Context, minutes int) (*slack.DNDStatus, error) { st, err := c.api.SetSnoozeContext(ctx, minutes) diff --git a/internal/slack/client_test.go b/internal/slack/client_test.go index 7b0ff8a8..1072d394 100644 --- a/internal/slack/client_test.go +++ b/internal/slack/client_test.go @@ -142,6 +142,7 @@ type mockSlackAPI struct { getEmojiFn func() (map[string]string, error) getPermalinkContextFn func(ctx context.Context, params *slack.PermalinkParameters) (string, error) setUserPresenceContextFn func(ctx context.Context, presence string) error + setUserCustomStatusContextFn func(ctx context.Context, statusText, statusEmoji string, statusExpiration int64) error getUserPresenceContextFn func(ctx context.Context, user string) (*slack.UserPresence, error) setSnoozeContextFn func(ctx context.Context, minutes int) (*slack.DNDStatus, error) endSnoozeContextFn func(ctx context.Context) (*slack.DNDStatus, error) @@ -250,6 +251,13 @@ func (m *mockSlackAPI) SetUserPresenceContext(ctx context.Context, presence stri return nil } +func (m *mockSlackAPI) SetUserCustomStatusContext(ctx context.Context, statusText, statusEmoji string, statusExpiration int64) error { + if m.setUserCustomStatusContextFn != nil { + return m.setUserCustomStatusContextFn(ctx, statusText, statusEmoji, statusExpiration) + } + return nil +} + func (m *mockSlackAPI) GetUserPresenceContext(ctx context.Context, user string) (*slack.UserPresence, error) { if m.getUserPresenceContextFn != nil { return m.getUserPresenceContextFn(ctx, user) @@ -407,6 +415,50 @@ func TestClient_SetUserPresence(t *testing.T) { } } +func TestClient_SetCustomStatus(t *testing.T) { + var calls int + var gotText, gotEmoji string + var gotExpiration int64 + mock := &mockSlackAPI{ + setUserCustomStatusContextFn: func(ctx context.Context, statusText, statusEmoji string, statusExpiration int64) error { + calls++ + gotText, gotEmoji, gotExpiration = statusText, statusEmoji, statusExpiration + return nil + }, + } + c := &Client{api: mock} + if err := c.SetCustomStatus(context.Background(), "Working from home", ":house:", 12345); err != nil { + t.Fatalf("SetCustomStatus: %v", err) + } + if calls != 1 { + t.Errorf("expected 1 SetCustomStatus call, got %d", calls) + } + if gotText != "Working from home" || gotEmoji != ":house:" || gotExpiration != 12345 { + t.Errorf("got (%q, %q, %d), want (%q, %q, %d)", + gotText, gotEmoji, gotExpiration, "Working from home", ":house:", int64(12345)) + } +} + +func TestClient_SetCustomStatus_Error(t *testing.T) { + wantErr := errors.New("api boom") + mock := &mockSlackAPI{ + setUserCustomStatusContextFn: func(ctx context.Context, statusText, statusEmoji string, statusExpiration int64) error { + return wantErr + }, + } + c := &Client{api: mock} + err := c.SetCustomStatus(context.Background(), "On vacation", ":palm_tree:", 0) + if err == nil { + t.Fatal("expected error, got nil") + } + if !errors.Is(err, wantErr) { + t.Errorf("expected wrapped wantErr, got %v", err) + } + if !strings.Contains(err.Error(), "setting custom status") { + t.Errorf("expected 'setting custom status' prefix, got %q", err.Error()) + } +} + func TestClient_SetUserPresence_Error(t *testing.T) { wantErr := errors.New("api boom") mock := &mockSlackAPI{ diff --git a/internal/ui/app_presence_test.go b/internal/ui/app_presence_test.go index 56aea0f5..429b1160 100644 --- a/internal/ui/app_presence_test.go +++ b/internal/ui/app_presence_test.go @@ -8,9 +8,11 @@ package ui import ( + "strings" "testing" "time" + "github.com/gammons/slk/internal/ui/peerstatus" "github.com/gammons/slk/internal/ui/presencemenu" ) @@ -117,6 +119,188 @@ func TestApplyOptimisticStatusInitializesMissingTeamEntry(t *testing.T) { } } +// Self-status architecture, round 3 of review: an earlier draft of +// this feature cached "your own" custom status in a second, +// client-only store (workspaceStatus.StatusText/StatusEmoji) written +// only when Apply ran a WFH/Vacation/ClearStatus action. Review caught +// that this meant the status vanished on restart, never reflected an +// edit made from the real Slack client, and that a failed API call +// still displayed as if it had succeeded (nothing corrected it, since +// no event comes back to say otherwise). All three are fixed by +// routing self-status through the SAME peers store already used for +// everyone else's custom status: Slack's status/profile events carry +// no self-filter, so the current user's own status flows through +// UserStatusChangeMsg/SetPeers exactly like a teammate's does — restart +// persistence, external edits, and expiry (via the existing +// peerStatusTickMsg tick) all come for free. Apply() no longer writes +// anything for WFH/Vacation/ClearStatus; those three only forward to +// presenceSvc (the real Slack API call) and wait for the real event. + +func TestCustomStatusReadsFromPeerStore(t *testing.T) { + a := NewApp() + a.currentUserID = "U1" + + _, _ = a.Update(UserStatusChangeMsg{ + UserID: "U1", + Emoji: ":house:", + Text: "Working from home", + }) + + text, emoji := a.presence.CustomStatus("U1") + if text != "Working from home" { + t.Errorf("CustomStatus text = %q, want %q", text, "Working from home") + } + if emoji == "" { + t.Error("expected a non-empty display glyph") + } +} + +func TestCustomStatusEmptyWhenExpired(t *testing.T) { + a := NewApp() + a.currentUserID = "U1" + + _, _ = a.Update(UserStatusChangeMsg{ + UserID: "U1", + Emoji: ":palm_tree:", + Text: "On vacation", + Expires: time.Now().Add(-time.Minute), + }) + + text, emoji := a.presence.CustomStatus("U1") + if text != "" || emoji != "" { + t.Errorf("expected empty status once expired, got text=%q emoji=%q", text, emoji) + } +} + +func TestCustomStatusEmptyForUnknownUser(t *testing.T) { + a := NewApp() + + text, emoji := a.presence.CustomStatus("no-such-user") + if text != "" || emoji != "" { + t.Errorf("expected empty status for unknown user, got text=%q emoji=%q", text, emoji) + } +} + +// TestSelfStatusChangeRefreshesStatusBar: a UserStatusChangeMsg for the +// current user — the echo of our own Ctrl+S pick, or an edit made from +// the real Slack client — must update the status bar. +func TestSelfStatusChangeRefreshesStatusBar(t *testing.T) { + a := NewApp() + a.currentUserID = "U1" + + _, _ = a.Update(UserStatusChangeMsg{ + UserID: "U1", + Emoji: ":house:", + Text: "Working from home", + }) + + if !strings.Contains(a.statusbar.View(120), "Working from home") { + t.Error("status bar must show self custom status after UserStatusChangeMsg") + } +} + +// TestSelfStatusExpiryClearsStatusBar closes the gap explicitly called +// out as out-of-scope in the original draft: self-status now shares +// the peers store's existing expiry tick, so a status that reached its +// deadline clears from the status bar the same way a peer's does, with +// no separate mechanism needed. +func TestSelfStatusExpiryClearsStatusBar(t *testing.T) { + a := NewApp() + a.currentUserID = "U1" + _, _ = a.Update(UserStatusChangeMsg{ + UserID: "U1", + Emoji: ":house:", + Text: "Working from home", + Expires: time.Now().Add(time.Millisecond), + }) + time.Sleep(2 * time.Millisecond) + + _, _ = a.Update(peerStatusTickMsg{}) + + if strings.Contains(a.statusbar.View(120), "Working from home") { + t.Error("status bar must clear self custom status once its deadline passes") + } +} + +// TestWorkspaceSwitchSeedsSelfCustomStatus replaces the per-team cache +// this PR originally used (found buggy in review) with the +// per-workspace peers store SetPeers already replaces wholesale on +// every switch: each workspace's self status is independent and comes +// from the server-fed snapshot, not a client-only cache. +func TestWorkspaceSwitchSeedsSelfCustomStatus(t *testing.T) { + a := newTestApp(t, withActiveTeam("T1")) + a.currentUserID = "U1" + a.presence.peers = map[string]peerstatus.Status{ + "U1": peerstatus.Status{}.WithStatus(":house:", "Working from home", time.Time{}), + } + a.statusbar.SetCustomStatus(a.presence.CustomStatus("U1")) + + _, _ = a.Update(WorkspaceSwitchedMsg{TeamID: "T2", TeamName: "beta", UserID: "U2"}) + + if strings.Contains(a.statusbar.View(120), "Working from home") { + t.Error("status bar still shows the previous workspace's self status after switching") + } +} + +func TestWorkspaceSwitchRoundTripRestoresSelfCustomStatus(t *testing.T) { + a := newTestApp(t, withActiveTeam("T1")) + a.currentUserID = "U1" + + _, _ = a.Update(WorkspaceSwitchedMsg{ + TeamID: "T1", TeamName: "acme", UserID: "U1", + UserStatuses: map[string]peerstatus.Status{ + "U1": peerstatus.Status{}.WithStatus(":house:", "Working from home", time.Time{}), + }, + }) + _, _ = a.Update(WorkspaceSwitchedMsg{ + TeamID: "T2", TeamName: "beta", UserID: "U2", + UserStatuses: map[string]peerstatus.Status{ + "U2": peerstatus.Status{}.WithStatus(":palm_tree:", "On vacation", time.Time{}), + }, + }) + _, _ = a.Update(WorkspaceSwitchedMsg{ + TeamID: "T1", TeamName: "acme", UserID: "U1", + UserStatuses: map[string]peerstatus.Status{ + "U1": peerstatus.Status{}.WithStatus(":house:", "Working from home", time.Time{}), + }, + }) + + out := a.statusbar.View(120) + if !strings.Contains(out, "Working from home") { + t.Errorf("expected T1's self status restored on switch-back, got: %q", out) + } + if strings.Contains(out, "On vacation") { + t.Errorf("T2's self status leaked into T1's status bar: %q", out) + } +} + +// TestInitialActiveActivationSeedsSelfCustomStatus pins the race found +// in review: workspaces connect concurrently, and router.Add happens +// for every one of them regardless of InitialActive — so a non-default +// workspace can finish connecting, be switched to, and have a status +// set on it before the slower, configured-default workspace's +// WorkspaceReadyMsg arrives and claims InitialActive. That claim must +// still refresh the status bar from the newly-active workspace's own +// seeded peers, not leave the previous workspace's status on screen. +func TestInitialActiveActivationSeedsSelfCustomStatus(t *testing.T) { + a := newTestApp(t, withActiveTeam("T-other")) + a.currentUserID = "U-other" + a.presence.peers = map[string]peerstatus.Status{ + "U-other": peerstatus.Status{}.WithStatus(":house:", "Working from home", time.Time{}), + } + a.statusbar.SetCustomStatus(a.presence.CustomStatus("U-other")) + + _, _ = a.Update(WorkspaceReadyMsg{ + TeamID: "T-default", + InitialActive: true, + UserID: "U-default", + }) + + if strings.Contains(a.statusbar.View(120), "Working from home") { + t.Error("status bar still shows the previous workspace's self status after InitialActive activation") + } +} + func TestStatusChangeMsgUpdatesPerTeamCache(t *testing.T) { a := NewApp() a.activeTeamID = "T1" diff --git a/internal/ui/mode_normal.go b/internal/ui/mode_normal.go index 8be9549c..4908939f 100644 --- a/internal/ui/mode_normal.go +++ b/internal/ui/mode_normal.go @@ -296,6 +296,8 @@ func handleNormalMode(a *App, msg tea.KeyMsg) tea.Cmd { case key.Matches(msg, a.keys.PresenceMenu): header := a.workspaceNameForActive() pres, dndEnabled, dndEnd, _ := a.presence.Status(a.activeTeamID) + statusText, statusEmoji := a.presence.CustomStatus(a.currentUserID) + a.presenceMenu.SetCurrentCustomStatus(statusText, statusEmoji) a.presenceMenu.OpenWith(header, pres, dndEnabled, dndEnd) a.SetMode(ModePresenceMenu) diff --git a/internal/ui/mode_presence_menu_test.go b/internal/ui/mode_presence_menu_test.go index 1b8beeb2..473e9818 100644 --- a/internal/ui/mode_presence_menu_test.go +++ b/internal/ui/mode_presence_menu_test.go @@ -1,6 +1,7 @@ package ui import ( + "strings" "testing" "time" @@ -11,9 +12,10 @@ import ( ) // presenceMenuAllRows is the unfiltered row count for a menu opened -// without DND active: Active, Away, six fixed snoozes, "until tomorrow -// morning", and "Snooze custom...". -const presenceMenuAllRows = 10 +// without DND active and no custom status set: Active, Away, Working +// from home, On vacation, six fixed snoozes, "until tomorrow morning", +// and "Snooze custom...". +const presenceMenuAllRows = 12 // openPresenceMenu mirrors the production open path // (mode_normal.go:228-231) for a workspace with no DND, and records @@ -247,6 +249,108 @@ func TestPresenceMenuModeKeys(t *testing.T) { } }, }, + { + // Custom status is no longer optimistic (round 3 of + // review): Apply is a no-op for WFH/Vacation/Clear, so + // selecting a row only forwards the action; the display + // changes once the real UserStatusChangeMsg echo arrives + // (see TestSelfStatusChangeRefreshesStatusBar). + name: "enter on Working from home forwards the action without an optimistic display", + opts: opts, + setup: func(t *testing.T, a *App) { + open(t, a) + typePresenceQuery(t, a, "home", 1) + }, + key: keyCode(tea.KeyEnter), + wantMode: ModeNormal, + assert: func(t *testing.T, a *App, _ tea.Cmd) { + if len(calls) != 1 || calls[0].action != presencemenu.ActionSetWFH { + t.Errorf("setStatusFn calls = %+v, want one ActionSetWFH", calls) + } + if strings.Contains(a.statusbar.View(120), "Working from home") { + t.Error("status bar must not show the status before the real event confirms it") + } + }, + }, + { + name: "enter on On vacation forwards the action without an optimistic display", + opts: opts, + setup: func(t *testing.T, a *App) { + open(t, a) + typePresenceQuery(t, a, "vacation", 1) + }, + key: keyCode(tea.KeyEnter), + wantMode: ModeNormal, + assert: func(t *testing.T, a *App, _ tea.Cmd) { + if len(calls) != 1 || calls[0].action != presencemenu.ActionSetVacation { + t.Errorf("setStatusFn calls = %+v, want one ActionSetVacation", calls) + } + }, + }, + { + // Pins the invariant that presence and custom status are + // independent in real Slack: picking Active must not wipe + // an existing custom status. An earlier draft of this + // feature got this backwards. The status is seeded the way + // it actually arrives in production — a + // UserStatusChangeMsg — not via Apply, which no longer + // writes custom status at all. + name: "enter on Active does not clear an existing custom status", + opts: opts, + setup: func(t *testing.T, a *App) { + a.currentUserID = "U1" + open(t, a) + _, _ = a.Update(UserStatusChangeMsg{UserID: "U1", Emoji: ":house:", Text: "Working from home"}) + // Re-open so the row-count/cursor state reflects the + // status just set (buildItems now has the Clear-status + // row too). + a.presenceMenu.SetCurrentCustomStatus("Working from home", "🏠") + pres, dndEnabled, dndEnd, _ := a.presence.Status("T1") + a.presenceMenu.OpenWith("ws", pres, dndEnabled, dndEnd) + }, + key: keyCode(tea.KeyEnter), + wantMode: ModeNormal, + assert: func(t *testing.T, a *App, _ tea.Cmd) { + pres, _, _, _ := a.presence.Status("T1") + if pres != "active" { + t.Errorf("presence = %q, want \"active\"", pres) + } + text, _ := a.presence.CustomStatus("U1") + if text != "Working from home" { + t.Errorf("CustomStatus = %q, want it preserved as \"Working from home\"", text) + } + }, + }, + { + name: "enter on Clear status forwards the action", + opts: opts, + setup: func(t *testing.T, a *App) { + a.currentUserID = "U1" + open(t, a) + _, _ = a.Update(UserStatusChangeMsg{UserID: "U1", Emoji: ":palm_tree:", Text: "On vacation"}) + pres, dndEnabled, dndEnd, _ := a.presence.Status("T1") + a.presenceMenu.SetCurrentCustomStatus("On vacation", "🌴") + a.presenceMenu.OpenWith("ws", pres, dndEnabled, dndEnd) + if got := modalRows(&a.presenceMenu); got != presenceMenuAllRows+1 { + t.Fatalf("precondition: %d rows, want %d (Clear-status row missing)", got, presenceMenuAllRows+1) + } + typePresenceQuery(t, a, "clear", 1) + }, + key: keyCode(tea.KeyEnter), + wantMode: ModeNormal, + assert: func(t *testing.T, a *App, _ tea.Cmd) { + if len(calls) != 1 || calls[0].action != presencemenu.ActionClearStatus { + t.Errorf("setStatusFn calls = %+v, want one ActionClearStatus", calls) + } + // Clearing is also event-driven: the cache still shows + // the previous status until the real event confirms + // the clear. + text, _ := a.presence.CustomStatus("U1") + if text != "On vacation" { + t.Errorf("CustomStatus = %q, want it unchanged until the echo arrives", text) + } + }, + }, { name: "enter with no matching row is a no-op", opts: opts, @@ -461,10 +565,11 @@ func TestPresenceMenuModeKeys(t *testing.T) { key: keyPress('a'), wantMode: ModePresenceMenu, assert: func(t *testing.T, a *App, _ tea.Cmd) { - // "Active" and "Away" prefix-match; no snooze label + // "Active" and "Away" prefix-match; "On vacation" + // substring-matches via "vacation"; no snooze label // contains an "a". - if got := modalRows(&a.presenceMenu); got != 2 { - t.Errorf("rows = %d, want 2 after filtering on \"a\"", got) + if got := modalRows(&a.presenceMenu); got != 3 { + t.Errorf("rows = %d, want 3 after filtering on \"a\"", got) } }, }, @@ -473,7 +578,7 @@ func TestPresenceMenuModeKeys(t *testing.T) { opts: opts, setup: func(t *testing.T, a *App) { open(t, a) - typePresenceQuery(t, a, "a", 2) + typePresenceQuery(t, a, "a", 3) }, key: keyCode(tea.KeyBackspace), wantMode: ModePresenceMenu, @@ -525,6 +630,28 @@ func TestPresenceMenuModeKeys(t *testing.T) { }) } +// TestPresenceMenuKeyOpensWithCurrentCustomStatus exercises the real +// ctrl+s path through handleNormalMode (mode_normal.go), not the +// openPresenceMenu test helper that mirrors it — pinning that the +// production code looks up the active workspace's custom status and +// passes it to presenceMenu before opening, so the Clear-status row and +// checkmark reflect reality on open, not just when a test calls +// OpenWith directly. +func TestPresenceMenuKeyOpensWithCurrentCustomStatus(t *testing.T) { + a := newTestApp(t, withActiveTeam("T1")) + a.currentUserID = "U1" + _, _ = a.Update(UserStatusChangeMsg{UserID: "U1", Emoji: ":house:", Text: "Working from home"}) + + _ = dispatchModeKey(a, keyMod('s', tea.ModCtrl)) + + if !a.presenceMenu.IsVisible() { + t.Fatal("ctrl+s did not open the presence menu") + } + if got := modalRows(&a.presenceMenu); got != presenceMenuAllRows+1 { + t.Errorf("rows = %d, want %d (Clear-status row missing: mode_normal.go must pass the current custom status to OpenWith)", got, presenceMenuAllRows+1) + } +} + // assertCommitsTo presses enter and asserts the menu committed the // given action, which is how this table observes cursor position. func assertCommitsTo(t *testing.T, a *App, want core.PresenceAction) { diff --git a/internal/ui/presence.go b/internal/ui/presence.go index 706fd3da..dbeefb16 100644 --- a/internal/ui/presence.go +++ b/internal/ui/presence.go @@ -46,6 +46,9 @@ func hasDeadline(st peerstatus.Status) bool { // workspaceStatus caches the latest StatusChangeMsg per team so the // status bar can refresh on workspace switch without round-tripping. +// +// Custom profile status (as opposed to presence/DND) is deliberately +// NOT cached here — see CustomStatus below for why. type workspaceStatus struct { Presence string DNDEnabled bool @@ -84,15 +87,41 @@ func (p *presenceController) Status(teamID string) (presence string, dndEnabled return st.Presence, st.DNDEnabled, st.DNDEndTS, true } -// Set records the status for teamID and returns the resulting struct -// so the caller (typically StatusChangeMsg arm) can push to the -// statusbar without a second lookup. -func (p *presenceController) Set(teamID string, presence string, dndEnabled bool, dndEnd time.Time) workspaceStatus { - st := workspaceStatus{ - Presence: presence, - DNDEnabled: dndEnabled, - DNDEndTS: dndEnd, +// CustomStatus returns userID's custom profile status (text, display +// glyph) if set and unexpired, or ("", "") otherwise. +// +// This reads the SAME peers store used for everyone else's custom +// status, including when userID is the current user: Slack's +// user_change/user_status_changed events carry no self-filter, so the +// authenticated user's own status flows through UserStatusChangeMsg +// and SetPeers exactly like a teammate's does. An earlier draft of +// this cached "your own" status separately (workspaceStatus.StatusText +// /StatusEmoji, written only by Apply), and review caught that this +// meant the status vanished on restart, never reflected an edit made +// from the real Slack client, and had no expiry — all three come for +// free by sharing this store instead, since it already has the startup +// seed (SetPeers), live updates and the peerStatusTickMsg expiry tick. +func (p *presenceController) CustomStatus(userID string) (text, emoji string) { + st, ok := p.peers[userID] + now := time.Now() + if !ok || !st.HasStatus(now) { + return "", "" } + return st.Text, st.StatusGlyph(now) +} + +// Set records the presence/DND status for teamID and returns the +// resulting struct so the caller (typically StatusChangeMsg arm) can +// push to the statusbar without a second lookup. StatusChangeMsg +// carries only presence/DND fields (it arrives on every reconnect and +// on any presence/DND change, not just the status preset selection), +// so the existing StatusText/StatusEmoji are preserved rather than +// rebuilt from a zero-value struct. +func (p *presenceController) Set(teamID string, presence string, dndEnabled bool, dndEnd time.Time) workspaceStatus { + st := p.byTeam[teamID] + st.Presence = presence + st.DNDEnabled = dndEnabled + st.DNDEndTS = dndEnd p.byTeam[teamID] = st return st } @@ -108,10 +137,17 @@ func (p *presenceController) ClearDNDFor(teamID string) workspaceStatus { return st } -// Apply mutates teamID's cached status based on the chosen presencemenu -// action. SetActive/SetAway touch only Presence; Snooze sets DND fields -// and leaves Presence alone; EndDND clears DND fields and leaves -// Presence alone. Returns the resulting struct. +// Apply mutates teamID's cached presence/DND based on the chosen +// presencemenu action. SetActive/SetAway touch only Presence; Snooze +// sets DND fields and leaves Presence alone; EndDND clears DND fields +// and leaves Presence alone. ActionSetWFH/ActionSetVacation/ +// ActionClearStatus are deliberately no-ops here: they carry no local +// cache write at all (see CustomStatus) — applyPresenceAction forwards +// them to presenceSvc, which makes the real Slack API call, and the +// resulting UserStatusChangeMsg echo is what actually updates the +// display. This also means a failed API call never shows as applied: +// nothing was shown optimistically to begin with. Returns the +// resulting struct. func (p *presenceController) Apply(teamID string, action core.PresenceAction, snoozeMinutes int) workspaceStatus { st := p.byTeam[teamID] switch action { @@ -280,6 +316,9 @@ func (p *presenceController) applyPeer(a *App, userID string, st peerstatus.Stat } a.threadPanel.PatchUserStatus(userID, st) p.pushDMStatus(a, userID, time.Now()) + if userID == a.currentUserID { + a.statusbar.SetCustomStatus(p.CustomStatus(userID)) + } return p.claimPeerTick(st) } @@ -359,6 +398,9 @@ func (p *presenceController) expirePeers(a *App, now time.Time) tea.Cmd { st = st.Clear(now) p.peers[uid] = st p.pushDMStatus(a, uid, now) + if uid == a.currentUserID { + a.statusbar.SetCustomStatus(p.CustomStatus(uid)) + } } if hasDeadline(st) { pending = true diff --git a/internal/ui/presence_action.go b/internal/ui/presence_action.go index e914ce95..58edbd6e 100644 --- a/internal/ui/presence_action.go +++ b/internal/ui/presence_action.go @@ -3,9 +3,14 @@ package ui import "github.com/gammons/slk/internal/core" // applyPresenceAction applies a presence choice for the workspace on -// screen. Optimistic: the cached status and the status bar change at -// once, before the API call returns; the WebSocket echo reaffirms them. -// The presence menu, its custom snooze and the /away family share it. +// screen. Presence/DND is optimistic: the cached status and the status +// bar change at once, before the API call returns; the WebSocket echo +// reaffirms them. Custom status (WFH/Vacation/Clear) is NOT optimistic +// — Apply is a no-op for those actions, and the status bar only +// updates once the real UserStatusChangeMsg echo arrives (see +// CustomStatus), so a failed request never displays as if it had +// succeeded. The presence menu, its custom snooze and the /away family +// share this. func (a *App) applyPresenceAction(action core.PresenceAction, snoozeMinutes int) { st := a.presence.Apply(a.activeTeamID, action, snoozeMinutes) a.statusbar.SetStatus(st.Presence, st.DNDEnabled, st.DNDEndTS) diff --git a/internal/ui/presencemenu/model.go b/internal/ui/presencemenu/model.go index ebb9d770..0399c0cb 100644 --- a/internal/ui/presencemenu/model.go +++ b/internal/ui/presencemenu/model.go @@ -22,6 +22,9 @@ const ( ActionSnooze = core.PresenceSnooze ActionCustomSnooze = core.PresenceCustomSnooze ActionEndDND = core.PresenceEndDND + ActionSetWFH = core.PresenceSetWFH + ActionSetVacation = core.PresenceSetVacation + ActionClearStatus = core.PresenceClearStatus ) // Result is returned when the user commits a selection. @@ -48,6 +51,15 @@ type Model struct { workspaceName string currentPres string // "active" / "away" / "" dndActive bool + + // currentStatusText/currentStatusEmoji are the profile status + // currently applied (e.g. "Working from home" / "🏠"), set via + // SetCurrentCustomStatus before OpenWith so the matching preset row + // renders with a checkmark and the Clear-status row appears. Slack + // allows an emoji-only status with no text, so Clear-status + // visibility must check both, not text alone. + currentStatusText string + currentStatusEmoji string } var _ interface { @@ -70,10 +82,19 @@ func (m *Model) OpenWith(workspaceName, presence string, dndEnabled bool, dndEnd m.workspaceName = workspaceName m.currentPres = presence m.dndActive = dndEnabled && (dndEnd.IsZero() || time.Now().Before(dndEnd)) - m.items = buildItems(presence, m.dndActive) + m.items = buildItems(presence, m.dndActive, m.currentStatusText, m.currentStatusEmoji) m.filter() } +// SetCurrentCustomStatus records the profile status currently applied +// (from presenceController's cache), so the next OpenWith call marks +// the matching preset row as current and shows the Clear-status row. +// Call before OpenWith. +func (m *Model) SetCurrentCustomStatus(text, emoji string) { + m.currentStatusText = text + m.currentStatusEmoji = emoji +} + // hasEndDNDItem reports whether the End-DND row is present. Exposed for tests. func (m Model) hasEndDNDItem() bool { for _, it := range m.items { @@ -84,16 +105,43 @@ func (m Model) hasEndDNDItem() bool { return false } +// hasClearStatusItem reports whether the Clear-status row is present. +// Exposed for tests. +func (m Model) hasClearStatusItem() bool { + for _, it := range m.items { + if it.action == ActionClearStatus { + return true + } + } + return false +} + +// currentItemLabels returns the labels of every item flagged current. +// Presence (Active/Away) and a custom status preset are independent in +// real Slack, so both can be current at once — this is not a +// single-selection concept. Exposed for tests. +func (m Model) currentItemLabels() []string { + var labels []string + for _, it := range m.items { + if it.current { + labels = append(labels, it.label) + } + } + return labels +} + // buildItems composes the menu rows based on current state. // // Labels are plain ASCII (no leading emoji glyphs) so that lipgloss's // width measurement and the terminal's actual cell width agree. Mixed // emoji width — particularly the moon — caused the modal's right // border to not draw correctly in some terminals. -func buildItems(presence string, dndActive bool) []item { +func buildItems(presence string, dndActive bool, statusText, statusEmoji string) []item { rows := []item{ {label: "Active", action: ActionSetActive, current: presence == "active" && !dndActive}, {label: "Away", action: ActionSetAway, current: presence == "away" && !dndActive}, + {label: "Working from home", action: ActionSetWFH, current: statusText == "Working from home"}, + {label: "On vacation", action: ActionSetVacation, current: statusText == "On vacation"}, {label: "Snooze for 20 minutes", action: ActionSnooze, minutes: 20}, {label: "Snooze for 1 hour", action: ActionSnooze, minutes: 60}, {label: "Snooze for 2 hours", action: ActionSnooze, minutes: 120}, @@ -103,6 +151,9 @@ func buildItems(presence string, dndActive bool) []item { {label: "Snooze until tomorrow morning", action: ActionSnooze, minutes: minutesUntilTomorrowMorning(time.Now())}, {label: "Snooze custom...", action: ActionCustomSnooze}, } + if statusText != "" || statusEmoji != "" { + rows = append(rows, item{label: "Clear status", action: ActionClearStatus}) + } if dndActive { rows = append(rows, item{label: "End snooze / DND", action: ActionEndDND}) } diff --git a/internal/ui/presencemenu/model_test.go b/internal/ui/presencemenu/model_test.go index f270534c..930306de 100644 --- a/internal/ui/presencemenu/model_test.go +++ b/internal/ui/presencemenu/model_test.go @@ -30,15 +30,123 @@ func TestModel_SelectAway(t *testing.T) { func TestModel_SnoozeOption(t *testing.T) { m := New() m.OpenWith("Workspace", "active", false, time.Time{}) - // Active(0), Away(1), Snooze 20m(2) — third item. - m.HandleKey("j") - m.HandleKey("j") + // Filter to the unique "20 minutes" row rather than a hardcoded + // cursor offset, so adding/removing preset rows above it can't + // silently change which row this test exercises. + for _, r := range "20 min" { + m.HandleKey(string(r)) + } r := m.HandleKey("enter") if r == nil || r.Action != ActionSnooze || r.SnoozeMinutes != 20 { t.Fatalf("expected 20m snooze, got %+v", r) } } +func TestModel_SelectWFH(t *testing.T) { + m := New() + m.OpenWith("Workspace", "active", false, time.Time{}) + m.HandleKey("h") + m.HandleKey("o") + m.HandleKey("m") + r := m.HandleKey("enter") + if r == nil || r.Action != ActionSetWFH { + t.Fatalf("expected ActionSetWFH, got %+v", r) + } +} + +func TestModel_SelectVacation(t *testing.T) { + m := New() + m.OpenWith("Workspace", "active", false, time.Time{}) + m.HandleKey("v") + m.HandleKey("a") + m.HandleKey("c") + r := m.HandleKey("enter") + if r == nil || r.Action != ActionSetVacation { + t.Fatalf("expected ActionSetVacation, got %+v", r) + } +} + +// TestModel_SetCurrentCustomStatus_MarksMatchingRowCurrent pins +// SetCurrentCustomStatus + OpenWith wiring the checkmark onto whichever +// preset row matches the status text currently applied. Presence and +// custom status are independent in real Slack, so Active stays +// checkmarked too — this is not a single-selection menu. +func TestModel_SetCurrentCustomStatus_MarksMatchingRowCurrent(t *testing.T) { + m := New() + m.SetCurrentCustomStatus("Working from home", "🏠") + m.OpenWith("Workspace", "active", false, time.Time{}) + + labels := m.currentItemLabels() + if !containsLabel(labels, "Working from home") { + t.Fatalf("expected \"Working from home\" marked current, got %v", labels) + } + if !containsLabel(labels, "Active") { + t.Fatalf("expected \"Active\" to remain marked current alongside the status, got %v", labels) + } +} + +func TestModel_NoCustomStatus_NeitherPresetRowCurrent(t *testing.T) { + m := New() + m.OpenWith("Workspace", "active", false, time.Time{}) + + labels := m.currentItemLabels() + if containsLabel(labels, "Working from home") || containsLabel(labels, "On vacation") { + t.Fatalf("expected no status preset marked current, got %v", labels) + } +} + +func containsLabel(labels []string, want string) bool { + for _, l := range labels { + if l == want { + return true + } + } + return false +} + +func TestModel_ClearStatusOnlyVisibleWhenStatusSet(t *testing.T) { + m := New() + m.OpenWith("Workspace", "active", false, time.Time{}) + if m.hasClearStatusItem() { + t.Error("Clear status should not appear when no status is set") + } + + m2 := New() + m2.SetCurrentCustomStatus("On vacation", "🌴") + m2.OpenWith("Workspace", "active", false, time.Time{}) + if !m2.hasClearStatusItem() { + t.Error("Clear status should appear when a status is set") + } +} + +// TestModel_ClearStatusVisibleForEmojiOnlyStatus pins a real bug found +// in review: Slack allows a status with only an emoji, no text (set +// from the real Slack client, say). Clear status must still appear so +// the user has a way to remove it from slk. +func TestModel_ClearStatusVisibleForEmojiOnlyStatus(t *testing.T) { + m := New() + m.SetCurrentCustomStatus("", "🎉") + m.OpenWith("Workspace", "active", false, time.Time{}) + if !m.hasClearStatusItem() { + t.Error("Clear status should appear for an emoji-only status") + } +} + +func TestModel_ClearStatusAction(t *testing.T) { + m := New() + m.SetCurrentCustomStatus("On vacation", "🌴") + m.OpenWith("Workspace", "active", false, time.Time{}) + m.HandleKey("c") + m.HandleKey("l") + m.HandleKey("e") + m.HandleKey("a") + m.HandleKey("r") + r := m.HandleKey("enter") + if r == nil || r.Action != ActionClearStatus { + t.Fatalf("expected ActionClearStatus, got %+v", r) + } +} + func TestModel_EndDNDOnlyVisibleWhenSnoozed(t *testing.T) { m := New() m.OpenWith("Workspace", "active", false, time.Time{}) @@ -56,9 +164,8 @@ func TestModel_EndDNDOnlyVisibleWhenSnoozed(t *testing.T) { func TestModel_CustomSnoozeAction(t *testing.T) { m := New() m.OpenWith("Workspace", "active", false, time.Time{}) - // Walk down past Active, Away, and the 7 snooze durations to "Snooze custom..." - for i := 0; i < 9; i++ { - m.HandleKey("j") + for _, r := range "custom" { + m.HandleKey(string(r)) } r := m.HandleKey("enter") if r == nil || r.Action != ActionCustomSnooze { diff --git a/internal/ui/reducer_workspace.go b/internal/ui/reducer_workspace.go index 1cd00f0b..d7bf2795 100644 --- a/internal/ui/reducer_workspace.go +++ b/internal/ui/reducer_workspace.go @@ -241,6 +241,7 @@ func reduceWorkspaceReady(a *App, m WorkspaceReadyMsg) tea.Cmd { a.backJump = nil pres, dndEnabled, dndEnd, _ := a.presence.Status(a.activeTeamID) a.statusbar.SetStatus(pres, dndEnabled, dndEnd) + a.statusbar.SetCustomStatus(a.presence.CustomStatus(a.currentUserID)) a.workspaceRail.SelectByID(m.TeamID) if len(m.Channels) > 0 { // Restore the last-visited channel across restarts (persisted in @@ -409,7 +410,11 @@ func reduceWorkspaceSwitched(a *App, m WorkspaceSwitchedMsg) tea.Cmd { var batch []tea.Cmd // After resetWindowTree above, so the fresh panes get the statuses. + // Must run before SetCustomStatus below: self-status is read from + // this same peers store, so it has to be seeded for the new + // workspace first. batch = append(batch, a.presence.SetPeers(a, m.UserStatuses)) + a.statusbar.SetCustomStatus(a.presence.CustomStatus(a.currentUserID)) // Restore the last-viewed channel for this workspace if we // have one and it still exists; otherwise fall back to the // first channel in the sidebar. Move the sidebar cursor to diff --git a/internal/ui/statusbar/model.go b/internal/ui/statusbar/model.go index fc42600c..2c373178 100644 --- a/internal/ui/statusbar/model.go +++ b/internal/ui/statusbar/model.go @@ -8,6 +8,7 @@ import ( "charm.land/lipgloss/v2" "github.com/gammons/slk/internal/ui/styles" + "github.com/muesli/reflow/truncate" ) // ConnectionState represents the WebSocket connection status. @@ -32,6 +33,8 @@ type Model struct { presence string // "active", "away", or "" (unknown — segment hidden) dndEnabled bool dndEndTS time.Time // zero if not in DND + statusText string // profile status preset text (e.g. "Working from home"); "" hides the segment + statusEmoji string // display glyph for statusText syncing bool // true while a background cache-verify fetch is in flight helpHint string // muted text rendered in the gap area; empty disables commandLine string // "" == inactive; otherwise the :prompt shown in place of channel/workspace @@ -124,6 +127,17 @@ func (m *Model) SetStatus(presence string, dndEnabled bool, dndEndTS time.Time) m.dirty() } +// SetCustomStatus updates the profile status preset segment (e.g. +// "Working from home"). Empty text hides the segment. +func (m *Model) SetCustomStatus(text, emoji string) { + if m.statusText == text && m.statusEmoji == emoji { + return + } + m.statusText = text + m.statusEmoji = emoji + m.dirty() +} + func (m *Model) SetInThread(inThread bool) { if m.inThread != inThread { m.inThread = inThread @@ -259,6 +273,12 @@ func (m Model) View(width int) string { Render(m.toast)) } + // The custom-status segment is sized to fit below, once the width + // left over by every other segment is known; statusInsertIdx marks + // where it belongs in the visual order (between toast and + // presence/DND) without needing those segments built around it. + statusInsertIdx := len(rightParts) + // Presence + DND segment (hidden when presence is "" and not in DND) if m.dndEnabled { rightParts = append(rightParts, @@ -310,15 +330,6 @@ func (m Model) View(width int) string { sep := lipgloss.NewStyle().Background(styles.SurfaceDark).Render(" ") trailing := lipgloss.NewStyle().Background(styles.SurfaceDark).Render(" ") - rightContent := "" - for i, p := range rightParts { - if i > 0 { - rightContent += sep - } - rightContent += p - } - rightContent += trailing // trailing padding (extra space for unicode width variance) - // Right-align so the rightmost segment ends at column (width-1), // matching the inner right edge of the message-pane / thread-pane // compose box (which ends 1 column before the screen edge -- the @@ -326,6 +337,34 @@ func (m Model) View(width int) string { // content rows above). Without this gap, the connection indicator // overhangs visually past the compose box. const rightGutter = 1 + + // Size the custom-status segment to whatever room is left after + // every other segment. Slack allows up to 100 characters of status + // text — unlike this feature's own two short presets, a status set + // from the real Slack client has no length slk controls, so it + // must degrade (truncate, then emoji-only, then dropped) rather + // than push the bar over width. + if m.statusText != "" || m.statusEmoji != "" { + // otherWidth already counts len(rightParts)-1 separators (the + // ones between the existing parts). Inserting one more part — + // anywhere among them, not just at an end — always turns that + // into len(rightParts) separators total: exactly one more than + // otherWidth already has, never two, regardless of where it + // lands. + otherWidth := lipgloss.Width(joinRightParts(rightParts, sep)) + extraSep := 0 + if len(rightParts) > 0 { + extraSep = 1 + } + budget := width - rightGutter - lipgloss.Width(left) - otherWidth - lipgloss.Width(trailing) - extraSep*lipgloss.Width(sep) + if seg := fitStatusSegment(m.statusEmoji, m.statusText, budget); seg != "" { + rightParts = append(rightParts[:statusInsertIdx:statusInsertIdx], + append([]string{seg}, rightParts[statusInsertIdx:]...)...) + } + } + + rightContent := joinRightParts(rightParts, sep) + trailing // trailing padding (extra space for unicode width variance) + gap := width - rightGutter - lipgloss.Width(left) - lipgloss.Width(rightContent) if gap < 0 { gap = 0 @@ -362,6 +401,68 @@ func (m Model) View(width int) string { return lipgloss.JoinHorizontal(lipgloss.Center, left, filler, rightContent, rightPad) } +// joinRightParts joins the right-side segments with sep between each, +// matching how they're visually separated in the rendered bar. +func joinRightParts(parts []string, sep string) string { + s := "" + for i, p := range parts { + if i > 0 { + s += sep + } + s += p + } + return s +} + +// fitStatusSegment returns the styled custom-status segment sized to +// fit within maxWidth columns, degrading in steps: the full "emoji +// text", then emoji plus as much text as fits with a trailing +// ellipsis, then emoji alone, then "" if nothing fits. Needed because +// Slack allows up to 100 characters of status text — far longer than +// this feature's own two short presets — so a status set from the +// real Slack client must not be allowed to push the status bar over +// its exact-width budget. +func fitStatusSegment(emoji, text string, maxWidth int) string { + style := lipgloss.NewStyle().Foreground(styles.Accent).Background(styles.SurfaceDark) + full := strings.TrimSpace(emoji + " " + text) + if full == "" || maxWidth <= 0 { + return "" + } + if lipgloss.Width(full) <= maxWidth { + return style.Render(full) + } + emojiAlone := func() string { + if emoji != "" && lipgloss.Width(emoji) <= maxWidth { + return style.Render(emoji) + } + return "" + } + if text == "" { + return emojiAlone() + } + prefix := emoji + if prefix != "" { + prefix += " " + } + prefixWidth := lipgloss.Width(prefix) + if prefixWidth >= maxWidth { + return emojiAlone() + } + // truncate.StringWithTail is the same width-aware truncator already + // used elsewhere in this codebase (internal/ui/presencemenu, + // internal/ui/thread/breadcrumb.go, ...) — reused here rather than + // hand-rolling rune-by-rune width summation, which can mismeasure + // combined grapheme clusters (variation selectors, ZWJ sequences) + // in the status text and let a segment exceed maxWidth. + truncated := truncate.StringWithTail(text, uint(maxWidth-prefixWidth), "…") + if truncated == "" || truncated == "…" { + // Not even one character of text fit: an ellipsis with nothing + // preceding it reads as broken, so fall back to emoji alone. + return emojiAlone() + } + return style.Render(prefix + truncated) +} + // formatDND renders the DND segment with an optional countdown to the // snooze end. Zero endTS or past endTS produces a bare "🌙 DND". func formatDND(endTS time.Time) string { diff --git a/internal/ui/statusbar/model_test.go b/internal/ui/statusbar/model_test.go index 5cca888c..2677ed6c 100644 --- a/internal/ui/statusbar/model_test.go +++ b/internal/ui/statusbar/model_test.go @@ -160,6 +160,182 @@ func TestStatusBar_DNDNoEndTimestamp(t *testing.T) { } } +func TestStatusBar_CustomStatusSegment(t *testing.T) { + m := New() + m.SetCustomStatus("Working from home", "🏠") + out := stripANSI(m.View(120)) + if !strings.Contains(out, "🏠 Working from home") { + t.Errorf("expected '🏠 Working from home', got: %q", out) + } +} + +// TestStatusBar_CustomStatusSegment_EmojiOnly pins a real bug found in +// review: Slack allows a status with only an emoji, no text (e.g. set +// from the real Slack client). The segment must still show it, and +// without a stray leading/trailing space from the empty text half. +func TestStatusBar_CustomStatusSegment_EmojiOnly(t *testing.T) { + m := New() + m.SetCustomStatus("", "🎉") + m.SetConnectionState(StateConnected) + out := stripANSI(m.View(120)) + if !strings.Contains(out, "🎉") { + t.Errorf("expected emoji-only status to render, got: %q", out) + } + // A stray leading/trailing space from the empty text half would + // double up with the normal inter-segment separator, e.g. + // "🎉 ● Connected" instead of "🎉 ● Connected". + if strings.Contains(out, "🎉 ") { + t.Errorf("stray space after emoji-only status, got: %q", out) + } +} + +func TestStatusBar_NoCustomStatus_NoSegment(t *testing.T) { + m := New() + // Default state — no SetCustomStatus call. + out := stripANSI(m.View(120)) + if strings.Contains(out, "Working from home") || strings.Contains(out, "On vacation") { + t.Errorf("expected no custom-status segment when unset, got: %q", out) + } +} + +func TestModel_SetCustomStatus_BumpsVersionOnChange(t *testing.T) { + m := New() + v0 := m.Version() + m.SetCustomStatus("On vacation", "🌴") + if m.Version() == v0 { + t.Fatal("SetCustomStatus must bump Version() on change") + } + v1 := m.Version() + m.SetCustomStatus("On vacation", "🌴") // same value + if m.Version() != v1 { + t.Fatal("SetCustomStatus with unchanged value must be a no-op") + } +} + +// TestStatusBar_LongCustomStatusTruncatesToFitWidth pins a real bug +// found in review: the architecture now shows ANY status set from the +// real Slack client, not just the two short fixed presets this PR +// shipped — and Slack allows up to 100 characters of status text. A +// long status must not push the exact-width invariant over budget or +// hide the connection indicator; it must be cut to fit. +func TestStatusBar_LongCustomStatusTruncatesToFitWidth(t *testing.T) { + m := New() + m.SetMode(testMode("NORMAL")) + m.SetChannel("general") + m.SetWorkspace("Acme") + m.SetConnectionState(StateConnected) + m.SetCustomStatus(strings.Repeat("a very long status indeed ", 5), "🏠") + + const width = 60 + view := m.View(width) + if got := lipgloss.Width(view); got != width { + t.Fatalf("View(%d) is %d columns wide, want %d", width, got, width) + } + if !strings.Contains(stripANSI(view), "Connected") { + t.Errorf("connection indicator must still render, got: %q", stripANSI(view)) + } +} + +// TestStatusBar_StatusSegmentDropsWhenNoRoom: at an extremely narrow +// width even an emoji-only segment may not fit; the bar must still +// hold its exact width and keep the connection indicator rather than +// silently exceeding width. +func TestStatusBar_StatusSegmentDropsWhenNoRoom(t *testing.T) { + m := New() + m.SetMode(testMode("NORMAL")) + m.SetChannel("general") + m.SetWorkspace("Acme") + m.SetConnectionState(StateConnected) + m.SetCustomStatus("Working from home", "🏠") + + const width = 44 // exactly fits mode+channel+workspace+connection with no room for any status + view := m.View(width) + if got := lipgloss.Width(view); got != width { + t.Fatalf("View(%d) is %d columns wide, want %d", width, got, width) + } + if !strings.Contains(stripANSI(view), "Connected") { + t.Errorf("connection indicator must still render even when the status segment is dropped, got: %q", stripANSI(view)) + } +} + +// TestStatusBar_TruncationMiddleCase pins the degrade-in-steps middle +// step specifically (full and emoji-alone were the only two extremes +// the earlier tests exercised): a status too long to show in full, but +// with enough room for the emoji plus some truncated text and an +// ellipsis — not dropped to emoji-alone. +func TestStatusBar_TruncationMiddleCase(t *testing.T) { + m := New() + m.SetMode(testMode("NORMAL")) + m.SetChannel("general") + m.SetWorkspace("Acme") + m.SetConnectionState(StateConnected) + m.SetCustomStatus("a moderately long status message here", "🏠") + + const width = 70 + out := stripANSI(m.View(width)) + if !strings.Contains(out, "🏠") { + t.Fatalf("expected emoji to render, got: %q", out) + } + if !strings.Contains(out, "…") { + t.Fatalf("expected an ellipsis marking truncated text, got: %q", out) + } + // Some actual text must precede the ellipsis — not just a bare + // "emoji …" with nothing truncated. + if strings.Contains(out, "🏠 …") { + t.Errorf("ellipsis with no preceding text: should fall back to emoji alone instead, got: %q", out) + } + if lipgloss.Width(m.View(width)) != width { + t.Errorf("View(%d) is %d columns wide, want %d", width, lipgloss.Width(m.View(width)), width) + } +} + +// TestStatusBar_InteriorInsertionDoesNotOverSubtractBudget pins a real +// bug found in review: when the status segment is inserted BETWEEN two +// existing segments (something before it, something after it — here, +// unread present, toast absent, connection present), the budget +// calculation double-counted a separator that was already included in +// otherWidth, undercounting the true available budget by one column. +// An emoji-only status that needs exactly the remaining room was +// dropped even though it would actually have fit. +func TestStatusBar_InteriorInsertionDoesNotOverSubtractBudget(t *testing.T) { + m := New() + m.SetMode(testMode("NORMAL")) + m.SetChannel("general") + m.SetWorkspace("Acme") + m.SetUnreadCount(3) + m.SetConnectionState(StateConnected) + m.SetCustomStatus("", "🎉") + + // 57 is the minimum width that fits every other segment with zero + // slack (empirically: narrower widths don't shrink it further, the + // bar just can't go below it). The emoji (width 2) plus one + // separator needs exactly 2 more columns — 60 total. The old, + // over-subtracting budget math needed 61: one column more than + // actually required. + const width = 60 + out := stripANSI(m.View(width)) + if !strings.Contains(out, "🎉") { + t.Errorf("emoji-only status should fit exactly at this width, got: %q", out) + } + if got := lipgloss.Width(m.View(width)); got != width { + t.Errorf("View(%d) is %d columns wide, want %d", width, got, width) + } +} + +func TestStatusBar_CustomStatusDoesNotBreakExactWidth(t *testing.T) { + m := New() + m.SetMode(testMode("NORMAL")) + m.SetChannel("general") + m.SetWorkspace("Acme") + m.SetCustomStatus("Working from home", "🏠") + + for _, width := range []int{80, 120, 240} { + if got := lipgloss.Width(m.View(width)); got != width { + t.Errorf("View(%d) is %d columns wide, want %d", width, got, width) + } + } +} + func TestStatusBar_PresenceUnknown_NoSegment(t *testing.T) { m := New() // Default state — no SetStatus call. Status segment should not appear. diff --git a/internal/ui/testdata/golden/dialog_presence_menu.ansi b/internal/ui/testdata/golden/dialog_presence_menu.ansi index d0ab6ad9..2939f48d 100644 --- a/internal/ui/testdata/golden/dialog_presence_menu.ansi +++ b/internal/ui/testdata/golden/dialog_presence_menu.ansi @@ -6,21 +6,22 @@  ┃  # general  ┃ │ │   ┃  # engineering ● ┃ │ ── Today ── │   ┃  # muted-noise  ┃ │▌bob 9:00 AM │  - ┃ ┃ │▌shipped the thing │  - ┃  ▾ DMs ╭──────────────────────────────────────────────────────────╮ │  - ┃  ● bob  │ │ │  - ┃  ○ carol  │ Status — Acme │ │  - ┃ │ ▌ █ Type to filter… │ │  + ┃ ╭──────────────────────────────────────────────────────────╮ │  + ┃  ▾ DMs │ │ │  + ┃  ● bob  │ Status — Acme │ │  + ┃  ○ carol  │ ▌ █ Type to filter… │ │   ┃ │ │ │   ┃ │  ✓ Active  │ │   ┃ │  Away  │ │  + ┃ │  Working from home  │ │  + ┃ │ ▌On vacation  │ │   ┃ │  Snooze for 20 minutes  │ │  - ┃ │ ▌Snooze for 1 hour  │ │  + ┃ │  Snooze for 1 hour  │ │   ┃ │  Snooze for 2 hours  │ │  - ┃ │  Snooze for 4 hours  │ │  - ┃ │  Snooze for 8 hours  │ │  - ┃ │  Snooze for 24 hours  │ wrap at every width the │  - ┃ │  Snooze until tomorrow morning  │wrapping behaviour │  + ┃ │  Snooze for 4 hours  │ │  + ┃ │  Snooze for 8 hours  │ wrap at every width the │  + ┃ │  Snooze for 24 hours  │wrapping behaviour │  + ┃ │  Snooze until tomorrow morning  │ │   ┃ │  Snooze custom...  │ │   ┃ │ │ │   ┃ ╰──────────────────────────────────────────────────────────╯ │  @@ -28,7 +29,6 @@  ┃ ┃ │ │   ┃ ┃ │ │   ┃ ┃ │ │  - ┃ ┃ │ │   ┃ ┃ │▌  │   ┃ ┃ │▌ Message #general... (i to insert)  │   ┃ ┃ │▌  │  diff --git a/wiki/Features.md b/wiki/Features.md index 5a4d5f98..6c151824 100644 --- a/wiki/Features.md +++ b/wiki/Features.md @@ -132,6 +132,7 @@ See [[Terminal Compatibility|Terminal-Compatibility]] for which protocol your te - Set self presence (Active / Away) and DND/snooze from `Ctrl+S` - Standard snooze durations (20m / 1h / 2h / 4h / 8h / 24h / until tomorrow morning) plus custom minutes +- Workspace status presets (Working from home / On vacation) and Clear status, independent of presence — picking Active/Away never clears an existing status. Per-workspace, like presence and DND. - Live status segment in the status bar with snooze countdown - Reflects external state changes — set from the official Slack client or via your own API scripts — in real time over the WebSocket - Shows other people's state too: their custom status emoji (🎧 instead while they are in a huddle) follows their name on DM rows, message authors and channel-finder rows, `⊘` replaces the presence dot while they are in DND, and an open DM's header shows the full status text and when their DND ends diff --git a/wiki/Keybindings.md b/wiki/Keybindings.md index 63d2f46e..327ce498 100644 --- a/wiki/Keybindings.md +++ b/wiki/Keybindings.md @@ -62,7 +62,7 @@ | Click | Any (on image) | Open full-screen preview | | `Ctrl+y` | Any | Switch theme | | `Alt+y` / `Alt+Shift+y` | Normal | Next / previous theme (wraps; saved per workspace) | -| `Ctrl+s` | Any | Set status (Active / Away / DND snooze) | +| `Ctrl+s` | Any | Set status (Active / Away / DND snooze / Working from home / On vacation / Clear status) | | `q` | Normal | Close the thread pane (does nothing when none is open) | | `Q` | Normal | Quit (with confirmation) | | `Ctrl+c` | Any | Quit (with confirmation) |