FilePreviewPanel: Architectural issues with state management and encapsulation

[coderabbitai]

Summary

The FilePreviewPanel type introduced in PR #997 has several architectural issues related to state management, encapsulation, and maintainability that need to be addressed.

Issues Identified

1. Direct Field Manipulation

Problem: Width, height, and open fields are being set directly throughout the codebase instead of using proper encapsulation methods.

Examples:

• Direct field access patterns found in multiple files
• Missing methods like IsOpen(), ToggleOpen(), SetSize(width, height int)

2. Complex and Scattered Width/Height Calculations

Problem: Width and height calculation logic is spread across multiple files and methods, making it prone to errors and hard to maintain.

Locations:

• getFilePreviewWidth() in model.go
• setFilePreviewPanelSize() in model.go
• toggleFilePreviewPanel function
• Various render methods

3. Missing Footer Height Adjustments

Problem: Height value is not properly adjusted when footer display is toggled via toggleFooterController.

Impact: This can cause rendering issues and incorrect layout calculations.

4. Lack of Unit Tests

Problem: No unit tests exist for FilePreviewPanel width, height, and rendering functionality.

Risk: Changes to this component are not validated, increasing the likelihood of regressions.

5. Tight Coupling with UI Rendering

Problem: FilePreviewPanel is tightly coupled with UI rendering logic, making it hard to test and maintain.

Examples:

• Direct dependency on ui.FilePreviewPanelRenderer
• RenderText method mixing business logic with presentation

6. Async Rendering Concerns

Problem: With the introduction of async rendering (FilePreviewUpdateMsg, filePreviewCmd), there may be race conditions or inconsistent state updates.

Risk: Preview content might be rendered with outdated dimensions or state.

7. Configuration Coupling

Problem: Preview panel logic is tightly coupled with global configuration, making it hard to test in isolation.

Suggested Improvements

1. Add Proper Encapsulation Methods:

   • IsOpen() bool
   • ToggleOpen()
   • SetDimensions(width, height int)
   • GetDimensions() (int, int)

2. Centralize Size Calculations:

   • Move all width/height logic to dedicated methods
   • Create a single source of truth for size calculations
   • Handle footer adjustments automatically

3. Add Comprehensive Unit Tests:

   • Test dimension calculations
   • Test state transitions (open/close)
   • Test rendering with different configurations

4. Improve Separation of Concerns:

   • Separate rendering logic from state management
   • Reduce coupling with global configuration
   • Make components more testable

5. Handle Async State Properly:

   • Ensure thread-safe access to dimensions
   • Validate state consistency in async updates

Related

• PR: fix: Async rendering, Include clipboard check in paste items, and update linter configs #997
• Comment: fix: Async rendering, Include clipboard check in paste items, and update linter configs #997 (comment)
• Reporter: @lazysegtree

Priority

Medium - These are architectural improvements that will improve maintainability and reduce the likelihood of bugs, but don't affect current functionality.