fix visuals on review tab

This commit is contained in:
Emil Lerch 2026-06-28 07:20:19 -07:00
parent f1f665758c
commit 8986537d93
Signed by: lobo
GPG key ID: A7B62D657EF764F8
2 changed files with 282 additions and 38 deletions

View file

@ -747,8 +747,7 @@ pub const App = struct {
if (mouse.col >= col and mouse.col < col + lbl_len) {
if (self.isDisabled(t)) return;
self.active_tab = t;
self.scroll_offset = 0;
self.loadTabData();
self.switchedToTab();
ctx.queueRefresh() catch |err| std.log.debug("queueRefresh failed: {t}", .{err});
return ctx.consumeAndRedraw();
}
@ -1048,7 +1047,7 @@ pub const App = struct {
if (self.input_len > 0) {
self.setActiveSymbol(self.input_buf[0..self.input_len]);
self.active_tab = .quote;
self.loadTabData();
self.switchedToTab();
ctx.queueRefresh() catch |err| std.log.debug("queueRefresh failed: {t}", .{err});
}
self.mode = .normal;
@ -1061,6 +1060,10 @@ pub const App = struct {
fn handleNormalKey(self: *App, ctx: *vaxis.vxfw.EventContext, key: vaxis.Key) void {
// Ctrl+L: full screen redraw (standard TUI convention, not configurable)
if (key.codepoint == 'l' and key.mods.ctrl) {
// Also drop any status override so the bar returns to
// the active tab's default hint - "redraw" should make
// the screen pristine, status line included.
self.resetStatus();
ctx.queueRefresh() catch |err| std.log.debug("queueRefresh failed: {t}", .{err});
return ctx.consumeAndRedraw();
}
@ -1100,15 +1103,13 @@ pub const App = struct {
},
.prev_tab => {
self.prevTab();
self.scroll_offset = 0;
self.loadTabData();
self.switchedToTab();
ctx.queueRefresh() catch |err| std.log.debug("queueRefresh failed: {t}", .{err});
return ctx.consumeAndRedraw();
},
.next_tab => {
self.nextTab();
self.scroll_offset = 0;
self.loadTabData();
self.switchedToTab();
ctx.queueRefresh() catch |err| std.log.debug("queueRefresh failed: {t}", .{err});
return ctx.consumeAndRedraw();
},
@ -1118,8 +1119,7 @@ pub const App = struct {
const target = tabs[idx];
if (self.isDisabled(target)) return;
self.active_tab = target;
self.scroll_offset = 0;
self.loadTabData();
self.switchedToTab();
ctx.queueRefresh() catch |err| std.log.debug("queueRefresh failed: {t}", .{err});
return ctx.consumeAndRedraw();
}
@ -1374,6 +1374,27 @@ pub const App = struct {
self.status_len = len;
}
/// Drop any active status override so the next draw falls back
/// to the active tab's default hint (globals + `status_hints`).
/// Called on navigation events - tab switches and the Ctrl+L
/// redraw - so a stale message set on one screen can't linger
/// past the point where it stops being relevant.
pub fn resetStatus(self: *App) void {
self.status_len = 0;
}
/// Common tail for every "the active tab changed" path: reset
/// the viewport scroll, drop any stale status override (so the
/// new tab shows its own default hint unless its `activate`
/// sets something), and load the new tab's data. Centralized so
/// no switch site can forget a step. Callers set `active_tab`
/// (directly or via `nextTab`/`prevTab`) before calling this.
fn switchedToTab(self: *App) void {
self.scroll_offset = 0;
self.resetStatus();
self.loadTabData();
}
/// Cell pixel size for the active terminal, used by tabs that
/// render bitmap charts via the Kitty graphics protocol. Falls
/// back to (8, 16) when vaxis hasn't reported pixel dimensions
@ -1728,14 +1749,37 @@ pub const App = struct {
// ASCII: single byte, single column
buf[row * width + col] = .{ .char = .{ .grapheme = ascii_g[byte] }, .style = s };
bi += 1;
col += 1;
} else {
// Multi-byte UTF-8: determine sequence length
const seq_len: usize = if (byte >= 0xF0) 4 else if (byte >= 0xE0) 3 else if (byte >= 0xC0) 2 else 1;
const end = @min(bi + seq_len, line.text.len);
buf[row * width + col] = .{ .char = .{ .grapheme = line.text[bi..end] }, .style = s };
var end = @min(bi + seq_len, line.text.len);
// Fold a trailing U+FE0F (bytes EF B8 8F) variation
// selector into THIS cell and report the cell as 2
// display columns wide. zfin's wide glyphs (severity
// / status-grid emoji) all carry FE0F to force emoji
// presentation; vaxis defaults a cell to width 1 and
// only measures the grapheme when width == 0, so
// without this it tracks each emoji as a single
// column. Its partial-redraw diff then advances the
// terminal cursor one column short per emoji, which
// is what smears stale glyphs across the screen as
// the findings list scrolls. Telling it the true
// width keeps the cursor accounting aligned. The
// skipped second column stays the row-fill blank;
// vaxis skips it because it advances its buffer index
// by the cell width. Only fold when both columns fit.
var cell_width: u8 = 1;
if (col + 1 < width and end + 3 <= line.text.len and
line.text[end] == 0xEF and line.text[end + 1] == 0xB8 and line.text[end + 2] == 0x8F)
{
end += 3;
cell_width = 2;
}
buf[row * width + col] = .{ .char = .{ .grapheme = line.text[bi..end], .width = cell_width }, .style = s };
bi = end;
col += cell_width;
}
col += 1;
}
}
}
@ -2843,6 +2887,66 @@ test "formatStatusHint: single fragment has no separator" {
try testing.expectEqualStrings("ctrl+s save", out);
}
test "resetStatus: clears an active status override" {
var app: App = undefined;
app.setStatus("stuck message");
try testing.expect(app.status_len > 0);
app.resetStatus();
try testing.expectEqual(@as(usize, 0), app.status_len);
}
// drawStyledContent: wide-glyph (FE0F) width handling
test "drawStyledContent: folds a trailing FE0F into one width-2 cell" {
const w: u16 = 8;
var buf: [w]vaxis.Cell = undefined;
const lines = [_]StyledLine{
.{ .text = "a\u{26A0}\u{FE0F}b", .style = .{} }, // a + warning-emoji + b
};
try App.drawStyledContent(undefined, undefined, &buf, w, 1, &lines);
// col 0: plain ASCII, default width 1.
try testing.expectEqualStrings("a", buf[0].char.grapheme);
try testing.expectEqual(@as(u8, 1), buf[0].char.width);
// col 1: both codepoints (base + FE0F) in one cell, 2 cols wide.
try testing.expectEqualStrings("\u{26A0}\u{FE0F}", buf[1].char.grapheme);
try testing.expectEqual(@as(u8, 2), buf[1].char.width);
// col 2: the emoji's second column - left as the row-fill blank;
// vaxis skips it because the prior cell reports width 2.
try testing.expectEqualStrings(" ", buf[2].char.grapheme);
// col 3: the next glyph lands after the 2-wide emoji, not at col 2.
try testing.expectEqualStrings("b", buf[3].char.grapheme);
}
test "drawStyledContent: a multibyte glyph without FE0F stays one column" {
const w: u16 = 4;
var buf: [w]vaxis.Cell = undefined;
const lines = [_]StyledLine{
.{ .text = "\u{2014}x", .style = .{} }, // em-dash (1 col) + x
};
try App.drawStyledContent(undefined, undefined, &buf, w, 1, &lines);
try testing.expectEqualStrings("\u{2014}", buf[0].char.grapheme);
try testing.expectEqual(@as(u8, 1), buf[0].char.width);
try testing.expectEqualStrings("x", buf[1].char.grapheme);
}
test "drawStyledContent: an FE0F emoji with no room for two columns is not folded" {
// width 2: 'a' takes col 0, leaving only col 1 - not enough room
// for a 2-wide cell, so the emoji is left at width 1 (truncation
// territory at the right edge, same as before the fold).
const w: u16 = 2;
var buf: [w]vaxis.Cell = undefined;
const lines = [_]StyledLine{
.{ .text = "a\u{26A0}\u{FE0F}", .style = .{} },
};
try App.drawStyledContent(undefined, undefined, &buf, w, 1, &lines);
try testing.expectEqualStrings("a", buf[0].char.grapheme);
try testing.expectEqual(@as(u8, 1), buf[1].char.width);
}
// symbol toggle helpers
test "shouldStashSymbol: stashes a differing non-empty symbol" {

View file

@ -269,6 +269,22 @@ pub const tab = struct {
pub const deactivate = framework.noopDeactivate(State);
/// Status-bar override. While collecting an ack note, render the
/// note-entry key hints full-width; otherwise the App-level
/// default status applies. Driving the prompt off `input_mode`
/// (instead of a one-shot `setStatus`) means it appears exactly
/// when the modal is open and the App restores the tab's default
/// hint the instant we leave it - no frozen prompt after Esc or
/// commit. It also marks the tab modal, so a stray tab-bar click
/// can't switch tabs out from under a half-typed note.
pub fn statusOverride(state: *State, app: *App) ?framework.StatusOverride {
_ = app;
return switch (state.input_mode) {
.normal => null,
.ack_note => .{ .hint = "Type reasoning. Enter = next line. Ctrl+Enter = save. Esc = cancel." },
};
}
/// Framework poll-tick hook: true while the observation panel
/// has async checks in flight. Drives the App's poll timer so
/// the `tick` hook below gets called without user input. The
@ -416,6 +432,11 @@ pub const tab = struct {
/// Wheel events fall through to App's scroll handling via
/// `onWheelMove` returning false (see below).
pub fn handleMouse(state: *State, app: *App, mouse: vaxis.Mouse) bool {
// While collecting an ack note the tab is modal (statusOverride
// is non-null, so the App routes every mouse event here).
// Swallow them all: a click that moved the cursor mid-note
// would make commitAckNote write to a different finding.
if (state.input_mode == .ack_note) return true;
if (mouse.button != .left) return false;
if (mouse.type != .press) return false;
const view = state.view orelse return false;
@ -542,6 +563,9 @@ pub const tab = struct {
return true;
};
state.note_len = 0;
// The bar just grew by a line; keep its bottom on
// screen so the next line the user types is visible.
ensureAckInputVisible(state, app);
return true;
},
.committed => {
@ -667,15 +691,37 @@ fn wrapCursor(current: usize, delta: isize, total: usize) usize {
/// (acceptable - the cursor IS at row zero anyway).
fn ensureCursorVisible(state: *const State, scroll_offset: *usize, visible_height: usize) void {
const visual = cursorVisualRow(state) orelse return;
if (visual < scroll_offset.*) {
scroll_offset.* = visual;
return;
}
if (visible_height > 0 and visual >= scroll_offset.* + visible_height) {
scroll_offset.* = visual - visible_height + 1;
ensureRowVisible(visual, scroll_offset, visible_height);
}
/// Scroll `scroll_offset` the minimum amount so content row
/// `target` lands inside the `[scroll_offset, scroll_offset +
/// visible_height)` viewport. Pure over the offset pointer;
/// shared by the cursor-follow path and the ack-input-follow path.
fn ensureRowVisible(target: usize, scroll_offset: *usize, visible_height: usize) void {
if (target < scroll_offset.*) {
scroll_offset.* = target;
} else if (visible_height > 0 and target >= scroll_offset.* + visible_height) {
scroll_offset.* = target - visible_height + 1;
}
}
/// While the inline note-input bar is open, keep its bottom line
/// (the hint row, below the in-progress input line) on screen.
/// Without this, acknowledging a finding that sits on the last
/// visible row opens the whole input bar below the viewport and
/// the user types blind. Called when entering ack mode and after
/// each committed fragment (which grows the bar by one line).
fn ensureAckInputVisible(state: *State, app: *App) void {
if (state.input_mode != .ack_note) return;
const cv = cursorVisualRow(state) orelse return;
const fv = state.findings_view orelse return;
const local = cursorLocalIndex(state);
if (local >= fv.rows.len) return;
const bottom = cv + expansionLineCount(fv.rows[local]) + inputBarLineCount(state);
ensureRowVisible(bottom, &app.scroll_offset, app.visible_height);
}
/// Compute the cursor's content-area row index (0 = first
/// rendered line). Returns null when neither view is loaded or
/// when the section indices haven't been populated yet (first
@ -765,7 +811,13 @@ fn ackCurrentFinding(state: *State, app: *App) void {
// input mode from `.normal`, and `commitAckNote`/`cancelAckNote`
// both clear it on exit). Defensive: clear anyway.
clearNoteFragments(state, app.allocator);
app.setStatus("Type reasoning. Enter = next line. Ctrl+Enter = save. Esc = cancel.");
// No setStatus: the `statusOverride` hook renders the input
// prompt while `input_mode == .ack_note` and the App restores
// the tab's default hint automatically when we exit the mode
// (on Esc or commit) - no stale prompt left frozen on the bar.
// Scroll so the freshly-opened input bar is actually on screen
// even when the finding sat on the last visible row.
ensureAckInputVisible(state, app);
}
/// Un-acknowledge the cursor-selected finding. Flips the journal
@ -876,7 +928,9 @@ fn cancelAckNote(state: *State, app: *App) void {
state.input_mode = .normal;
state.note_len = 0;
clearNoteFragments(state, app.allocator);
// No setStatus: the disappearing input bar is its own feedback.
// No setStatus needed: flipping `input_mode` back to `.normal`
// makes `statusOverride` return null, so the App falls straight
// back to the tab's default hint - the bar visibly resets.
}
/// Commit the in-progress ack note: dupe any final unflushed
@ -943,10 +997,11 @@ fn commitAckNote(state: *State, app: *App) void {
};
rebuildFindingsView(state, app);
// No setStatus on success: the visible removal of the row
// from the findings list (or the "[acked]" prefix when
// show_acked is on) is feedback enough. Leaving the prior
// help/hint visible is the user's preference.
// No setStatus on success: the `defer` above flips `input_mode`
// back to `.normal`, so `statusOverride` returns null and the
// bar falls back to the tab's default hint. The row leaving the
// findings list (or gaining an "[acked]" prefix when show_acked
// is on) is the confirmation.
}
// Data loading
@ -1416,19 +1471,16 @@ pub fn buildStyledLines(state: *State, app: *App, arena: std.mem.Allocator) ![]c
return lines.toOwnedSlice(arena);
}
/// Glyph used in the findings table's severity column. Each is two
/// display columns wide (emoji-presentation) so renderers don't have
/// to width-correct.
/// Per-finding-row glyph indicating severity. Each glyph string
/// includes a trailing U+FE0F variation selector to force emoji
/// presentation. The variation selector also serves a critical
/// rendering role here: `drawStyledContent` allocates one buffer
/// cell per UTF-8 sequence (advancing col by 1), so a single-
/// codepoint emoji like takes 1 buffer cell while terminals
/// render it as 2 visual cols, desyncing the col counter from
/// terminal display. Appending FE0F gives it a second codepoint
/// (which gets its own cell) so buffer-col advancement matches
/// terminal-col advancement at exactly 2 per emoji.
/// (two-display-column) presentation. The selector is also the
/// signal `drawStyledContent` keys off: when it sees a sequence
/// followed by FE0F it folds both codepoints into one buffer cell
/// and marks it `width = 2`, so vaxis advances the terminal cursor
/// by the true width. Without that, vaxis would track the emoji as
/// one column and its scroll-time partial redraws would smear stale
/// glyphs. (The renderer owns the width fix now; the FE0F here just
/// guarantees the glyph it's correcting is genuinely 2 wide.)
fn severityGlyph(sev: observations.Severity) []const u8 {
return switch (sev) {
.warn => "⚠️", // U+26A0 + FE0F
@ -1447,9 +1499,8 @@ fn severityGlyph(sev: observations.Severity) []const u8 {
/// in the renderer means the renderer is ready when the async
/// path lands.
/// Per-check status-grid glyph. See `severityGlyph` for the
/// FE0F-trailing convention - it forces emoji presentation and
/// gives the renderer a second cell to track so buffer-col
/// advancement matches terminal-col advancement.
/// FE0F-trailing convention - it forces emoji presentation and lets
/// `drawStyledContent` mark the cell two columns wide.
fn checkStatusGlyph(result: observations.CheckResult) []const u8 {
return switch (result) {
.pass => "\u{FE0F}",
@ -2916,6 +2967,95 @@ test "onCursorMove: empty table returns false" {
state.findings_view = null;
}
// ensureRowVisible / ack-input scroll-into-view
test "ensureRowVisible: scrolls down so a below-viewport row is last visible" {
var off: usize = 0;
ensureRowVisible(23, &off, 10);
try testing.expectEqual(@as(usize, 14), off); // 23 - 10 + 1
}
test "ensureRowVisible: scrolls up so an above-viewport row is first visible" {
var off: usize = 30;
ensureRowVisible(5, &off, 10);
try testing.expectEqual(@as(usize, 5), off);
}
test "ensureRowVisible: no change when the row is already on screen" {
var off: usize = 10;
ensureRowVisible(12, &off, 10);
try testing.expectEqual(@as(usize, 10), off);
}
test "ensureAckInputVisible: pulls a bottom-row ack input bar onto screen" {
// One finding, no holdings -> unified cursor 0 is that finding.
// Pretend it renders at content row 20 (findings_first_row) and
// the viewport is only 10 rows tall starting at offset 0, so the
// freshly-opened input bar (detail + in-progress + hint = 3 rows
// below the finding) sits entirely below the fold.
var findings = [_]observations_view.FindingRow{
.{ .severity = .warn, .kind = "k", .target = "t", .text = "x", .is_acked = false },
};
const empty_view: review_view.ReviewView = .{
.rows = &.{},
.totals = std.mem.zeroes(review_view.ReviewTotals),
.as_of = zfin.Date.fromYmd(2026, 6, 8),
.total_liquid = 0,
.portfolio_path = "x",
};
var state: State = .{
.view = empty_view,
.cursor = 0,
.expanded_finding = 0,
.input_mode = .ack_note,
.findings_first_row = 20,
.findings_view = .{
.rows = &findings,
.total_active = 1,
.total_acked = 0,
.total_resolved = 0,
},
};
var app: App = undefined;
app.scroll_offset = 0;
app.visible_height = 10;
ensureAckInputVisible(&state, &app);
// finding at row 20; bottom of the bar at 20 + 1 + 2 = 23; the
// offset advances so row 23 is the last visible: 23 - 10 + 1 = 14.
try testing.expectEqual(@as(usize, 14), app.scroll_offset);
state.view = null;
state.findings_view = null;
}
test "ensureAckInputVisible: no-op when not collecting a note" {
var state: State = .{ .input_mode = .normal, .findings_first_row = 20 };
var app: App = undefined;
app.scroll_offset = 7;
app.visible_height = 10;
ensureAckInputVisible(&state, &app);
try testing.expectEqual(@as(usize, 7), app.scroll_offset);
}
// statusOverride (ack-note prompt)
test "statusOverride: null in normal mode, key hints while collecting a note" {
var state: State = .{};
var app: App = undefined;
try testing.expect(tab.statusOverride(&state, &app) == null);
state.input_mode = .ack_note;
const ov = tab.statusOverride(&state, &app) orelse return error.TestUnexpectedResult;
switch (ov) {
.hint => |hint| {
try testing.expect(std.mem.indexOf(u8, hint, "Ctrl+Enter") != null);
try testing.expect(std.mem.indexOf(u8, hint, "Esc") != null);
},
else => return error.TestUnexpectedResult,
}
}
test "onCursorMove: wheel-sized delta still moves by one row" {
// Wheel events arrive as ±3 per detent. The cursor should
// step by ±1 regardless - wheel == j/k, not "skip three rows".