Skip to content

feat: modularize TUI dashboard components into individual view files #363

Merged
SuperCoolPencil merged 30 commits intomainfrom
fix-activity-log
Apr 16, 2026
Merged

feat: modularize TUI dashboard components into individual view files #363
SuperCoolPencil merged 30 commits intomainfrom
fix-activity-log

Conversation

@SuperCoolPencil
Copy link
Copy Markdown
Member

@SuperCoolPencil SuperCoolPencil commented Apr 12, 2026

Greptile Summary

This PR splits the monolithic TUI view into focused per-component files (view_dashboard_*.go) and introduces a centralised DashboardLayout struct that drives all dimension calculations. It also resolves several bugs flagged in prior review rounds: the GotoBottom-on-every-frame scroll lock, the inflated GetGraphAreaDimensions argument, the processProgressMsg discarding its tea.Cmd, the AvailableHeight vs raw termH threshold mismatch, and the missing cancel-path restore for filepickerOriginalPath.

Confidence Score: 5/5

Safe to merge — all previously flagged P0/P1 bugs are resolved; remaining findings are P2 style clean-ups.

All critical layout, scroll-lock, tea.Cmd propagation, and threshold bugs from prior review rounds appear addressed in this version. The three remaining comments (dead found variable, SingleLineHeight used as a width offset, and per-iteration style allocation in the modal loop) are minor readability issues with no runtime impact.

internal/tui/update_events.go (dead found variable), internal/tui/view.go (SingleLineHeight as horizontal offset), internal/tui/components/add_download_modal.go (style allocation in loop)

Important Files Changed

Filename Overview
internal/tui/update_events.go Fixes early-return bug in DownloadStartedMsg and processProgressMsg now returns tea.Cmd; dead found variable left over from refactoring.
internal/tui/layout_helpers.go New DashboardLayout struct and CalculateDashboardLayout centralise layout math; correctly uses l.AvailableHeight for all threshold comparisons.
internal/tui/view_dashboard_log.go renderLogBox now only calls m.logViewport.View(); GotoBottom is no longer called on every render frame, fixing the scroll-lock bug.
internal/tui/view_dashboard_graph.go Passes outer width directly to GetGraphAreaDimensions, fixing the previous contentWidth+4 inflation that could overflow boxes.
internal/tui/helpers.go refreshLogViewportContent wraps log entries at viewport width with bottom-alignment padding; SetContent/GotoBottom called only in addLogEntry, not on every View().
internal/tui/update.go WindowSizeMsg handler replaced with DashboardLayout-driven sizing; viewport and list dimensions now derived from the shared layout struct.
internal/tui/update_filepicker.go Cancel handler now restores filepickerOriginalPath to the correct field for every browsing context (settings, extension, category manager, input).
internal/tui/components/add_download_modal.go Input width now computed dynamically per-row; horizontalPadding style created on every iteration — should be moved outside the loop.
internal/tui/process.go processProgressMsg now returns tea.Cmd so the progress-bar spring animation command is propagated to the runtime instead of being discarded.
internal/tui/view.go Modal dimensions are now dynamic via GetDynamicModalDimensions; SingleLineHeight used as a horizontal offset (semantic mismatch, functionally correct).

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    WS[WindowSizeMsg] --> CDL[CalculateDashboardLayout]
    CDL --> LVP[logViewport.SetWidth/Height]
    CDL --> LSZ[list.SetSize]
    CDL --> FPH[filepicker.SetHeight]

    View[View] --> CDL2[CalculateDashboardLayout]
    CDL2 --> HB[renderHeaderBox\nview_dashboard_header.go]
    CDL2 --> LB[renderLogBox\nview_dashboard_log.go]
    CDL2 --> DL[renderDownloadsBox\nview_dashboard_list.go]
    CDL2 --> GB[renderGraphBox\nview_dashboard_graph.go]
    CDL2 --> DB[renderDetailsBox\nview_dashboard_details.go]
    CDL2 --> CB[renderChunkMapBox\nview_dashboard_chunkmap.go]

    HB & LB --> Header[headerBox]
    GB & DB & CB --> RightCol[rightColumn]
    Header & DL & RightCol --> Body[body]
    Body --> FV[fullView + footer]
Loading

Comments Outside Diff (1)

  1. internal/tui/view.go, line 326-342 (link)

    P1 Double computation with mismatched widths breaks detail pane height measurement

    renderFocusedDetails is called here to measure detailHeight for layout, using detailWidth = rightWidth - PaneStyle.GetHorizontalFrameSize(). Because DefaultPaddingX = 1, PaneStyle.GetHorizontalFrameSize() = 4, so the measurement width is rightWidth - 4.

    renderDetailsBox then calls renderFocusedDetails again internally with contentWidth = width - 2 = rightWidth - 2 — 2 units wider. Inside renderFocusedDetails, the actual inner content width is w - 4, so:

    • Height-measurement call: inner contentWidth = rightWidth - 8
    • Rendering call: inner contentWidth = rightWidth - 6

    If any text (filename, path, ID, stats) wraps at one width but not the other, the measured detailHeight diverges from the actual rendered height, mis-sizing the right column's layout. The original code computed this once with a consistent width.

    The simplest fix is to pass the already-computed detailContent into renderDetailsBox instead of recomputing:

    // view.go — pass pre-computed content
    detailBox := m.renderDetailsBox(rightWidth, detailHeight, detailContent)
    // view_dashboard_details.go — accept pre-computed content
    func (m *RootModel) renderDetailsBox(width, height int, innerContent string) string {
        return renderBtopBox("", PaneTitleStyle.Render(" File Details "), innerContent, width, height, colors.Gray)
    }
    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: internal/tui/view.go
    Line: 326-342
    
    Comment:
    **Double computation with mismatched widths breaks detail pane height measurement**
    
    `renderFocusedDetails` is called here to measure `detailHeight` for layout, using `detailWidth = rightWidth - PaneStyle.GetHorizontalFrameSize()`. Because `DefaultPaddingX = 1`, `PaneStyle.GetHorizontalFrameSize() = 4`, so the measurement width is `rightWidth - 4`.
    
    `renderDetailsBox` then calls `renderFocusedDetails` again internally with `contentWidth = width - 2 = rightWidth - 2` — 2 units wider. Inside `renderFocusedDetails`, the actual inner content width is `w - 4`, so:
    - Height-measurement call: inner `contentWidth = rightWidth - 8`
    - Rendering call: inner `contentWidth = rightWidth - 6`
    
    If any text (filename, path, ID, stats) wraps at one width but not the other, the measured `detailHeight` diverges from the actual rendered height, mis-sizing the right column's layout. The original code computed this once with a consistent width.
    
    The simplest fix is to pass the already-computed `detailContent` into `renderDetailsBox` instead of recomputing:
    
    ```go
    // view.go — pass pre-computed content
    detailBox := m.renderDetailsBox(rightWidth, detailHeight, detailContent)
    ```
    
    ```go
    // view_dashboard_details.go — accept pre-computed content
    func (m *RootModel) renderDetailsBox(width, height int, innerContent string) string {
        return renderBtopBox("", PaneTitleStyle.Render(" File Details "), innerContent, width, height, colors.Gray)
    }
    ```
    
    How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
This is a comment left during a code review.
Path: internal/tui/update_events.go
Line: 147-183

Comment:
**Dead `found` variable in `DownloadStartedMsg` case**

The `found` variable is declared `false` on line 147 and never set to `true` anywhere in this case. The `if d := ...; d != nil` branch now returns early, so `if !found {` at line 173 is always `true` when reached — the variable and condition are unreachable dead code.

```suggestion
	case events.DownloadStartedMsg:
		if d := m.FindDownloadByID(msg.DownloadID); d != nil {
```
Then remove the `if !found {` wrapper around the fallthrough block (keep its body as flat code).

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: internal/tui/view.go
Line: 475

Comment:
**`SingleLineHeight` used as a horizontal width offset**

`components.SingleLineHeight` is documented as "a standard single line height (1)" and is used correctly in `view_dashboard_header.go` to subtract a row from a height. Here it's subtracted from a _width_ (`maxProgWidth`), making the intent unclear. A plain `1` or a dedicated `HorizontalPadding` constant would be clearer.

```suggestion
		maxProgWidth := contentWidth - lipgloss.Width(labelStr) - 1
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: internal/tui/components/add_download_modal.go
Line: 136-140

Comment:
**Style object and padding calculation repeated per iteration**

`horizontalPadding` creates a new `lipgloss.Style` on every loop iteration to read a value that never changes between iterations. Move both the `const labelWidth` and the `horizontalPadding` calculation outside the loop.

```suggestion
	const labelWidth = 10
	horizontalPadding := lipgloss.NewStyle().Padding(0, 2).GetHorizontalFrameSize()
	for i := 0; i < len(m.Inputs) && i < len(m.Labels); i++ {
		inputW := m.Width - BorderFrameWidth - horizontalPadding - labelWidth
```

How can I resolve this? If you propose a fix, please make it concise.

Reviews (10): Last reviewed commit: "chore: update demo assets and record new..." | Re-trigger Greptile

Comment thread internal/tui/view_dashboard_header.go
Comment thread internal/tui/view_dashboard_graph.go Outdated
Comment thread internal/tui/view.go Outdated
Comment thread internal/tui/view_dashboard_log.go Outdated
Comment thread internal/tui/update.go Outdated
Comment thread internal/tui/layout_helpers.go Outdated
Comment thread internal/tui/update_events.go
@SuperCoolPencil
Copy link
Copy Markdown
Member Author

Screenshot from 2026-04-16 18-15-16
image
image

@SuperCoolPencil SuperCoolPencil merged commit f37b47b into main Apr 16, 2026
16 checks passed
@SuperCoolPencil SuperCoolPencil deleted the fix-activity-log branch April 16, 2026 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant