From 408681c8e4c8f878849e56bac3bec9f670cd389e Mon Sep 17 00:00:00 2001 From: swanadiva Date: Mon, 29 Jun 2026 11:50:46 +0700 Subject: [PATCH] =?UTF-8?q?feat:=20Phase=202=20UX=20Polish=20=E2=80=94=20t?= =?UTF-8?q?heme=20system,=20config=20profiles,=20error=20banner,=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add Theme struct with 3 predefined themes (dark/light/druntime) - Add Profile struct for named configuration profiles - Add ErrorBanner with severity levels and auto-dismiss - Add unit tests for theme, error banner, and models - Update CHANGELOG.md and PROJECT_STATE.md --- CHANGELOG.md | 28 +++++++ PROJECT_STATE.md | 6 +- internal/models/models.go | 59 +++++++++++++- pkg/tui/error_banner.go | 137 +++++++++++++++++++++++++++++++ pkg/tui/themes.go | 148 ++++++++++++++++++++++++++++++++++ test/models_test.go | 80 ++++++++++++++++++ test/tui/error_banner_test.go | 95 ++++++++++++++++++++++ test/tui/theme_test.go | 71 ++++++++++++++++ 8 files changed, 617 insertions(+), 7 deletions(-) create mode 100644 pkg/tui/error_banner.go create mode 100644 pkg/tui/themes.go create mode 100644 test/models_test.go create mode 100644 test/tui/error_banner_test.go create mode 100644 test/tui/theme_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index d760e36..955eb5d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -148,3 +148,31 @@ ### Models Updated - `AppConfig`: added `EncryptionEnabled`, `PasswordHash`, `KnownHostsFile` + +## Phase 2 UX Polish (Done) + +### Theme System +- **NEW `themes.go`**: `Theme` struct with full color palette +- 3 predefined themes: `dark` (Gruvbox), `light`, `dracula` +- `SetTheme(name)` — updates all component styles at runtime +- `GetActiveTheme()` — returns current theme +- All palette vars now derived from active theme + +### Config Profiles +- **`models.go`**: `Profile` struct — name, theme, default group, default auth, editor, notes +- `AppConfig.Profiles []Profile` — list of named profiles +- `AppConfig.ActiveProfile string` — currently active profile +- `GetProfile(name)`, `GetActiveProfile()`, `AddProfile()`, `RemoveProfile()` +- Default config includes "default" profile + +### Enhanced Error Display +- **NEW `error_banner.go`**: `ErrorBanner` with severity levels (Error/Warning/Info) +- Structured format: title + detail + numbered hints +- Auto-dismiss after 5 seconds (configurable) +- Color-coded: red (error), yellow (warning), blue (info) +- `Show()`, `Hide()`, `Update()`, `View(width)` methods + +### Tests +- **`test/tui/theme_test.go`**: GetTheme, SetTheme, GetActiveTheme, registry +- **`test/tui/error_banner_test.go`**: Show/Hide, AutoDismiss, Severity, View +- **`test/models_test.go`**: DefaultConfig, Profile CRUD diff --git a/PROJECT_STATE.md b/PROJECT_STATE.md index 54a5ee0..9609246 100644 --- a/PROJECT_STATE.md +++ b/PROJECT_STATE.md @@ -2,9 +2,9 @@ > **Purpose**: Enable seamless continuation of development by any agent/LLM across sessions > -> **Last Updated**: 2025-01-31 (SFTP Polish + File Transfer Planning) -> **Current Status**: ✅ MVP Complete — Phase 1 Done, SFTP Responsive + Polish Done -> **Phase**: Phase 2 (Security + UX Polish) +> **Last Updated**: 2025-01-31 (Phase 2 UX Polish Complete) +> **Current Status**: ✅ MVP Complete — Phase 1 Done, Phase 2 Done +> **Phase**: Phase 3 (Testing + Documentation) --- diff --git a/internal/models/models.go b/internal/models/models.go index a9c3b6d..277953b 100644 --- a/internal/models/models.go +++ b/internal/models/models.go @@ -50,6 +50,16 @@ type Snippet struct { UpdatedAt time.Time `json:"updated_at"` } +// Profile represents a named configuration profile +type Profile struct { + Name string `json:"name"` + Theme string `json:"theme"` + DefaultGroup string `json:"default_group,omitempty"` + DefaultAuth string `json:"default_auth,omitempty"` // "password", "key", "both" + Editor string `json:"editor,omitempty"` + Notes string `json:"notes,omitempty"` +} + // AppConfig represents the application configuration type AppConfig struct { Version string `json:"version"` @@ -60,6 +70,10 @@ type AppConfig struct { AutoSync bool `json:"auto_sync"` SyncProvider string `json:"sync_provider,omitempty"` + // Profiles + Profiles []Profile `json:"profiles,omitempty"` + ActiveProfile string `json:"active_profile,omitempty"` + // Security EncryptionEnabled bool `json:"encryption_enabled"` PasswordHash string `json:"password_hash,omitempty"` // SHA-256 hash for verification @@ -68,10 +82,10 @@ type AppConfig struct { // KnownHost represents a verified host key type KnownHost struct { - Hostname string `json:"hostname"` - Port int `json:"port"` - KeyType string `json:"key_type"` // "ssh-rsa", "ssh-ed25519", etc. - KeyHash string `json:"key_hash"` // Base64-encoded host key + Hostname string `json:"hostname"` + Port int `json:"port"` + KeyType string `json:"key_type"` // "ssh-rsa", "ssh-ed25519", etc. + KeyHash string `json:"key_hash"` // Base64-encoded host key AddedAt time.Time `json:"added_at"` } @@ -85,5 +99,42 @@ func DefaultConfig() *AppConfig { Editor: "vim", AutoSync: false, EncryptionEnabled: false, + Profiles: []Profile{ + { + Name: "default", + Theme: "dark", + }, + }, + ActiveProfile: "default", + } +} + +// GetProfile returns a profile by name +func (c *AppConfig) GetProfile(name string) *Profile { + for i := range c.Profiles { + if c.Profiles[i].Name == name { + return &c.Profiles[i] + } + } + return nil +} + +// GetActiveProfile returns the active profile +func (c *AppConfig) GetActiveProfile() *Profile { + return c.GetProfile(c.ActiveProfile) +} + +// AddProfile adds a new profile +func (c *AppConfig) AddProfile(p Profile) { + c.Profiles = append(c.Profiles, p) +} + +// RemoveProfile removes a profile by name +func (c *AppConfig) RemoveProfile(name string) { + for i := range c.Profiles { + if c.Profiles[i].Name == name { + c.Profiles = append(c.Profiles[:i], c.Profiles[i+1:]...) + return + } } } \ No newline at end of file diff --git a/pkg/tui/error_banner.go b/pkg/tui/error_banner.go new file mode 100644 index 0000000..074140c --- /dev/null +++ b/pkg/tui/error_banner.go @@ -0,0 +1,137 @@ +package tui + +import ( + "fmt" + "strings" + "time" + + "github.com/charmbracelet/lipgloss" +) + +// ErrorSeverity indicates the level of an error message +type ErrorSeverity int + +const ( + SevError ErrorSeverity = iota + SevWarning + SevInfo +) + +// ErrorBanner displays a structured error message with title, details, and hints +type ErrorBanner struct { + Title string + Detail string + Hints []string + Severity ErrorSeverity + AutoDismiss bool + DismissAfter time.Duration + createdAt time.Time + visible bool +} + +// NewErrorBanner creates a new error banner with the given severity +func NewErrorBanner(severity ErrorSeverity) *ErrorBanner { + return &ErrorBanner{ + Severity: severity, + AutoDismiss: true, + DismissAfter: 5 * time.Second, + visible: false, + } +} + +// Show displays the error banner with the given message +func (b *ErrorBanner) Show(title, detail string, hints ...string) { + b.Title = title + b.Detail = detail + b.Hints = hints + b.visible = true + b.createdAt = time.Now() +} + +// Hide hides the error banner +func (b *ErrorBanner) Hide() { + b.visible = false +} + +// IsVisible returns whether the banner is currently visible +func (b *ErrorBanner) IsVisible() bool { + return b.visible +} + +// Update checks if auto-dismiss time has elapsed +func (b *ErrorBanner) Update() { + if b.visible && b.AutoDismiss && time.Since(b.createdAt) > b.DismissAfter { + b.visible = false + } +} + +// View renders the error banner +func (b *ErrorBanner) View(width int) string { + if !b.visible { + return "" + } + + var ( + titleStyle lipgloss.Style + borderColor lipgloss.Color + prefix string + ) + + switch b.Severity { + case SevError: + titleStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("#ea6962")).Bold(true) + borderColor = lipgloss.Color("#ea6962") + prefix = "✖" + case SevWarning: + titleStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("#d8a657")).Bold(true) + borderColor = lipgloss.Color("#d8a657") + prefix = "⚠" + case SevInfo: + titleStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("#7daea3")).Bold(true) + borderColor = lipgloss.Color("#7daea3") + prefix = "ℹ" + } + + var sb strings.Builder + + // Title line with prefix + sb.WriteString(titleStyle.Render(fmt.Sprintf("%s %s", prefix, b.Title))) + + // Detail line + if b.Detail != "" { + sb.WriteString("\n") + sb.WriteString(strings.Repeat(" ", len(prefix)+1)) + detailStyle := lipgloss.NewStyle().Foreground(activeTheme.Fg) + sb.WriteString(detailStyle.Render(b.Detail)) + } + + // Hints + if len(b.Hints) > 0 { + sb.WriteString("\n") + sb.WriteString(strings.Repeat(" ", len(prefix)+1)) + hintStyle := lipgloss.NewStyle().Foreground(activeTheme.FgMute) + sb.WriteString(hintStyle.Render("Hints:")) + for i, hint := range b.Hints { + sb.WriteString("\n") + sb.WriteString(strings.Repeat(" ", len(prefix)+2)) + sb.WriteString(hintStyle.Render(fmt.Sprintf("%d. %s", i+1, hint))) + } + } + + // Wrap in a styled box + borderStyle := lipgloss.NewStyle(). + Border(lipgloss.RoundedBorder()). + BorderForeground(borderColor). + Padding(0, 1). + Width(min(width-2, 80)) + + return borderStyle.Render(sb.String()) +} + +// min returns the smaller of two integers +func min(a, b int) int { + if a < b { + return a + } + return b +} diff --git a/pkg/tui/themes.go b/pkg/tui/themes.go new file mode 100644 index 0000000..26f04ec --- /dev/null +++ b/pkg/tui/themes.go @@ -0,0 +1,148 @@ +package tui + +import "github.com/charmbracelet/lipgloss" + +// Theme defines a complete color palette for the TUI +type Theme struct { + Name string + Fg lipgloss.Color + FgMute lipgloss.Color + Bg lipgloss.Color + BgSel lipgloss.Color + Red lipgloss.Color + Orange lipgloss.Color + Yellow lipgloss.Color + Green lipgloss.Color + Aqua lipgloss.Color + Blue lipgloss.Color + Purple lipgloss.Color + Border lipgloss.Color + TabBg lipgloss.Color +} + +// Predefined themes +var ( + ThemeDark = Theme{ + Name: "dark", + Fg: lipgloss.Color("#d4be98"), + FgMute: lipgloss.Color("#7c6f64"), + Bg: lipgloss.Color("#1d2021"), + BgSel: lipgloss.Color("#45403d"), + Red: lipgloss.Color("#ea6962"), + Orange: lipgloss.Color("#e78a4e"), + Yellow: lipgloss.Color("#d8a657"), + Green: lipgloss.Color("#a9b665"), + Aqua: lipgloss.Color("#89b482"), + Blue: lipgloss.Color("#7daea3"), + Purple: lipgloss.Color("#d3869b"), + Border: lipgloss.Color("#504945"), + TabBg: lipgloss.Color("#1d2021"), + } + + ThemeLight = Theme{ + Name: "light", + Fg: lipgloss.Color("#3c3836"), + FgMute: lipgloss.Color("#7c6f64"), + Bg: lipgloss.Color("#f2e5bc"), + BgSel: lipgloss.Color("#d5c4a1"), + Red: lipgloss.Color("#cc241d"), + Orange: lipgloss.Color("#d65d0e"), + Yellow: lipgloss.Color("#d79921"), + Green: lipgloss.Color("#98971a"), + Aqua: lipgloss.Color("#689d6a"), + Blue: lipgloss.Color("#458588"), + Purple: lipgloss.Color("#b16286"), + Border: lipgloss.Color("#a89984"), + TabBg: lipgloss.Color("#f2e5bc"), + } + + ThemeDracula = Theme{ + Name: "dracula", + Fg: lipgloss.Color("#f8f8f2"), + FgMute: lipgloss.Color("#6272a4"), + Bg: lipgloss.Color("#282a36"), + BgSel: lipgloss.Color("#44475a"), + Red: lipgloss.Color("#ff5555"), + Orange: lipgloss.Color("#ffb86c"), + Yellow: lipgloss.Color("#f1fa8c"), + Green: lipgloss.Color("#50fa7b"), + Aqua: lipgloss.Color("#8be9fd"), + Blue: lipgloss.Color("#6272a4"), + Purple: lipgloss.Color("#bd93f9"), + Border: lipgloss.Color("#44475a"), + TabBg: lipgloss.Color("#282a36"), + } +) + +// Themes is the registry of all available themes +var Themes = map[string]Theme{ + "dark": ThemeDark, + "light": ThemeLight, + "dracula": ThemeDracula, +} + +// activeTheme holds the currently active theme +var activeTheme = ThemeDark + +// GetTheme returns a theme by name, defaults to dark +func GetTheme(name string) Theme { + if t, ok := Themes[name]; ok { + return t + } + return ThemeDark +} + +// SetTheme applies a theme by name and updates all component styles +func SetTheme(name string) { + theme := GetTheme(name) + activeTheme = theme + applyTheme(theme) +} + +// GetActiveTheme returns the currently active theme +func GetActiveTheme() Theme { + return activeTheme +} + +// applyTheme updates all component styles from the given theme +func applyTheme(t Theme) { + // Palette aliases + gbFg = t.Fg + gbFgMute = t.FgMute + gbBgSel = t.BgSel + gbRed = t.Red + gbOrange = t.Orange + gbYellow = t.Yellow + gbGreen = t.Green + gbAqua = t.Aqua + gbBlue = t.Blue + gbPurple = t.Purple + gbBorder = t.Border + + // Component styles + TabActiveStyle = lipgloss.NewStyle().Background(t.Yellow).Foreground(t.Bg).Bold(true).Padding(0, 2) + TabInactiveStyle = lipgloss.NewStyle().Background(t.Border).Foreground(t.FgMute).Padding(0, 2) + TabBarStyle = lipgloss.NewStyle().Background(t.TabBg) + StatusBarStyle = lipgloss.NewStyle().Background(t.Green).Foreground(t.Bg).Padding(0, 1) + AppTitleStyle = lipgloss.NewStyle().Foreground(t.Yellow).Bold(true) + HighlightStyle = lipgloss.NewStyle().Foreground(t.Orange).Bold(true) + SelectedStyle = lipgloss.NewStyle().Foreground(t.Fg).Background(t.BgSel).Bold(true).Padding(0, 1) + ErrorStyle = lipgloss.NewStyle().Foreground(t.Red).Bold(true) + SuccessStyle = lipgloss.NewStyle().Foreground(t.Green).Bold(true) + InfoStyle = lipgloss.NewStyle().Foreground(t.Aqua) + SubtitleStyle = lipgloss.NewStyle().Foreground(t.FgMute) + HostNameStyle = lipgloss.NewStyle().Foreground(t.Yellow).Bold(true) + HostDetailStyle = lipgloss.NewStyle().Foreground(t.FgMute) + TagStyle = lipgloss.NewStyle().Foreground(t.Green) + TitleStyle = AppTitleStyle + SectionStyle = lipgloss.NewStyle().Foreground(t.Orange).Bold(true) + BorderStyle = lipgloss.NewStyle().Border(lipgloss.RoundedBorder()).BorderForeground(t.Border).Padding(1, 2) + + // Pane styles + StylePaneActive = lipgloss.NewStyle().Border(lipgloss.RoundedBorder()).BorderForeground(t.Green).Padding(0, 1) + StylePaneInactive = lipgloss.NewStyle().Border(lipgloss.RoundedBorder()).BorderForeground(t.Border).Padding(0, 1) + + // Host card styles + HostCardStyle = lipgloss.NewStyle().Border(lipgloss.RoundedBorder()).BorderForeground(t.Border).Padding(0, 1) + HostCardActiveStyle = lipgloss.NewStyle().Border(lipgloss.RoundedBorder()).BorderForeground(t.Green).Padding(0, 1) +} diff --git a/test/models_test.go b/test/models_test.go new file mode 100644 index 0000000..944c2a4 --- /dev/null +++ b/test/models_test.go @@ -0,0 +1,80 @@ +package errors_test + +import ( + "testing" + + "git.tukangketik.id/swanadiva/hostkeeper/internal/models" +) + +func TestDefaultConfig(t *testing.T) { + config := models.DefaultConfig() + + if config.Version != "1.0.0" { + t.Errorf("Version = %q, want %q", config.Version, "1.0.0") + } + if config.DefaultPort != 22 { + t.Errorf("DefaultPort = %d, want 22", config.DefaultPort) + } + if config.ConnectionTimeout != 30 { + t.Errorf("ConnectionTimeout = %d, want 30", config.ConnectionTimeout) + } + if config.Theme != "dark" { + t.Errorf("Theme = %q, want %q", config.Theme, "dark") + } + if len(config.Profiles) != 1 { + t.Errorf("Profiles has %d items, want 1", len(config.Profiles)) + } + if config.ActiveProfile != "default" { + t.Errorf("ActiveProfile = %q, want %q", config.ActiveProfile, "default") + } +} + +func TestAppConfigProfiles(t *testing.T) { + config := models.DefaultConfig() + + // Test GetProfile + profile := config.GetProfile("default") + if profile == nil { + t.Fatal("GetProfile(default) returned nil") + } + if profile.Name != "default" { + t.Errorf("Profile.Name = %q, want %q", profile.Name, "default") + } + + // Test GetProfile for non-existent profile + profile = config.GetProfile("nonexistent") + if profile != nil { + t.Error("GetProfile(nonexistent) should return nil") + } + + // Test GetActiveProfile + profile = config.GetActiveProfile() + if profile == nil { + t.Fatal("GetActiveProfile() returned nil") + } + if profile.Name != "default" { + t.Errorf("Active profile Name = %q, want %q", profile.Name, "default") + } + + // Test AddProfile + newProfile := models.Profile{ + Name: "work", + Theme: "light", + } + config.AddProfile(newProfile) + if len(config.Profiles) != 2 { + t.Errorf("After AddProfile, Profiles has %d items, want 2", len(config.Profiles)) + } + + // Test RemoveProfile + config.RemoveProfile("work") + if len(config.Profiles) != 1 { + t.Errorf("After RemoveProfile, Profiles has %d items, want 1", len(config.Profiles)) + } + + // Test RemoveProfile for non-existent profile + config.RemoveProfile("nonexistent") + if len(config.Profiles) != 1 { + t.Errorf("After RemoveProfile(nonexistent), Profiles has %d items, want 1", len(config.Profiles)) + } +} diff --git a/test/tui/error_banner_test.go b/test/tui/error_banner_test.go new file mode 100644 index 0000000..a39ee95 --- /dev/null +++ b/test/tui/error_banner_test.go @@ -0,0 +1,95 @@ +package tui_test + +import ( + "testing" + "time" + + "git.tukangketik.id/swanadiva/hostkeeper/pkg/tui" +) + +func TestErrorBannerShowHide(t *testing.T) { + banner := tui.NewErrorBanner(tui.SevError) + + // Initially not visible + if banner.IsVisible() { + t.Error("Banner should not be visible initially") + } + + // Show the banner + banner.Show("Test Error", "Something went wrong", "Check logs", "Restart app") + if !banner.IsVisible() { + t.Error("Banner should be visible after Show()") + } + + // Verify content + if banner.Title != "Test Error" { + t.Errorf("Title = %q, want %q", banner.Title, "Test Error") + } + if banner.Detail != "Something went wrong" { + t.Errorf("Detail = %q, want %q", banner.Detail, "Something went wrong") + } + if len(banner.Hints) != 2 { + t.Errorf("Hints has %d items, want 2", len(banner.Hints)) + } + + // Hide the banner + banner.Hide() + if banner.IsVisible() { + t.Error("Banner should not be visible after Hide()") + } +} + +func TestErrorBannerAutoDismiss(t *testing.T) { + banner := tui.NewErrorBanner(tui.SevWarning) + banner.AutoDismiss = true + banner.DismissAfter = 100 * time.Millisecond + + banner.Show("Test Warning", "Something") + if !banner.IsVisible() { + t.Error("Banner should be visible after Show()") + } + + // Wait for auto-dismiss + time.Sleep(150 * time.Millisecond) + banner.Update() + + if banner.IsVisible() { + t.Error("Banner should be auto-dismissed after delay") + } +} + +func TestErrorBannerSeverity(t *testing.T) { + tests := []struct { + severity tui.ErrorSeverity + name string + }{ + {tui.SevError, "error"}, + {tui.SevWarning, "warning"}, + {tui.SevInfo, "info"}, + } + + for _, tt := range tests { + banner := tui.NewErrorBanner(tt.severity) + if banner.Severity != tt.severity { + t.Errorf("NewErrorBanner(%v).Severity = %v, want %v", tt.name, banner.Severity, tt.severity) + } + } +} + +func TestErrorBannerView(t *testing.T) { + banner := tui.NewErrorBanner(tui.SevError) + banner.Show("Error Title", "Error detail", "Hint 1", "Hint 2") + + // Test that View returns non-empty string + output := banner.View(80) + if output == "" { + t.Error("View() returned empty string") + } + + // Test that View returns empty string when not visible + banner.Hide() + output = banner.View(80) + if output != "" { + t.Error("View() should return empty string when not visible") + } +} diff --git a/test/tui/theme_test.go b/test/tui/theme_test.go new file mode 100644 index 0000000..4d1e790 --- /dev/null +++ b/test/tui/theme_test.go @@ -0,0 +1,71 @@ +package tui_test + +import ( + "testing" + + "git.tukangketik.id/swanadiva/hostkeeper/pkg/tui" +) + +func TestGetTheme(t *testing.T) { + // Test getting existing themes + tests := []struct { + name string + expected string + }{ + {"dark", "dark"}, + {"light", "light"}, + {"dracula", "dracula"}, + } + + for _, tt := range tests { + theme := tui.GetTheme(tt.name) + if theme.Name != tt.expected { + t.Errorf("GetTheme(%q) = %q, want %q", tt.name, theme.Name, tt.expected) + } + } + + // Test getting non-existing theme defaults to dark + theme := tui.GetTheme("nonexistent") + if theme.Name != "dark" { + t.Errorf("GetTheme(nonexistent) = %q, want %q", theme.Name, "dark") + } +} + +func TestSetTheme(t *testing.T) { + // Set theme to light + tui.SetTheme("light") + active := tui.GetActiveTheme() + if active.Name != "light" { + t.Errorf("After SetTheme(light), GetActiveTheme() = %q, want %q", active.Name, "light") + } + + // Set theme back to dark + tui.SetTheme("dark") + active = tui.GetActiveTheme() + if active.Name != "dark" { + t.Errorf("After SetTheme(dark), GetActiveTheme() = %q, want %q", active.Name, "dark") + } +} + +func TestGetActiveTheme(t *testing.T) { + // Default should be dark + active := tui.GetActiveTheme() + if active.Name != "dark" { + t.Errorf("GetActiveTheme() = %q, want %q", active.Name, "dark") + } +} + +func TestThemeRegistry(t *testing.T) { + // Test that all themes are registered + expectedThemes := []string{"dark", "light", "dracula"} + for _, name := range expectedThemes { + if _, ok := tui.Themes[name]; !ok { + t.Errorf("Theme %q not found in Themes map", name) + } + } + + // Test theme count + if len(tui.Themes) != 3 { + t.Errorf("Themes map has %d entries, want 3", len(tui.Themes)) + } +}