Skip to content

Commit 436d755

Browse files
Thomas StrombergThomas Stromberg
Thomas Stromberg
authored and
Thomas Stromberg
committed
improve cross-platform behaviors
1 parent 3aa24aa commit 436d755

File tree

3 files changed

+62
-110
lines changed

3 files changed

+62
-110
lines changed

‎cmd/goose/loginitem_darwin.go‎

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ func appPath() (string, error) {
142142
}
143143

144144
// addLoginItemUI adds the login item menu option (macOS only).
145-
func addLoginItemUI(ctx context.Context, _ *App) {
145+
func addLoginItemUI(ctx context.Context, app *App) {
146146
// Check if we're running from an app bundle
147147
execPath, err := os.Executable()
148148
if err != nil {
@@ -163,33 +163,28 @@ func addLoginItemUI(ctx context.Context, _ *App) {
163163
return
164164
}
165165

166-
loginItem := systray.AddMenuItem("Start at Login", "Automatically start when you log in")
167-
168-
// Set initial state
166+
// Add text checkmark for consistency with other menu items
167+
var loginText string
169168
if isLoginItem(ctx) {
170-
loginItem.Check()
169+
loginText = "✓ Start at Login"
170+
} else {
171+
loginText = "Start at Login"
171172
}
173+
loginItem := systray.AddMenuItem(loginText, "Automatically start when you log in")
172174

173175
loginItem.Click(func() {
174176
isEnabled := isLoginItem(ctx)
175177
newState := !isEnabled
176178

177179
if err := setLoginItem(ctx, newState); err != nil {
178180
slog.Error("Failed to set login item", "error", err)
179-
// Revert the UI state on error
180-
if isEnabled {
181-
loginItem.Check()
182-
} else {
183-
loginItem.Uncheck()
184-
}
185181
return
186182
}
187183

188184
// Update UI state
189-
if newState {
190-
loginItem.Check()
191-
} else {
192-
loginItem.Uncheck()
193-
}
185+
slog.Info("[SETTINGS] Start at Login toggled", "enabled", newState)
186+
187+
// Rebuild menu to update checkmark
188+
app.rebuildMenu(ctx)
194189
})
195190
}

‎cmd/goose/main_test.go‎

Lines changed: 20 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -199,9 +199,9 @@ func TestTrayIconRestoredAfterNetworkRecovery(t *testing.T) {
199199
initialTitle := mock.title
200200

201201
// Expected title varies by platform
202-
expectedTitle := "1/1" // Linux format
202+
expectedTitle := "" // Most platforms: icon only, no text
203203
if runtime.GOOS == "darwin" {
204-
expectedTitle = "1" // macOS format
204+
expectedTitle = "1" // macOS: show count with icon
205205
}
206206

207207
if initialTitle != expectedTitle {
@@ -250,7 +250,7 @@ func TestTrayTitleUpdates(t *testing.T) {
250250
name: "no PRs",
251251
incoming: []PR{},
252252
outgoing: []PR{},
253-
expectedTitle: "😊",
253+
expectedTitle: "", // No count shown when no blocked PRs
254254
},
255255
{
256256
name: "only incoming blocked",
@@ -259,7 +259,7 @@ func TestTrayTitleUpdates(t *testing.T) {
259259
{Repository: "test/repo", Number: 2, NeedsReview: true, UpdatedAt: time.Now()},
260260
},
261261
outgoing: []PR{},
262-
expectedTitle: "🪿 2",
262+
expectedTitle: "2", // macOS format: just the count
263263
},
264264
{
265265
name: "only outgoing blocked",
@@ -269,7 +269,7 @@ func TestTrayTitleUpdates(t *testing.T) {
269269
{Repository: "test/repo", Number: 4, IsBlocked: true, UpdatedAt: time.Now()},
270270
{Repository: "test/repo", Number: 5, IsBlocked: true, UpdatedAt: time.Now()},
271271
},
272-
expectedTitle: "🎉 3",
272+
expectedTitle: "3", // macOS format: just the count
273273
},
274274
{
275275
name: "both incoming and outgoing blocked",
@@ -279,7 +279,7 @@ func TestTrayTitleUpdates(t *testing.T) {
279279
outgoing: []PR{
280280
{Repository: "test/repo", Number: 2, IsBlocked: true, UpdatedAt: time.Now()},
281281
},
282-
expectedTitle: "🪿 1 🎉 1",
282+
expectedTitle: "1 / 1", // macOS format: "incoming / outgoing"
283283
},
284284
{
285285
name: "mixed blocked and unblocked",
@@ -291,7 +291,7 @@ func TestTrayTitleUpdates(t *testing.T) {
291291
{Repository: "test/repo", Number: 3, IsBlocked: false, UpdatedAt: time.Now()},
292292
{Repository: "test/repo", Number: 4, IsBlocked: true, UpdatedAt: time.Now()},
293293
},
294-
expectedTitle: "🪿 1 🎉 1",
294+
expectedTitle: "1 / 1", // macOS format: "incoming / outgoing"
295295
},
296296
{
297297
name: "hidden org filters out blocked PRs",
@@ -301,7 +301,7 @@ func TestTrayTitleUpdates(t *testing.T) {
301301
},
302302
outgoing: []PR{},
303303
hiddenOrgs: map[string]bool{"hidden-org": true},
304-
expectedTitle: "🪿 1",
304+
expectedTitle: "1", // macOS format: just the count
305305
},
306306
{
307307
name: "stale PRs filtered when hideStaleIncoming is true",
@@ -311,7 +311,7 @@ func TestTrayTitleUpdates(t *testing.T) {
311311
},
312312
outgoing: []PR{},
313313
hideStaleIncoming: true,
314-
expectedTitle: "🪿 1",
314+
expectedTitle: "1", // macOS format: just the count
315315
},
316316
}
317317

@@ -322,22 +322,19 @@ func TestTrayTitleUpdates(t *testing.T) {
322322
app.hiddenOrgs = tt.hiddenOrgs
323323
app.hideStaleIncoming = tt.hideStaleIncoming
324324

325-
// Get the title that would be set
326-
counts := app.countPRs()
327-
var title string
328-
switch {
329-
case counts.IncomingBlocked == 0 && counts.OutgoingBlocked == 0:
330-
title = "😊"
331-
case counts.IncomingBlocked > 0 && counts.OutgoingBlocked > 0:
332-
title = fmt.Sprintf("🪿 %d 🎉 %d", counts.IncomingBlocked, counts.OutgoingBlocked)
333-
case counts.IncomingBlocked > 0:
334-
title = fmt.Sprintf("🪿 %d", counts.IncomingBlocked)
335-
default:
336-
title = fmt.Sprintf("🎉 %d", counts.OutgoingBlocked)
325+
// Call setTrayTitle to get the actual title
326+
app.setTrayTitle()
327+
actualTitle := app.systrayInterface.(*MockSystray).title
328+
329+
// Adjust expected title based on platform
330+
expectedTitle := tt.expectedTitle
331+
if runtime.GOOS != "darwin" {
332+
// Non-macOS platforms show icon only (no text)
333+
expectedTitle = ""
337334
}
338335

339-
if title != tt.expectedTitle {
340-
t.Errorf("Expected tray title %q, got %q", tt.expectedTitle, title)
336+
if actualTitle != expectedTitle {
337+
t.Errorf("Expected tray title %q, got %q", expectedTitle, actualTitle)
341338
}
342339
})
343340
}

‎cmd/goose/ui.go‎

Lines changed: 31 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -194,38 +194,35 @@ func (app *App) setTrayTitle() {
194194
var title string
195195
var iconType IconType
196196

197-
// On Linux, always show counts if there are any PRs
198-
// This helps since not all desktop environments show the text
199-
if runtime.GOOS == "linux" && (counts.IncomingTotal > 0 || counts.OutgoingTotal > 0) {
200-
// Show blocked/total format for better visibility
201-
if counts.IncomingBlocked > 0 && counts.OutgoingBlocked > 0 {
202-
title = fmt.Sprintf("%d/%d • %d/%d", counts.IncomingBlocked, counts.IncomingTotal, counts.OutgoingBlocked, counts.OutgoingTotal)
197+
// On macOS, show counts with the icon
198+
// On all other platforms (Linux, Windows, FreeBSD, etc), just show the icon
199+
if runtime.GOOS == "darwin" {
200+
// macOS: show counts alongside icon
201+
switch {
202+
case counts.IncomingBlocked == 0 && counts.OutgoingBlocked == 0:
203+
title = ""
204+
iconType = IconSmiling
205+
case counts.IncomingBlocked > 0 && counts.OutgoingBlocked > 0:
206+
title = fmt.Sprintf("%d / %d", counts.IncomingBlocked, counts.OutgoingBlocked)
203207
iconType = IconBoth
204-
} else if counts.IncomingBlocked > 0 {
205-
title = fmt.Sprintf("%d/%d", counts.IncomingBlocked, counts.IncomingTotal)
208+
case counts.IncomingBlocked > 0:
209+
title = fmt.Sprintf("%d", counts.IncomingBlocked)
206210
iconType = IconGoose
207-
} else if counts.OutgoingBlocked > 0 {
208-
title = fmt.Sprintf("%d/%d", counts.OutgoingBlocked, counts.OutgoingTotal)
211+
default:
212+
title = fmt.Sprintf("%d", counts.OutgoingBlocked)
209213
iconType = IconPopper
210-
} else {
211-
// No blocked PRs but there are PRs
212-
title = fmt.Sprintf("0/%d", counts.IncomingTotal+counts.OutgoingTotal)
213-
iconType = IconSmiling
214214
}
215215
} else {
216-
// Original behavior for other platforms
216+
// All other platforms: icon only, no text
217+
title = ""
217218
switch {
218219
case counts.IncomingBlocked == 0 && counts.OutgoingBlocked == 0:
219-
title = ""
220220
iconType = IconSmiling
221221
case counts.IncomingBlocked > 0 && counts.OutgoingBlocked > 0:
222-
title = fmt.Sprintf("%d / %d", counts.IncomingBlocked, counts.OutgoingBlocked)
223222
iconType = IconBoth
224223
case counts.IncomingBlocked > 0:
225-
title = fmt.Sprintf("%d", counts.IncomingBlocked)
226224
iconType = IconGoose
227225
default:
228-
title = fmt.Sprintf("%d", counts.OutgoingBlocked)
229226
iconType = IconPopper
230227
}
231228
}
@@ -788,7 +785,7 @@ func (app *App) addStaticMenuItems(ctx context.Context) {
788785
// Add checkbox items for each org
789786
for _, org := range orgs {
790787
orgName := org // Capture for closure
791-
// Add text checkmark for Linux
788+
// Add text checkmark for all platforms
792789
var orgText string
793790
if hiddenOrgs[orgName] {
794791
orgText = "✓ " + orgName
@@ -797,122 +794,90 @@ func (app *App) addStaticMenuItems(ctx context.Context) {
797794
}
798795
orgItem := hideOrgsMenu.AddSubMenuItem(orgText, "")
799796

800-
// Check if org is currently hidden (for non-Linux)
801-
if runtime.GOOS != "linux" && hiddenOrgs[orgName] {
802-
orgItem.Check()
803-
}
804-
805797
orgItem.Click(func() {
806798
app.mu.Lock()
807799
if app.hiddenOrgs[orgName] {
808800
delete(app.hiddenOrgs, orgName)
809-
if runtime.GOOS != "linux" {
810-
orgItem.Uncheck()
811-
}
812801
slog.Info("[SETTINGS] Unhiding org", "org", orgName)
813802
} else {
814803
app.hiddenOrgs[orgName] = true
815-
if runtime.GOOS != "linux" {
816-
orgItem.Check()
817-
}
818804
slog.Info("[SETTINGS] Hiding org", "org", orgName)
819805
}
820806
app.mu.Unlock()
821807

822808
// Save settings
823809
app.saveSettings()
824810

825-
// Note: Menu needs manual refresh on Linux to see changes
811+
// Rebuild menu to update checkmarks
812+
app.rebuildMenu(ctx)
826813
})
827814
}
828815
}
829816

830817
// Hide stale PRs
831-
// Add 'Hide stale PRs' option with text checkmark for Linux
818+
// Add 'Hide stale PRs' option with text checkmark for all platforms
832819
var hideStaleText string
833820
if app.hideStaleIncoming {
834821
hideStaleText = "✓ Hide stale PRs (>90 days)"
835822
} else {
836823
hideStaleText = "Hide stale PRs (>90 days)"
837824
}
838825
hideStaleItem := app.systrayInterface.AddMenuItem(hideStaleText, "")
839-
if runtime.GOOS != "linux" && app.hideStaleIncoming {
840-
hideStaleItem.Check()
841-
}
842826
hideStaleItem.Click(func() {
843827
app.mu.Lock()
844828
app.hideStaleIncoming = !app.hideStaleIncoming
845829
hideStale := app.hideStaleIncoming
846830
app.mu.Unlock()
847831

848-
if runtime.GOOS != "linux" {
849-
if hideStale {
850-
hideStaleItem.Check()
851-
} else {
852-
hideStaleItem.Uncheck()
853-
}
854-
}
855-
856832
// Save settings to disk
857833
app.saveSettings()
858834

859-
// Note: Menu needs manual refresh on Linux to see changes
860835
slog.Info("[SETTINGS] Hide stale PRs toggled", "enabled", hideStale)
836+
837+
// Rebuild menu to update checkmarks
838+
app.rebuildMenu(ctx)
861839
})
862840

863841
// Add login item option (macOS only)
864842
addLoginItemUI(ctx, app)
865843

866844
// Audio cues
867-
// Add 'Audio cues' option with text checkmark for Linux
845+
// Add 'Audio cues' option with text checkmark for all platforms
868846
app.mu.RLock()
869847
var audioText string
870848
if app.enableAudioCues {
871849
audioText = "✓ Honks enabled"
872850
} else {
873851
audioText = "Honks enabled"
874852
}
875-
enableAudioCues := app.enableAudioCues
876853
app.mu.RUnlock()
877854
audioItem := app.systrayInterface.AddMenuItem(audioText, "Play sounds for notifications")
878-
if runtime.GOOS != "linux" && enableAudioCues {
879-
audioItem.Check()
880-
}
881855
audioItem.Click(func() {
882856
app.mu.Lock()
883857
app.enableAudioCues = !app.enableAudioCues
884858
enabled := app.enableAudioCues
885859
app.mu.Unlock()
886860

887-
if runtime.GOOS != "linux" {
888-
if enabled {
889-
audioItem.Check()
890-
} else {
891-
audioItem.Uncheck()
892-
}
893-
}
894-
895861
slog.Info("[SETTINGS] Audio cues toggled", "enabled", enabled)
896862

897863
// Save settings to disk
898864
app.saveSettings()
865+
866+
// Rebuild menu to update checkmarks
867+
app.rebuildMenu(ctx)
899868
})
900869

901870
// Auto-open blocked PRs in browser
902-
// Add 'Auto-open PRs' option with text checkmark for Linux
871+
// Add 'Auto-open PRs' option with text checkmark for all platforms
903872
app.mu.RLock()
904873
var autoText string
905874
if app.enableAutoBrowser {
906875
autoText = "✓ Auto-open incoming PRs"
907876
} else {
908877
autoText = "Auto-open incoming PRs"
909878
}
910-
enableAutoBrowser := app.enableAutoBrowser
911879
app.mu.RUnlock()
912880
autoOpenItem := app.systrayInterface.AddMenuItem(autoText, "Automatically open newly blocked PRs in browser (rate limited)")
913-
if runtime.GOOS != "linux" && enableAutoBrowser {
914-
autoOpenItem.Check()
915-
}
916881
autoOpenItem.Click(func() {
917882
app.mu.Lock()
918883
app.enableAutoBrowser = !app.enableAutoBrowser
@@ -923,18 +888,13 @@ func (app *App) addStaticMenuItems(ctx context.Context) {
923888
}
924889
app.mu.Unlock()
925890

926-
if runtime.GOOS != "linux" {
927-
if enabled {
928-
autoOpenItem.Check()
929-
} else {
930-
autoOpenItem.Uncheck()
931-
}
932-
}
933-
934891
slog.Info("[SETTINGS] Auto-open blocked PRs toggled", "enabled", enabled)
935892

936893
// Save settings to disk
937894
app.saveSettings()
895+
896+
// Rebuild menu to update checkmarks
897+
app.rebuildMenu(ctx)
938898
})
939899

940900
// Quit

0 commit comments

Comments
 (0)