From 9843ec0691cbfe8df714e7c1202fcb30ce22018e Mon Sep 17 00:00:00 2001 From: "m@yim.jp" Date: Sun, 21 Dec 2025 05:33:44 +0000 Subject: [PATCH 1/6] feat: async loading UX with spinners - Startup: STS GetCallerIdentity runs in background, service list shows immediately - ResourceBrowser: spinner animation during loading states - DetailView: async refresh for extended details, immediate navigation --- internal/app/app.go | 30 ++++++++----- internal/view/detail_view.go | 75 ++++++++++++++++++++++++++++--- internal/view/resource_browser.go | 41 +++++++++++------ internal/view/view_test.go | 4 +- 4 files changed, 118 insertions(+), 32 deletions(-) diff --git a/internal/app/app.go b/internal/app/app.go index 883ba6cd..a0682829 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -21,6 +21,11 @@ import ( // clearErrorMsg is sent to clear transient errors after a timeout type clearErrorMsg struct{} +// awsContextReadyMsg is sent when AWS context initialization completes +type awsContextReadyMsg struct { + err error +} + // App is the main application model // appStyles holds cached lipgloss styles for performance type appStyles struct { @@ -87,19 +92,16 @@ func New(ctx context.Context, reg *registry.Registry) *App { // Init implements tea.Model func (a *App) Init() tea.Cmd { - // Initialize AWS context (detect region from IMDS, fetch account ID) - if err := aws.InitContext(a.ctx); err != nil { - config.Global().AddWarning("AWS init failed: " + err.Error()) - } + // Start with the service browser view immediately (no blocking on AWS calls) + a.currentView = view.NewServiceBrowser(a.ctx, a.registry) - // Show warnings if any - if len(config.Global().Warnings()) > 0 { - a.showWarnings = true + // Initialize AWS context in background (region detection, account ID fetch) + initAWSCmd := func() tea.Msg { + err := aws.InitContext(a.ctx) + return awsContextReadyMsg{err: err} } - // Start with the service browser view - a.currentView = view.NewServiceBrowser(a.ctx, a.registry) - return a.currentView.Init() + return tea.Batch(a.currentView.Init(), initAWSCmd) } // Update implements tea.Model @@ -259,6 +261,14 @@ func (a *App) Update(msg tea.Msg) (tea.Model, tea.Cmd) { a.err = nil return a, nil + case awsContextReadyMsg: + if msg.err != nil { + config.Global().AddWarning("AWS init failed: " + msg.err.Error()) + a.showWarnings = true + } + // Trigger a re-render to update header with account ID + return a, nil + case navmsg.RegionChangedMsg: log.Info("region changed", "region", msg.Region) // Pop views until we find a refreshable one (ResourceBrowser or ServiceBrowser) diff --git a/internal/view/detail_view.go b/internal/view/detail_view.go index 6c2d4224..40007b89 100644 --- a/internal/view/detail_view.go +++ b/internal/view/detail_view.go @@ -4,6 +4,7 @@ import ( "context" "strings" + "github.com/charmbracelet/bubbles/spinner" "github.com/charmbracelet/bubbles/viewport" tea "github.com/charmbracelet/bubbletea" "github.com/charmbracelet/lipgloss" @@ -43,14 +44,22 @@ type DetailView struct { width int height int registry *registry.Registry + dao dao.DAO // for async refresh + refreshing bool // true while fetching extended details + spinner spinner.Model styles detailViewStyles } // NewDetailView creates a new DetailView -func NewDetailView(ctx context.Context, resource dao.Resource, renderer render.Renderer, service, resType string, reg *registry.Registry) *DetailView { +func NewDetailView(ctx context.Context, resource dao.Resource, renderer render.Renderer, service, resType string, reg *registry.Registry, d dao.DAO) *DetailView { hp := NewHeaderPanel() hp.SetWidth(120) // Default width until SetSize is called + // Initialize spinner + s := spinner.New() + s.Spinner = spinner.Dot + s.Style = lipgloss.NewStyle().Foreground(ui.Current().Accent) + return &DetailView{ ctx: ctx, resource: resource, @@ -58,33 +67,79 @@ func NewDetailView(ctx context.Context, resource dao.Resource, renderer render.R service: service, resType: resType, registry: reg, + dao: d, headerPanel: hp, + spinner: s, styles: newDetailViewStyles(), } } +// detailRefreshMsg is sent when async resource refresh completes +type detailRefreshMsg struct { + resource dao.Resource + err error +} + // Init implements tea.Model func (d *DetailView) Init() tea.Cmd { + // Start async refresh for extended details if DAO is available + if d.dao != nil { + d.refreshing = true + return tea.Batch(d.spinner.Tick, d.refreshResource) + } return nil } +// refreshResource fetches extended resource details in background +func (d *DetailView) refreshResource() tea.Msg { + if d.dao == nil { + return detailRefreshMsg{resource: d.resource} + } + refreshed, err := d.dao.Get(d.ctx, d.resource.GetID()) + if err != nil { + return detailRefreshMsg{resource: d.resource, err: err} + } + return detailRefreshMsg{resource: refreshed} +} + // Update implements tea.Model func (d *DetailView) Update(msg tea.Msg) (tea.Model, tea.Cmd) { - if keyMsg, ok := msg.(tea.KeyMsg); ok { + switch msg := msg.(type) { + case detailRefreshMsg: + d.refreshing = false + if msg.err == nil { + d.resource = msg.resource + // Re-render content with refreshed data + if d.ready { + content := d.renderContent() + d.viewport.SetContent(content) + } + } + return d, nil + + case spinner.TickMsg: + if d.refreshing { + var cmd tea.Cmd + d.spinner, cmd = d.spinner.Update(msg) + return d, cmd + } + return d, nil + + case tea.KeyMsg: // Check for esc (both string and raw byte) - let app handle back navigation - isEsc := keyMsg.String() == "esc" || keyMsg.Type == tea.KeyEsc || keyMsg.Type == tea.KeyEscape || - (keyMsg.Type == tea.KeyRunes && len(keyMsg.Runes) == 1 && keyMsg.Runes[0] == 27) + isEsc := msg.String() == "esc" || msg.Type == tea.KeyEsc || msg.Type == tea.KeyEscape || + (msg.Type == tea.KeyRunes && len(msg.Runes) == 1 && msg.Runes[0] == 27) if isEsc { return d, nil } // Check navigation shortcuts - if model, cmd := d.handleNavigation(keyMsg.String()); model != nil { + if model, cmd := d.handleNavigation(msg.String()); model != nil { return model, cmd } // Open action menu (only if actions exist) - if keyMsg.String() == "a" { + if msg.String() == "a" { if actions := action.Global.Get(d.service, d.resType); len(actions) > 0 { actionMenu := NewActionMenu(d.ctx, d.resource, d.service, d.resType) return d, func() tea.Msg { @@ -176,7 +231,13 @@ func (d *DetailView) SetSize(width, height int) tea.Cmd { // StatusLine implements View func (d *DetailView) StatusLine() string { - parts := []string{d.resource.GetID(), "↑/↓:scroll"} + parts := []string{d.resource.GetID()} + + if d.refreshing { + parts = append(parts, d.spinner.View()+" refreshing...") + } + + parts = append(parts, "↑/↓:scroll") if actions := action.Global.Get(d.service, d.resType); len(actions) > 0 { parts = append(parts, "a:actions") diff --git a/internal/view/resource_browser.go b/internal/view/resource_browser.go index 71cdc0ef..3b298d76 100644 --- a/internal/view/resource_browser.go +++ b/internal/view/resource_browser.go @@ -7,6 +7,7 @@ import ( "strings" "time" + "github.com/charmbracelet/bubbles/spinner" "github.com/charmbracelet/bubbles/table" "github.com/charmbracelet/bubbles/textinput" tea "github.com/charmbracelet/bubbletea" @@ -87,6 +88,9 @@ type ResourceBrowser struct { sortColumn int // column index to sort by (-1 = no sort) sortAscending bool // sort direction + // Loading spinner + spinner spinner.Model + // Cached styles (initialized in initStyles) styles resourceBrowserStyles } @@ -135,6 +139,11 @@ func newResourceBrowser(ctx context.Context, reg *registry.Registry, service, re hp := NewHeaderPanel() hp.SetWidth(120) // Default width until SetSize is called + // Initialize spinner + s := spinner.New() + s.Spinner = spinner.Dot + s.Style = lipgloss.NewStyle().Foreground(ui.Current().Accent) + return &ResourceBrowser{ ctx: ctx, registry: reg, @@ -144,6 +153,7 @@ func newResourceBrowser(ctx context.Context, reg *registry.Registry, service, re loading: true, filterInput: ti, headerPanel: hp, + spinner: s, styles: newResourceBrowserStyles(), pageSize: 100, // default page size for paginated resources sortColumn: -1, // no sorting by default @@ -153,10 +163,11 @@ func newResourceBrowser(ctx context.Context, reg *registry.Registry, service, re // Init implements tea.Model func (r *ResourceBrowser) Init() tea.Cmd { + cmds := []tea.Cmd{r.loadResources, r.spinner.Tick} if r.autoReload { - return tea.Batch(r.loadResources, r.tickCmd()) + cmds = append(cmds, r.tickCmd()) } - return r.loadResources + return tea.Batch(cmds...) } // tickCmd returns a command that ticks after the auto-reload interval @@ -327,7 +338,7 @@ func (r *ResourceBrowser) Update(msg tea.Msg) (tea.Model, tea.Cmd) { // Reload resources (e.g., after region/profile change) r.loading = true r.err = nil - return r, r.loadResources + return r, tea.Batch(r.loadResources, r.spinner.Tick) case SortMsg: // Handle sort command @@ -402,7 +413,7 @@ func (r *ResourceBrowser) Update(msg tea.Msg) (tea.Model, tea.Cmd) { case "ctrl+r": r.loading = true r.err = nil - return r, r.loadResources + return r, tea.Batch(r.loadResources, r.spinner.Tick) case "c": // Clear all filters (text filter and field filter) r.filterText = "" @@ -415,13 +426,8 @@ func (r *ResourceBrowser) Update(msg tea.Msg) (tea.Model, tea.Cmd) { case "d", "enter": if len(r.filtered) > 0 && r.table.Cursor() < len(r.filtered) { resource := r.filtered[r.table.Cursor()] - // Try to refresh resource via Get() for extended details - if d, err := r.registry.GetDAO(r.ctx, r.service, r.resourceType); err == nil { - if refreshed, err := d.Get(r.ctx, resource.GetID()); err == nil { - resource = refreshed - } - } - detailView := NewDetailView(r.ctx, resource, r.renderer, r.service, r.resourceType, r.registry) + // Pass DAO for async refresh in DetailView + detailView := NewDetailView(r.ctx, resource, r.renderer, r.service, r.resourceType, r.registry, r.dao) return r, func() tea.Msg { return NavigateMsg{View: detailView} } @@ -453,7 +459,7 @@ func (r *ResourceBrowser) Update(msg tea.Msg) (tea.Model, tea.Cmd) { r.loading = true r.filterText = "" r.filterInput.SetValue("") - return r, r.loadResources + return r, tea.Batch(r.loadResources, r.spinner.Tick) } case "N": // Manual next page load (useful when filter is active) @@ -462,6 +468,15 @@ func (r *ResourceBrowser) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return r, r.loadNextPage } } + + case spinner.TickMsg: + // Update spinner while loading + if r.loading { + var cmd tea.Cmd + r.spinner, cmd = r.spinner.Update(msg) + return r, cmd + } + return r, nil } var cmd tea.Cmd @@ -607,7 +622,7 @@ func (r *ResourceBrowser) buildTable() { func (r *ResourceBrowser) View() string { if r.loading { header := r.headerPanel.Render(r.service, r.resourceType, nil) - return header + "\n" + ui.DimStyle().Render("Loading...") + return header + "\n" + r.spinner.View() + " Loading..." } if r.err != nil { diff --git a/internal/view/view_test.go b/internal/view/view_test.go index 991bb5b6..c5efdd62 100644 --- a/internal/view/view_test.go +++ b/internal/view/view_test.go @@ -44,7 +44,7 @@ func TestDetailViewEsc(t *testing.T) { resource := &mockResource{id: "i-123", name: "test-instance"} ctx := context.Background() - dv := NewDetailView(ctx, resource, nil, "ec2", "instances", nil) + dv := NewDetailView(ctx, resource, nil, "ec2", "instances", nil, nil) dv.SetSize(100, 50) // Initialize viewport // Send esc to DetailView @@ -65,7 +65,7 @@ func TestDetailViewEscString(t *testing.T) { resource := &mockResource{id: "i-123", name: "test-instance"} ctx := context.Background() - dv := NewDetailView(ctx, resource, nil, "ec2", "instances", nil) + dv := NewDetailView(ctx, resource, nil, "ec2", "instances", nil, nil) dv.SetSize(100, 50) // Test that "esc" string is correctly identified From 0051963619bd4d0bda91a7d12d75fad7e6ab0336 Mon Sep 17 00:00:00 2001 From: "m@yim.jp" Date: Sun, 21 Dec 2025 05:37:58 +0000 Subject: [PATCH 2/6] fix: address review feedback - Log error on DetailView refresh failure - Add spinner.Tick to tab/shift+tab resource type switching --- internal/view/detail_view.go | 5 ++++- internal/view/resource_browser.go | 4 ++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/internal/view/detail_view.go b/internal/view/detail_view.go index 40007b89..0dcfc0c5 100644 --- a/internal/view/detail_view.go +++ b/internal/view/detail_view.go @@ -10,6 +10,7 @@ import ( "github.com/charmbracelet/lipgloss" "github.com/clawscli/claws/internal/action" "github.com/clawscli/claws/internal/dao" + "github.com/clawscli/claws/internal/log" "github.com/clawscli/claws/internal/registry" "github.com/clawscli/claws/internal/render" "github.com/clawscli/claws/internal/ui" @@ -107,7 +108,9 @@ func (d *DetailView) Update(msg tea.Msg) (tea.Model, tea.Cmd) { switch msg := msg.(type) { case detailRefreshMsg: d.refreshing = false - if msg.err == nil { + if msg.err != nil { + log.Warn("failed to refresh resource details", "error", msg.err) + } else { d.resource = msg.resource // Re-render content with refreshed data if d.ready { diff --git a/internal/view/resource_browser.go b/internal/view/resource_browser.go index 3b298d76..0f5959c8 100644 --- a/internal/view/resource_browser.go +++ b/internal/view/resource_browser.go @@ -446,11 +446,11 @@ func (r *ResourceBrowser) Update(msg tea.Msg) (tea.Model, tea.Cmd) { case "tab": // Cycle to next resource type r.cycleResourceType(1) - return r, r.loadResources + return r, tea.Batch(r.loadResources, r.spinner.Tick) case "shift+tab": // Cycle to previous resource type r.cycleResourceType(-1) - return r, r.loadResources + return r, tea.Batch(r.loadResources, r.spinner.Tick) case "1", "2", "3", "4", "5", "6", "7", "8", "9": // Switch to resource type by number idx := int(msg.String()[0] - '1') From 948a693fa3c5c379757a934ecacd2599611ac6a3 Mon Sep 17 00:00:00 2001 From: "m@yim.jp" Date: Sun, 21 Dec 2025 05:53:39 +0000 Subject: [PATCH 3/6] fix: address code review feedback for loading-ux - Add Supports(OpGet) guard before calling dao.Get() in DetailView - Extract IsEscKey() helper to eliminate ESC detection duplication - Add ui.NewSpinner() helper for consistent spinner styling - Fix incorrect comment on buildTable() function - Show 'AWS initializing...' in status bar during init - Remove unnecessary t.Logf in tests --- internal/app/app.go | 15 +++++++++++---- internal/ui/theme.go | 13 ++++++++++++- internal/view/detail_view.go | 17 +++++------------ internal/view/resource_browser.go | 14 +++----------- internal/view/service_browser.go | 16 +++++++++++++--- internal/view/view.go | 7 +++++++ internal/view/view_test.go | 1 - 7 files changed, 51 insertions(+), 32 deletions(-) diff --git a/internal/app/app.go b/internal/app/app.go index a0682829..5ed3f76d 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -74,6 +74,9 @@ type App struct { showWarnings bool warningsReady bool // true after first render, to ignore initial terminal responses + // AWS initialization state + awsInitializing bool + // Cached styles styles appStyles } @@ -94,6 +97,7 @@ func New(ctx context.Context, reg *registry.Registry) *App { func (a *App) Init() tea.Cmd { // Start with the service browser view immediately (no blocking on AWS calls) a.currentView = view.NewServiceBrowser(a.ctx, a.registry) + a.awsInitializing = true // Initialize AWS context in background (region detection, account ID fetch) initAWSCmd := func() tea.Msg { @@ -165,10 +169,7 @@ func (a *App) Update(msg tea.Msg) (tea.Model, tea.Cmd) { case tea.KeyMsg: // Handle back navigation (esc or backspace) - // Check for ESC key in various forms (KeyEsc, KeyEscape, or raw ESC byte as KeyRunes) - isEsc := msg.String() == "esc" || msg.Type == tea.KeyEsc || msg.Type == tea.KeyEscape || - (msg.Type == tea.KeyRunes && len(msg.Runes) == 1 && msg.Runes[0] == 27) - isBack := isEsc || msg.Type == tea.KeyBackspace + isBack := view.IsEscKey(msg) || msg.Type == tea.KeyBackspace if isBack { // If current view has active input, let it handle esc first @@ -262,6 +263,7 @@ func (a *App) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return a, nil case awsContextReadyMsg: + a.awsInitializing = false if msg.err != nil { config.Global().AddWarning("AWS init failed: " + msg.err.Error()) a.showWarnings = true @@ -375,6 +377,11 @@ func (a *App) View() string { statusContent = roIndicator + " " + statusContent } + // Add AWS initializing indicator + if a.awsInitializing { + statusContent = ui.DimStyle().Render("AWS initializing...") + " • " + statusContent + } + status := a.styles.status.Render(statusContent) return content + "\n" + status diff --git a/internal/ui/theme.go b/internal/ui/theme.go index 6f793deb..ba2821ad 100644 --- a/internal/ui/theme.go +++ b/internal/ui/theme.go @@ -1,6 +1,9 @@ package ui -import "github.com/charmbracelet/lipgloss" +import ( + "github.com/charmbracelet/bubbles/spinner" + "github.com/charmbracelet/lipgloss" +) // Theme defines the color scheme for the application type Theme struct { @@ -101,3 +104,11 @@ func WarningStyle() lipgloss.Style { func DangerStyle() lipgloss.Style { return lipgloss.NewStyle().Foreground(current.Danger) } + +// NewSpinner creates a consistently styled spinner for loading states +func NewSpinner() spinner.Model { + s := spinner.New() + s.Spinner = spinner.Dot + s.Style = lipgloss.NewStyle().Foreground(current.Accent) + return s +} diff --git a/internal/view/detail_view.go b/internal/view/detail_view.go index 0dcfc0c5..e1697851 100644 --- a/internal/view/detail_view.go +++ b/internal/view/detail_view.go @@ -56,11 +56,6 @@ func NewDetailView(ctx context.Context, resource dao.Resource, renderer render.R hp := NewHeaderPanel() hp.SetWidth(120) // Default width until SetSize is called - // Initialize spinner - s := spinner.New() - s.Spinner = spinner.Dot - s.Style = lipgloss.NewStyle().Foreground(ui.Current().Accent) - return &DetailView{ ctx: ctx, resource: resource, @@ -70,7 +65,7 @@ func NewDetailView(ctx context.Context, resource dao.Resource, renderer render.R registry: reg, dao: d, headerPanel: hp, - spinner: s, + spinner: ui.NewSpinner(), styles: newDetailViewStyles(), } } @@ -83,8 +78,8 @@ type detailRefreshMsg struct { // Init implements tea.Model func (d *DetailView) Init() tea.Cmd { - // Start async refresh for extended details if DAO is available - if d.dao != nil { + // Start async refresh for extended details if DAO supports Get operation + if d.dao != nil && d.dao.Supports(dao.OpGet) { d.refreshing = true return tea.Batch(d.spinner.Tick, d.refreshResource) } @@ -129,10 +124,8 @@ func (d *DetailView) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return d, nil case tea.KeyMsg: - // Check for esc (both string and raw byte) - let app handle back navigation - isEsc := msg.String() == "esc" || msg.Type == tea.KeyEsc || msg.Type == tea.KeyEscape || - (msg.Type == tea.KeyRunes && len(msg.Runes) == 1 && msg.Runes[0] == 27) - if isEsc { + // Let app handle back navigation + if IsEscKey(msg) { return d, nil } diff --git a/internal/view/resource_browser.go b/internal/view/resource_browser.go index 0f5959c8..9d5d6ee2 100644 --- a/internal/view/resource_browser.go +++ b/internal/view/resource_browser.go @@ -139,11 +139,6 @@ func newResourceBrowser(ctx context.Context, reg *registry.Registry, service, re hp := NewHeaderPanel() hp.SetWidth(120) // Default width until SetSize is called - // Initialize spinner - s := spinner.New() - s.Spinner = spinner.Dot - s.Style = lipgloss.NewStyle().Foreground(ui.Current().Accent) - return &ResourceBrowser{ ctx: ctx, registry: reg, @@ -153,7 +148,7 @@ func newResourceBrowser(ctx context.Context, reg *registry.Registry, service, re loading: true, filterInput: ti, headerPanel: hp, - spinner: s, + spinner: ui.NewSpinner(), styles: newResourceBrowserStyles(), pageSize: 100, // default page size for paginated resources sortColumn: -1, // no sorting by default @@ -371,10 +366,7 @@ func (r *ResourceBrowser) Update(msg tea.Msg) (tea.Model, tea.Cmd) { case tea.KeyMsg: // Handle filter mode if r.filterActive { - // Check for esc (both string and raw byte) - isEsc := msg.String() == "esc" || msg.Type == tea.KeyEsc || msg.Type == tea.KeyEscape || - (msg.Type == tea.KeyRunes && len(msg.Runes) == 1 && msg.Runes[0] == 27) - if isEsc { + if IsEscKey(msg) { r.filterActive = false r.filterInput.Blur() return r, nil @@ -542,7 +534,7 @@ func (r *ResourceBrowser) loadNextPage() tea.Msg { } } -// handleNavigation checks if a key matches a navigation shortcut +// buildTable rebuilds the table with current filtered resources func (r *ResourceBrowser) buildTable() { if r.renderer == nil { return diff --git a/internal/view/service_browser.go b/internal/view/service_browser.go index c71219a7..2c0af241 100644 --- a/internal/view/service_browser.go +++ b/internal/view/service_browser.go @@ -228,11 +228,13 @@ func (s *ServiceBrowser) rebuildFlatItems() { } func (s *ServiceBrowser) handleFilterInput(msg tea.KeyMsg) (tea.Model, tea.Cmd) { - switch msg.String() { - case "esc": + if IsEscKey(msg) { s.filterActive = false s.filterInput.Blur() return s, nil + } + + switch msg.String() { case "enter": s.filterActive = false @@ -297,7 +299,7 @@ func (s *ServiceBrowser) handleNavigation(msg tea.KeyMsg) (tea.Model, tea.Cmd) { s.filterInput.Focus() return s, textinput.Blink - case "c", "esc": + case "c": if s.filterText != "" { s.filterText = "" s.filterInput.SetValue("") @@ -306,6 +308,14 @@ func (s *ServiceBrowser) handleNavigation(msg tea.KeyMsg) (tea.Model, tea.Cmd) { } } + // Also allow esc to clear filter (handles various escape sequences) + if IsEscKey(msg) && s.filterText != "" { + s.filterText = "" + s.filterInput.SetValue("") + s.rebuildFlatItems() + s.cursor = 0 + } + // Update viewport content and scroll to cursor s.updateViewport() diff --git a/internal/view/view.go b/internal/view/view.go index 55888a25..8684ee69 100644 --- a/internal/view/view.go +++ b/internal/view/view.go @@ -74,6 +74,13 @@ type Refreshable interface { CanRefresh() bool } +// IsEscKey returns true if the key message represents an escape key press. +// This handles various terminal escape sequences consistently across views. +func IsEscKey(msg tea.KeyMsg) bool { + return msg.String() == "esc" || msg.Type == tea.KeyEsc || msg.Type == tea.KeyEscape || + (msg.Type == tea.KeyRunes && len(msg.Runes) == 1 && msg.Runes[0] == 27) +} + // NavigationHelper provides common navigation functionality type NavigationHelper struct { Ctx context.Context diff --git a/internal/view/view_test.go b/internal/view/view_test.go index c5efdd62..a34d38f2 100644 --- a/internal/view/view_test.go +++ b/internal/view/view_test.go @@ -70,7 +70,6 @@ func TestDetailViewEscString(t *testing.T) { // Test that "esc" string is correctly identified escMsg := tea.KeyMsg{Type: tea.KeyEsc} - t.Logf("Esc key string: %q", escMsg.String()) if escMsg.String() != "esc" { t.Errorf("Expected esc key String() to be 'esc', got %q", escMsg.String()) From c1cebceaacd251b3328b502505e81e8126b3db77 Mon Sep 17 00:00:00 2001 From: "m@yim.jp" Date: Sun, 21 Dec 2025 06:03:02 +0000 Subject: [PATCH 4/6] fix: show refresh errors in DetailView status line + add tests - Add refreshErr field to DetailView, display in status line - Verify ServiceBrowser handles empty accountID (already ok) - Add tests for IsEscKey, NewSpinner, DetailView refresh/init --- internal/ui/theme_test.go | 21 +++++ internal/view/detail_view.go | 5 ++ internal/view/view_test.go | 151 +++++++++++++++++++++++++++++++++++ 3 files changed, 177 insertions(+) diff --git a/internal/ui/theme_test.go b/internal/ui/theme_test.go index 29026786..70634a3b 100644 --- a/internal/ui/theme_test.go +++ b/internal/ui/theme_test.go @@ -96,6 +96,27 @@ func TestDangerStyle(t *testing.T) { } } +func TestNewSpinner(t *testing.T) { + s := NewSpinner() + + // Spinner should be initialized + if s.Spinner.Frames == nil { + t.Error("NewSpinner() should have spinner frames") + } + + // Should use Dot spinner (has specific frame count) + // spinner.Dot has 10 frames + if len(s.Spinner.Frames) == 0 { + t.Error("NewSpinner() should have non-empty frames") + } + + // View should produce output + view := s.View() + if view == "" { + t.Error("NewSpinner().View() should produce output") + } +} + func TestThemeFields(t *testing.T) { theme := DefaultTheme() diff --git a/internal/view/detail_view.go b/internal/view/detail_view.go index e1697851..9e7d346c 100644 --- a/internal/view/detail_view.go +++ b/internal/view/detail_view.go @@ -47,6 +47,7 @@ type DetailView struct { registry *registry.Registry dao dao.DAO // for async refresh refreshing bool // true while fetching extended details + refreshErr error // error from last refresh attempt spinner spinner.Model styles detailViewStyles } @@ -105,7 +106,9 @@ func (d *DetailView) Update(msg tea.Msg) (tea.Model, tea.Cmd) { d.refreshing = false if msg.err != nil { log.Warn("failed to refresh resource details", "error", msg.err) + d.refreshErr = msg.err } else { + d.refreshErr = nil d.resource = msg.resource // Re-render content with refreshed data if d.ready { @@ -231,6 +234,8 @@ func (d *DetailView) StatusLine() string { if d.refreshing { parts = append(parts, d.spinner.View()+" refreshing...") + } else if d.refreshErr != nil { + parts = append(parts, "⚠ refresh failed") } parts = append(parts, "↑/↓:scroll") diff --git a/internal/view/view_test.go b/internal/view/view_test.go index a34d38f2..828b9ce4 100644 --- a/internal/view/view_test.go +++ b/internal/view/view_test.go @@ -2,6 +2,8 @@ package view import ( "context" + "fmt" + "strings" "testing" tea "github.com/charmbracelet/bubbletea" @@ -506,6 +508,155 @@ func TestCommandInput_Update_Enter_Service(t *testing.T) { } } +// IsEscKey tests + +func TestIsEscKey(t *testing.T) { + tests := []struct { + name string + msg tea.KeyMsg + want bool + }{ + {"KeyEsc", tea.KeyMsg{Type: tea.KeyEsc}, true}, + {"KeyEscape", tea.KeyMsg{Type: tea.KeyEscape}, true}, + {"raw ESC byte", tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{27}}, true}, + {"Enter", tea.KeyMsg{Type: tea.KeyEnter}, false}, + {"Space", tea.KeyMsg{Type: tea.KeySpace}, false}, + {"letter a", tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'a'}}, false}, + {"letter q", tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'q'}}, false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := IsEscKey(tt.msg) + if got != tt.want { + t.Errorf("IsEscKey() = %v, want %v", got, tt.want) + } + }) + } +} + +// DetailView async refresh tests + +func TestDetailViewRefreshError(t *testing.T) { + resource := &mockResource{id: "i-123", name: "test-instance"} + ctx := context.Background() + + dv := NewDetailView(ctx, resource, nil, "ec2", "instances", nil, nil) + dv.SetSize(100, 50) + + // Simulate refresh error + errMsg := detailRefreshMsg{ + resource: resource, + err: fmt.Errorf("access denied"), + } + + dv.Update(errMsg) + + // Check that error is stored + if dv.refreshErr == nil { + t.Error("Expected refreshErr to be set after error message") + } + + // Check status line contains error indicator + status := dv.StatusLine() + if !strings.Contains(status, "refresh failed") { + t.Errorf("StatusLine() = %q, want to contain 'refresh failed'", status) + } +} + +func TestDetailViewRefreshSuccess(t *testing.T) { + resource := &mockResource{id: "i-123", name: "test-instance"} + ctx := context.Background() + + dv := NewDetailView(ctx, resource, nil, "ec2", "instances", nil, nil) + dv.SetSize(100, 50) + + // Set an initial error + dv.refreshErr = fmt.Errorf("previous error") + + // Simulate successful refresh + newResource := &mockResource{id: "i-123", name: "updated-instance"} + successMsg := detailRefreshMsg{ + resource: newResource, + err: nil, + } + + dv.Update(successMsg) + + // Error should be cleared + if dv.refreshErr != nil { + t.Error("Expected refreshErr to be nil after successful refresh") + } + + // Resource should be updated + if dv.resource.GetName() != "updated-instance" { + t.Errorf("resource.GetName() = %q, want 'updated-instance'", dv.resource.GetName()) + } +} + +// mockDAO for testing +type mockDAO struct { + dao.BaseDAO + supportsGet bool + getErr error +} + +func (m *mockDAO) List(ctx context.Context) ([]dao.Resource, error) { + return nil, nil +} + +func (m *mockDAO) Get(ctx context.Context, id string) (dao.Resource, error) { + if m.getErr != nil { + return nil, m.getErr + } + return &mockResource{id: id, name: "fetched"}, nil +} + +func (m *mockDAO) Delete(ctx context.Context, id string) error { + return nil +} + +func (m *mockDAO) Supports(op dao.Operation) bool { + if op == dao.OpGet { + return m.supportsGet + } + return true +} + +func TestDetailViewInitWithSupportsGet(t *testing.T) { + resource := &mockResource{id: "i-123", name: "test"} + ctx := context.Background() + + // DAO that supports Get + daoWithGet := &mockDAO{supportsGet: true} + dv := NewDetailView(ctx, resource, nil, "ec2", "instances", nil, daoWithGet) + + cmd := dv.Init() + if cmd == nil { + t.Error("Expected Init() to return command when DAO supports Get") + } + if !dv.refreshing { + t.Error("Expected refreshing to be true when DAO supports Get") + } +} + +func TestDetailViewInitWithoutSupportsGet(t *testing.T) { + resource := &mockResource{id: "i-123", name: "test"} + ctx := context.Background() + + // DAO that doesn't support Get + daoWithoutGet := &mockDAO{supportsGet: false} + dv := NewDetailView(ctx, resource, nil, "ec2", "instances", nil, daoWithoutGet) + + cmd := dv.Init() + if cmd != nil { + t.Error("Expected Init() to return nil when DAO doesn't support Get") + } + if dv.refreshing { + t.Error("Expected refreshing to be false when DAO doesn't support Get") + } +} + // HelpView tests func TestHelpView_New(t *testing.T) { From a56a58c1a4a9cc64af94ca51e049c7386859685f Mon Sep 17 00:00:00 2001 From: "m@yim.jp" Date: Sun, 21 Dec 2025 06:12:25 +0000 Subject: [PATCH 5/6] fix: add 5s timeout for AWS init + nil check for resource - AWS InitContext now has 5s timeout (awsInitTimeout const) to avoid hang - Add defensive nil check for d.resource in refreshResource() --- internal/app/app.go | 8 +++++++- internal/view/detail_view.go | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/internal/app/app.go b/internal/app/app.go index 5ed3f76d..7b3ed635 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -18,6 +18,9 @@ import ( "github.com/clawscli/claws/internal/view" ) +// awsInitTimeout is the maximum time to wait for AWS context initialization +const awsInitTimeout = 5 * time.Second + // clearErrorMsg is sent to clear transient errors after a timeout type clearErrorMsg struct{} @@ -100,8 +103,11 @@ func (a *App) Init() tea.Cmd { a.awsInitializing = true // Initialize AWS context in background (region detection, account ID fetch) + // Use timeout to avoid indefinite hang on network issues initAWSCmd := func() tea.Msg { - err := aws.InitContext(a.ctx) + ctx, cancel := context.WithTimeout(a.ctx, awsInitTimeout) + defer cancel() + err := aws.InitContext(ctx) return awsContextReadyMsg{err: err} } diff --git a/internal/view/detail_view.go b/internal/view/detail_view.go index 9e7d346c..c6ccca51 100644 --- a/internal/view/detail_view.go +++ b/internal/view/detail_view.go @@ -89,7 +89,7 @@ func (d *DetailView) Init() tea.Cmd { // refreshResource fetches extended resource details in background func (d *DetailView) refreshResource() tea.Msg { - if d.dao == nil { + if d.dao == nil || d.resource == nil { return detailRefreshMsg{resource: d.resource} } refreshed, err := d.dao.Get(d.ctx, d.resource.GetID()) From 266931db200da41353a1a94d8436741d6d44c6a6 Mon Sep 17 00:00:00 2001 From: "m@yim.jp" Date: Sun, 21 Dec 2025 06:24:44 +0000 Subject: [PATCH 6/6] fix: add debug log, tests, and comment for review feedback - Add debug log when AWS init fails - Add tests for awsContextReadyMsg success/timeout - Add comment explaining rune 27 (raw ESC byte) check --- internal/app/app.go | 1 + internal/app/app_test.go | 38 ++++++++++++++++++++++++++++++++++++++ internal/view/view.go | 2 ++ 3 files changed, 41 insertions(+) diff --git a/internal/app/app.go b/internal/app/app.go index 7b3ed635..b9bff5fc 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -271,6 +271,7 @@ func (a *App) Update(msg tea.Msg) (tea.Model, tea.Cmd) { case awsContextReadyMsg: a.awsInitializing = false if msg.err != nil { + log.Debug("AWS context initialization failed", "error", msg.err) config.Global().AddWarning("AWS init failed: " + msg.err.Error()) a.showWarnings = true } diff --git a/internal/app/app_test.go b/internal/app/app_test.go index 8747b901..526c122f 100644 --- a/internal/app/app_test.go +++ b/internal/app/app_test.go @@ -197,3 +197,41 @@ func TestNavigationFlow(t *testing.T) { t.Errorf("Expected viewStack length 0, got %d", len(app.viewStack)) } } + +func TestAWSContextReadyMsg_Success(t *testing.T) { + ctx := context.Background() + reg := registry.New() + + app := New(ctx, reg) + app.awsInitializing = true + + // Simulate successful AWS init + msg := awsContextReadyMsg{err: nil} + app.Update(msg) + + if app.awsInitializing { + t.Error("Expected awsInitializing to be false after success") + } + if app.showWarnings { + t.Error("Expected showWarnings to be false after success") + } +} + +func TestAWSContextReadyMsg_Timeout(t *testing.T) { + ctx := context.Background() + reg := registry.New() + + app := New(ctx, reg) + app.awsInitializing = true + + // Simulate timeout error + msg := awsContextReadyMsg{err: context.DeadlineExceeded} + app.Update(msg) + + if app.awsInitializing { + t.Error("Expected awsInitializing to be false after timeout") + } + if !app.showWarnings { + t.Error("Expected showWarnings to be true after timeout") + } +} diff --git a/internal/view/view.go b/internal/view/view.go index 8684ee69..4700a9b0 100644 --- a/internal/view/view.go +++ b/internal/view/view.go @@ -76,6 +76,8 @@ type Refreshable interface { // IsEscKey returns true if the key message represents an escape key press. // This handles various terminal escape sequences consistently across views. +// We check for rune 27 (raw ESC byte) because some terminals send ESC as a raw +// byte rather than a recognized key type. func IsEscKey(msg tea.KeyMsg) bool { return msg.String() == "esc" || msg.Type == tea.KeyEsc || msg.Type == tea.KeyEscape || (msg.Type == tea.KeyRunes && len(msg.Runes) == 1 && msg.Runes[0] == 27)