v0.14.0: Compact header mode

[yimsk]

Summary

• Add compact header mode (1-2 lines vs 5 lines) toggleable via Ctrl+E
• Support --compact/--no-compact CLI flags
• Add compact_header config setting with autosave support
• Dynamic profile/region width calculation with proper truncation

Changes

• 20 commits from feature/header-redesign
• New CompactHeaderChangedMsg message type
• Full test coverage for formatting functions
• Docs: keybindings.md, configuration.md updated

[claude]

PR Review: v0.14.0 - Compact Header Mode

Summary

This PR adds a compact header mode feature that allows users to toggle between a 5-line header (normal) and a 1-2 line header (compact) using Ctrl+E. The implementation includes CLI flags, config file support, autosave integration, and comprehensive test coverage.

Code Quality & Best Practices ✅

Strengths:

• Well-structured code: Clean separation of concerns with dedicated formatting functions (formatProfilesWithAccounts, formatRegions, formatSingleProfile)
• Consistent naming: Good use of descriptive names and clear constants
• Good documentation: Helpful comments explaining complex logic (e.g., width calculations, truncation behavior)
• Thread-safe config updates: Proper use of mutexes in config.go
• Test coverage: Excellent test coverage with 330 lines of tests in header_panel_test.go

Minor improvements needed:

1. Magic numbers (internal/view/header_panel.go:320-321):

   profileMaxWidth := remainingWidth * profileWidthRatio / regionWidthRatio
   regionMaxWidth := remainingWidth - profileMaxWidth

   The ratio calculation could benefit from a comment explaining the 2:1 ratio choice.

2. Duplicate separator rendering (internal/view/header_panel.go:125, 207, 378):
   The separator pattern valueStyle.Render(", ") appears multiple times. Consider extracting to a constant or helper.

Potential Bugs & Issues ⚠️

1. Integer Division Precision Loss (header_panel.go:320)

profileMaxWidth := remainingWidth * profileWidthRatio / regionWidthRatio

When remainingWidth * 2 / 3, you could lose precision with integer division. For example, if remainingWidth = 50, profile gets 33 and region gets 17 (50-33), not the intended 2:1 ratio.

Recommendation: Add a comment or consider rounding behavior.

2. Viewport Recalculation on Uninitialized State (detail_view.go:224-232)

func (d *DetailView) recalcViewport() {
    // ... calculations ...
    if !d.vp.Ready {
        return
    }
    content := d.renderContent()
    d.vp.Model.SetContent(content)
}

The Ready check comes AFTER size calculations. If viewport is not ready, size is set but content isn't. This could cause visual glitches on first render.

Recommendation: Consider checking Ready earlier or ensuring initialization order is clear.

3. Width Constraint Edge Case (header_panel.go:128)

if maxWidth <= 0 && len(selections) > 1 {

When maxWidth <= 0 with multiple selections, the code forces truncation. However, when maxWidth == 0 with a single selection, it renders the full profile without truncation. This inconsistency could be confusing.

Recommendation: Document this behavior or handle maxWidth <= 0 uniformly.

Performance Considerations ⚡

Good:

• Efficient truncation: The truncation logic in formatProfilesWithAccounts and formatRegions calculates width incrementally, avoiding multiple passes
• Cached styles: Using headerPanelStyles struct avoids recreating lipgloss styles on every render
• Early bailout: Functions check empty slices early (lines 121, 194)

Potential optimization:

• String allocation (header_panel.go:144, 209): Multiple make([]string, 0, len(...)) allocations. These are fine, but be aware this happens on every render. Consider profiling if performance becomes an issue in tight loops.

Security Concerns 🔒

No security issues identified.

The PR deals with UI rendering and configuration management. No user input validation issues, no command injection risks, no sensitive data exposure.

Test Coverage 📊

Excellent coverage! The PR includes:

• ✅ 330 lines of comprehensive tests in header_panel_test.go
• ✅ Tests for both normal and compact modes
• ✅ Tests for mode switching
• ✅ Edge cases: empty regions, single vs. multiple items, width constraints
• ✅ Tests for formatting functions with various truncation scenarios
• ✅ Config persistence tests in config_test.go

Coverage gaps:

1. No integration test for Ctrl+E keybinding triggering the mode switch
2. No test for autosave behavior when toggling compact mode
3. Settings view changes (internal/view/settings_view.go) have reduced test coverage (-2 lines)

Recommendation: Add a test case in app_test.go to verify the complete flow: key press → config update → autosave → UI update.

Additional Observations

Documentation

• ✅ Updated docs/keybindings.md with Ctrl+E binding
• ✅ Updated docs/configuration.md with compact_header setting
• ✅ Added CLI flags to help text

Message Propagation

The CompactHeaderChangedMsg is properly propagated to all views:

• ✅ dashboard_view.go:195
• ✅ service_browser.go:182
• ✅ detail_view.go:135
• ✅ resource_browser.go:227

Good job ensuring all views respond to the mode change!

Consistency

One minor inconsistency: command_input.go:104 calls ReloadStyles() but doesn't reset compact header. This is likely fine since styles and compact mode are independent, but worth noting.

Recommendations Summary

Must Fix

None - no blocking issues

Should Fix

1. Add comment explaining the 2:1 ratio calculation for profile/region widths
2. Document or handle maxWidth <= 0 behavior consistently
3. Consider checking viewport Ready state earlier in recalcViewport()

Nice to Have

1. Extract separator rendering to a helper/constant
2. Add integration test for the full compact mode toggle flow
3. Profile string allocation performance if rendering becomes a bottleneck

Verdict

LGTM with minor suggestions ✅

This is a well-implemented feature with good code quality, comprehensive tests, and thoughtful UX design. The compact header mode addresses a real user need for more screen real estate. The issues identified are minor and don't block merging.

Great work on the dynamic width calculation and truncation logic - that's non-trivial to get right!

---

Reviewed by Claude Code