diff options
| author | Paul Buetow <paul@buetow.org> | 2026-05-13 19:44:15 +0300 |
|---|---|---|
| committer | Paul Buetow <paul@buetow.org> | 2026-05-13 19:44:15 +0300 |
| commit | 78ea9e22e596255c5e23ce445d80641870674ca9 (patch) | |
| tree | 47dfc75179676ee6be6c79f7d61d14a232fdf680 /internal/tui | |
| parent | 16e413799363871c1efd73527fba299dfdfadfd3 (diff) | |
keep View() pure by moving state transitions into Update() handlers
The dashboard model's View() was mutating sub-model state on every render:
it called streamModel.SetFooterVisible() and flameModel.SetViewport() on
local copies instead of keeping those fields in sync through Update().
Moved the sync points to the Update() handlers that trigger each change:
- handleWindowSize: syncs stream footer visibility alongside SetViewport
- handleHelpToggleKey: already updated flame viewport; now also updates
stream footer visibility when the help bar is toggled
- handleKey: syncs flame viewport when switching to the flame tab, covering
the case that window-resize and help-toggle do not
Aligned the stream model's initial showFooter value with the dashboard's
default showHelp=false in NewModelWithConfig so there is no mismatch on
the first render.
Also extracted numeric tab shortcuts into the tab registry via ShortcutKey
fields so handleShortcutKey no longer needs updating when tabs are added.
Updated two tests that bypassed Update() to set dimensions directly; they
now use WindowSizeMsg so sub-model viewports are initialised correctly
under the new pure-View contract.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Diffstat (limited to 'internal/tui')
| -rw-r--r-- | internal/tui/dashboard/model.go | 65 | ||||
| -rw-r--r-- | internal/tui/dashboard/model_test.go | 7 | ||||
| -rw-r--r-- | internal/tui/dashboard/tabregistry.go | 31 | ||||
| -rw-r--r-- | internal/tui/tui_test.go | 8 |
4 files changed, 76 insertions, 35 deletions
diff --git a/internal/tui/dashboard/model.go b/internal/tui/dashboard/model.go index cc2a052..004350a 100644 --- a/internal/tui/dashboard/model.go +++ b/internal/tui/dashboard/model.go @@ -148,6 +148,9 @@ func NewModelWithConfig(engine SnapshotSource, streamSource eventstream.Source, isDark: true, focused: true, } + // showHelp starts false; align the stream footer visibility so it matches + // from the first render without relying on View() to fix up the mismatch. + m.streamModel.SetFooterVisible(false) m.SetDarkMode(true) return m } @@ -203,6 +206,9 @@ func (m Model) handleWindowSize(msg tea.WindowSizeMsg) (tea.Model, tea.Cmd) { m.clampTableColumns() streamWidth, streamHeight := streamViewport(msg.Width, msg.Height) m.streamModel.SetViewport(streamWidth, streamHeight) + // Sync stream footer visibility so it matches the current help-bar state. + // This covers the case where showHelp was set before the first resize event. + m.streamModel.SetFooterVisible(m.showHelp) flameWidth, flameHeight := flameViewport(msg.Width, msg.Height, m.showHelp) m.flamegraphModel.SetViewport(flameWidth, flameHeight) m.setBubbleViewports(flameWidth, flameHeight) @@ -333,6 +339,14 @@ func (m Model) handleKey(msg tea.KeyPressMsg) (tea.Model, tea.Cmd) { if !handled { return m.handleUnhandledKey(msg) } + // When the user switches to the flame tab from any other tab, the flamegraph + // model needs its viewport updated to reflect the current dimensions. + // Window-resize and help-toggle already call SetViewport; tab switching does + // not, so we compensate here before returning the updated model to the runtime. + if prevActiveTab != m.activeTab && m.activeTab == TabFlame { + flameWidth, flameHeight := flameViewport(m.width, m.height, m.showHelp) + m.flamegraphModel.SetViewport(flameWidth, flameHeight) + } return m, m.postKeyTransitionCmd(prevActiveTab, cmd) } @@ -576,8 +590,12 @@ func (m Model) handleHelpToggleKey(msg tea.KeyPressMsg) (bool, tea.Model, tea.Cm return false, m, nil } m.showHelp = !m.showHelp + // Keep sub-model state in sync so View() stays a pure render pass. + // The flamegraph viewport shrinks/grows when the help bar expands/collapses; + // the stream footer row is only shown when the full help bar is visible. flameWidth, flameHeight := flameViewport(m.width, m.height, m.showHelp) m.flamegraphModel.SetViewport(flameWidth, flameHeight) + m.streamModel.SetFooterVisible(m.showHelp) return true, m, nil } @@ -590,35 +608,18 @@ func (m Model) handleFlameConsumedKey(msg tea.KeyPressMsg) (bool, tea.Model, tea return true, m, cmd } +// handleShortcutKey processes tab-navigation and action shortcuts. Numeric +// shortcuts are resolved via the tab registry so that adding a new tab with a +// shortcut key requires only a new tabDescriptor entry — this function never +// needs to be modified (OCP). func (m *Model) handleShortcutKey(msg tea.KeyPressMsg) (bool, tea.Cmd) { switch { - case key.Matches(msg, m.keys.One): - m.activeTab = TabFlame - return true, nil case key.Matches(msg, m.keys.Tab): m.activeTab = nextTab(m.activeTab) return true, nil case key.Matches(msg, m.keys.ShiftTab): m.activeTab = prevTab(m.activeTab) return true, nil - case key.Matches(msg, m.keys.Two): - m.activeTab = TabOverview - return true, nil - case key.Matches(msg, m.keys.Three): - m.activeTab = TabSyscalls - return true, nil - case key.Matches(msg, m.keys.Four): - m.activeTab = TabFiles - return true, nil - case key.Matches(msg, m.keys.Five): - m.activeTab = TabProcesses - return true, nil - case key.Matches(msg, m.keys.Six): - m.activeTab = TabLatency - return true, nil - case key.Matches(msg, m.keys.Seven): - m.activeTab = TabStream - return true, nil case key.Matches(msg, m.keys.Visualize): return true, m.cycleVisualizationMode() case key.Matches(msg, m.keys.Metric): @@ -630,9 +631,15 @@ func (m *Model) handleShortcutKey(msg tea.KeyPressMsg) (bool, tea.Cmd) { return false, nil } return true, m.toggleFilesDirGrouping() - default: - return false, nil } + // Fall through to registry-driven numeric tab shortcuts. Each tab + // registers its own key binding in tabDescriptors; no changes here + // are needed when new tabs are added. + if tab, ok := tabForShortcutKey(msg, m.keys); ok { + m.activeTab = tab + return true, nil + } + return false, nil } func (m *Model) toggleFilesDirGrouping() tea.Cmd { @@ -1091,23 +1098,21 @@ func (m *Model) SetPidFilter(pid int) { } // View renders the tab bar, active tab scaffold, and help bar. +// This is a pure render pass: it reads model state but never mutates it. +// Sub-model state (stream footer visibility, flamegraph viewport dimensions) +// is kept in sync by the Update() handlers that trigger each state change, +// so no fixup is needed here. func (m Model) View() tea.View { width, height := common.EffectiveViewport(m.width, m.height) _, activeHeight := flameViewport(width, height, m.showHelp) - streamModel := m.streamModel - flameModel := m.flamegraphModel - streamModel.SetFooterVisible(m.showHelp) if m.activeTab == TabStream { _, activeHeight = streamViewport(width, height) } - if m.activeTab == TabFlame { - flameModel.SetViewport(width, activeHeight) - } var b strings.Builder b.WriteString(renderTabBar(m.activeTab, width)) b.WriteString("\n") - b.WriteString(m.renderActiveContent(width, activeHeight, &streamModel, &flameModel)) + b.WriteString(m.renderActiveContent(width, activeHeight, &m.streamModel, &m.flamegraphModel)) b.WriteString("\n") if m.showHelp { b.WriteString(renderHelpBarWithStatus(m.keys, width, m.filterSummary())) diff --git a/internal/tui/dashboard/model_test.go b/internal/tui/dashboard/model_test.go index 59c2155..87eee52 100644 --- a/internal/tui/dashboard/model_test.go +++ b/internal/tui/dashboard/model_test.go @@ -1507,8 +1507,11 @@ func TestViewRendersTabBarAndHelp(t *testing.T) { func TestFlameTabRendersWaitingForDataPlaceholder(t *testing.T) { m := NewModelWithConfig(nil, nil, 1000, common.DefaultKeyMap()) m.activeTab = TabFlame - m.width = 120 - m.height = 30 + // Dimensions must flow through Update so that sub-model viewports are + // kept in sync. Direct field assignment bypasses the sync logic in + // handleWindowSize, so use a WindowSizeMsg instead. + next, _ := m.Update(tea.WindowSizeMsg{Width: 120, Height: 30}) + m = next.(Model) out := m.View().Content if !strings.Contains(out, "Flame: waiting for data...") { diff --git a/internal/tui/dashboard/tabregistry.go b/internal/tui/dashboard/tabregistry.go index d16a363..2a5c7ff 100644 --- a/internal/tui/dashboard/tabregistry.go +++ b/internal/tui/dashboard/tabregistry.go @@ -9,6 +9,7 @@ import ( flamegraphtui "ior/internal/tui/flamegraph" "ior/internal/tui/messages" + "charm.land/bubbles/v2/key" tea "charm.land/bubbletea/v2" ) @@ -47,6 +48,11 @@ type tabDescriptor struct { // HandleScroll handles direction keys for this tab when bubbles are off. // Nil means the tab does not process scroll/navigation keys. HandleScroll tabScrollFn + // ShortcutKey extracts the numeric shortcut key binding for this tab from + // a KeyMap. It is called at runtime against the model's configured key map + // so that custom key maps (e.g. in tests) are respected. Nil means the tab + // has no direct numeric shortcut; it is still reachable via tab/shift+tab. + ShortcutKey func(keys common.KeyMap) key.Binding } // tabDescriptors is the central registry mapping every known Tab to its @@ -61,6 +67,7 @@ var tabDescriptors = map[Tab]tabDescriptor{ InitCmd: flameTickCmd, Render: tabRenderFlame, HandleScroll: nil, + ShortcutKey: func(k common.KeyMap) key.Binding { return k.One }, }, TabOverview: { Name: "Overview", @@ -69,6 +76,7 @@ var tabDescriptors = map[Tab]tabDescriptor{ AllowedVizModes: []tabVizMode{tabVizModeTable}, Render: tabRenderOverview, HandleScroll: nil, + ShortcutKey: func(k common.KeyMap) key.Binding { return k.Two }, }, TabSyscalls: { Name: "Syscalls", @@ -77,6 +85,7 @@ var tabDescriptors = map[Tab]tabDescriptor{ AllowedVizModes: []tabVizMode{tabVizModeTable, tabVizModeBubbles, tabVizModeTreemap}, Render: tabRenderSyscalls, HandleScroll: tabScrollSyscalls, + ShortcutKey: func(k common.KeyMap) key.Binding { return k.Three }, }, TabFiles: { Name: "Files", @@ -85,6 +94,7 @@ var tabDescriptors = map[Tab]tabDescriptor{ AllowedVizModes: []tabVizMode{tabVizModeTable}, Render: tabRenderFiles, HandleScroll: tabScrollFiles, + ShortcutKey: func(k common.KeyMap) key.Binding { return k.Four }, }, TabProcesses: { Name: "Processes", @@ -93,6 +103,7 @@ var tabDescriptors = map[Tab]tabDescriptor{ AllowedVizModes: []tabVizMode{tabVizModeTable, tabVizModeBubbles, tabVizModeTreemap}, Render: tabRenderProcesses, HandleScroll: tabScrollProcesses, + ShortcutKey: func(k common.KeyMap) key.Binding { return k.Five }, }, TabLatency: { Name: "Latency+Gaps", @@ -101,6 +112,7 @@ var tabDescriptors = map[Tab]tabDescriptor{ AllowedVizModes: []tabVizMode{tabVizModeTable}, Render: tabRenderLatency, HandleScroll: nil, + ShortcutKey: func(k common.KeyMap) key.Binding { return k.Six }, }, TabStream: { Name: "Stream", @@ -110,6 +122,7 @@ var tabDescriptors = map[Tab]tabDescriptor{ InitCmd: streamTickCmd, Render: tabRenderStream, HandleScroll: tabScrollStream, + ShortcutKey: func(k common.KeyMap) key.Binding { return k.Seven }, }, } @@ -137,6 +150,24 @@ func lookupTab(tab Tab) tabDescriptor { return tabDescriptor{Name: "Unknown", ShortName: "Unk", AllowedVizModes: []tabVizMode{tabVizModeTable}} } +// tabForShortcutKey searches the registry for the first tab whose ShortcutKey +// matches the given key press message. It returns the tab and true when a match +// is found; otherwise the zero Tab value and false. Using the registry here +// means adding a new tab with a shortcut only requires a new entry in +// tabDescriptors — handleShortcutKey in model.go never needs updating. +func tabForShortcutKey(msg tea.KeyPressMsg, keys common.KeyMap) (Tab, bool) { + for _, tab := range orderedTabs() { + d := tabDescriptors[tab] + if d.ShortcutKey == nil { + continue + } + if key.Matches(msg, d.ShortcutKey(keys)) { + return tab, true + } + } + return 0, false +} + // tabAllowedVizModes returns the visualisation modes allowed for tab, // respecting any runtime conditions (e.g. Files requires dir-grouped mode for // non-table views). This replaces the former allowedVizModes switch statement. diff --git a/internal/tui/tui_test.go b/internal/tui/tui_test.go index d76ccde..dae45f7 100644 --- a/internal/tui/tui_test.go +++ b/internal/tui/tui_test.go @@ -2043,15 +2043,17 @@ func TestDashboardTabKeysChangeActiveView(t *testing.T) { m := NewModel(-1, func(context.Context) error { return nil }) m.screen = ScreenDashboard m.attaching = false - m.width = 120 - m.height = 30 + // Dimensions must flow through Update so that sub-model viewports are + // kept in sync with the new pure-View contract. + next, _ := m.Update(tea.WindowSizeMsg{Width: 120, Height: 30}) + m = next.(Model) out := m.View().Content if !strings.Contains(out, "Flame: waiting for data") { t.Fatalf("expected flame waiting view by default") } - next, _ := m.Update(tea.KeyPressMsg{Code: []rune{'2'}[0], Text: string([]rune{'2'})}) + next, _ = m.Update(tea.KeyPressMsg{Code: []rune{'2'}[0], Text: string([]rune{'2'})}) updated := next.(Model) out = updated.View().Content if !strings.Contains(out, "Overview: waiting for stats") { |
