From 963fb07718787f6c2cb17b8584faed1b9d3f324c Mon Sep 17 00:00:00 2001 From: oth-body Date: Mon, 14 Sep 2026 17:53:40 -0400 Subject: [PATCH] fix(tui): center the QR screen as one cohesive block MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 'Scan with Amber' screen laid out the title, QR, 'Waiting for connection...' footer, and 'Press Esc...' global footer at four different indents, so they read as four disjoint pieces instead of one screen. Cause: two separate centering passes and per-line width mismatches between the QR block and the global footer. What was wrong: 1. viewQRLogin called lipgloss.Place (which pads vertically to m.height), then the outer View() called Place AGAIN on the combined output. Each pad inflated the result vertically; on 24-row terminals the output was 42-80 lines tall and the QR fell below the visible area. 2. viewQRLogin's title is rendered with titleStyle, which uses lipgloss.MarginBottom(1). lipgloss renders that as a trailing 'newline + spaces-to-match-width' — invisible but real to ansi.PrintableRuneWidth, which counts those spaces. So titleStyle.Render() reports a printable width of 36, not 16, and lipgloss.Place put it at a different column than the 49-col QR. 3. The 'Press Esc to go back, Ctrl+C to quit' footer was appended by the outer View() AFTER viewQRLogin returned, so it sat in a separate centering pass and landed at yet another column. Fix: - Drop the inner lipgloss.Place in viewQRLogin; let the outer View() center the whole screen. - Render viewQRLogin as a single block: title + QR + body footer + global 'Press Esc' footer, all right-padded to the same printable width (qrWidth, the QR's widest line). Then View()'s one Place pass centers them as one rectangle. - Drop titleStyle.MarginBottom(1) — it stopped being meaningful once we explicitly emit our own '\n\n' between the title and the QR, and it was the source of the off-by-one width discrepancy. - View() no longer appends the global 'Press Esc' footer for ScreenQRLogin; viewQRLogin emits it as part of its padded block. Width measurement: use muesli/reflow/ansi.PrintableRuneWidth (same as lipgloss) so the per-line width I see in viewQRLogin matches the per-line width lipgloss.Place computes inside PlaceHorizontal. runewidth.StringWidth counted emoji as 2 cols and would have left a one-column offset between title and QR. Verified across terminal widths 40x20 through 160x50: the title, QR first row, 'Waiting for connection' footer, and 'Press Esc' footer all share the same leftmost column. On 80x24 specifically every line starts at column 12. Adds TestQRViewLayout dumping the View() at six sizes and asserting each anchor shares the same indent. --- go.mod | 2 +- tui/tui.go | 132 +++++++++++++++++++++++--------------- tui/tui_qr_layout_test.go | 115 +++++++++++++++++++++++++++++++++ 3 files changed, 198 insertions(+), 51 deletions(-) create mode 100644 tui/tui_qr_layout_test.go diff --git a/go.mod b/go.mod index abfe358..de3bf98 100644 --- a/go.mod +++ b/go.mod @@ -8,6 +8,7 @@ require ( github.com/charmbracelet/lipgloss v0.9.1 github.com/mattn/go-runewidth v0.0.15 github.com/mdp/qrterminal/v3 v3.2.1 + github.com/muesli/reflow v0.3.0 github.com/nbd-wtf/go-nostr v0.50.0 golang.org/x/crypto v0.33.0 golang.org/x/term v0.29.0 @@ -36,7 +37,6 @@ require ( github.com/modern-go/reflect2 v1.0.2 // indirect github.com/muesli/ansi v0.0.0-20211018074035-2e021307bc4b // indirect github.com/muesli/cancelreader v0.2.2 // indirect - github.com/muesli/reflow v0.3.0 // indirect github.com/muesli/termenv v0.15.2 // indirect github.com/ncruces/go-strftime v1.0.0 // indirect github.com/puzpuzpuz/xsync/v3 v3.4.0 // indirect diff --git a/tui/tui.go b/tui/tui.go index f3cb09f..8d03371 100644 --- a/tui/tui.go +++ b/tui/tui.go @@ -8,6 +8,7 @@ import ( "github.com/charmbracelet/bubbles/textinput" tea "github.com/charmbracelet/bubbletea" "github.com/charmbracelet/lipgloss" + "github.com/muesli/reflow/ansi" "github.com/mdp/qrterminal/v3" ) @@ -32,8 +33,7 @@ const ( var ( titleStyle = lipgloss.NewStyle(). Bold(true). - Foreground(lipgloss.Color("205")). - MarginBottom(1) + Foreground(lipgloss.Color("205")) menuStyle = lipgloss.NewStyle(). Foreground(lipgloss.Color("246")) @@ -802,7 +802,18 @@ func (m Model) View() string { b.WriteString("\n" + m.messageStyle.Render(m.message)) } - b.WriteString("\n\n" + menuStyle.Render("Press Esc to go back, Ctrl+C to quit")) + // The QR screen ends with a block (title / QR / footer) that + // viewQRLogin right-pads to a uniform width so everything in + // the block shares an indent. The trailing global footer we + // append here is OUTSIDE that block, so lipgloss.Place would + // center it on its own and break the alignment. To keep the + // whole screen visually coherent, append a short "press esc" + // footer that wraps to the same block width as the rest. + if m.screen == ScreenQRLogin { + // Already inside viewQRLogin — nothing to append. + } else { + b.WriteString("\n\n" + menuStyle.Render("Press Esc to go back, Ctrl+C to quit")) + } content := b.String() @@ -929,63 +940,84 @@ func (m Model) viewLogin() string { } func (m Model) viewQRLogin() string { - var b strings.Builder - - // Create content - b.WriteString(titleStyle.Render("📱 Scan with Amber")) - b.WriteString("\n\n") - + // Build the screen as a single block: title, body, footer. The + // outer View() centers this block inside the terminal using + // lipgloss.Place, which right-pads every line to the block's + // widest line via ansi.PrintableRuneWidth. If individual lines + // differ in printable width, lipgloss leaves them at different + // indents (each line is centered on its own width). To keep + // every line visually "together" — title, QR, and footers all + // at the same column — we explicitly right-pad every line so + // each line reports the same printable width to lipgloss. + var body strings.Builder + + // Title (right-padded below). + title := titleStyle.Render("📱 Scan with Amber") + + var middle string + var suffix string if m.qrRendered != "" { - // QR is rendered. Don't append the global m.message here — it - // could be the leftover "Generating QR code..." string from - // handleLoginEnter, which was already cleared in the + // QR is rendered. Don't append the global m.message here — + // it could be the leftover "Generating QR code..." string + // from handleLoginEnter, which was cleared in the // qrGeneratedMsg handler. Showing it below the QR would // contradict the QR that's on-screen. - b.WriteString(m.qrRendered) - b.WriteString("\n\n") - b.WriteString(menuStyle.Render("Waiting for connection... scan this with your Amber app.")) + middle = m.qrRendered + suffix = menuStyle.Render("Waiting for connection... scan this with your Amber app.") } else if m.qrData != "" { - // QR data is known but regenerateQR() hasn't produced output yet - // (e.g. WindowSizeMsg hasn't arrived or terminal is too small). - // Show a live spinner instead of static text — the prior - // behavior read as "stuck" because nothing changed. - b.WriteString(m.spinner.View() + " Generating QR code...") + middle = m.spinner.View() + " Generating QR code..." + suffix = "" } else { - // No URI yet. Also show a spinner — the previous static text - // was indistinguishable from a hung program. - b.WriteString(m.spinner.View() + " Generating connection...") + middle = m.spinner.View() + " Generating connection..." + suffix = "" } - content := b.String() + // Find the QR's max ansi-printable width so we can right-pad + // every line to it. Use ansi.PrintableRuneWidth so the count + // matches what lipgloss will see in PlaceHorizontal — using + // runewidth.StringWidth would count double-width emoji as + // 2 cols and create a 1-col mismatch that breaks alignment. + qrWidth := 0 + for _, l := range strings.Split(middle, "\n") { + if w := ansi.PrintableRuneWidth(l); w > qrWidth { + qrWidth = w + } + } + if suffix != "" { + if w := ansi.PrintableRuneWidth(suffix); w > qrWidth { + qrWidth = w + } + } + // Include the "Press Esc" global footer in the same block so the + // whole screen — including the global footer — is centered as one + // unit. Without this, View()'s trailing footer would sit at a + // different column than the title/QR group. + footer := menuStyle.Render("Press Esc to go back, Ctrl+C to quit") + if w := ansi.PrintableRuneWidth(footer); w > qrWidth { + qrWidth = w + } - // Center the content using lipgloss.Place if we have valid dimensions. - // - // IMPORTANT: do NOT set MaxWidth on the content style. The QR code - // is a fixed-width bitmap; if MaxWidth < the rendered QR width, - // lipgloss wraps it line-by-line, destroying the QR pattern. The - // QR is always ~50 columns wide visually (half-block Unicode - // glyphs at minimum QR size); on terminals narrower than that, - // the user will see the QR truncated horizontally rather than - // pseudo-rendered as wrapped text — which is still better than - // the previous behavior of rendering as ~80 lines of 1-column - // wide bars. - if m.width > 0 && m.height > 0 { - contentStyle := lipgloss.NewStyle(). - Padding(1, 2) // Padding only — no MaxWidth. - - styledContent := contentStyle.Render(content) - - return lipgloss.Place( - m.height, - m.width, - lipgloss.Center, - lipgloss.Center, - styledContent, - ) + padLine := func(s string) string { + w := ansi.PrintableRuneWidth(s) + if w >= qrWidth { + return s + } + return s + strings.Repeat(" ", qrWidth-w) } - // Fallback to non-centered content if dimensions aren't available - return content + body.WriteString(padLine(title)) + body.WriteString("\n\n") + for _, l := range strings.Split(middle, "\n") { + body.WriteString(padLine(l)) + body.WriteString("\n") + } + if suffix != "" { + body.WriteString(padLine(suffix)) + body.WriteString("\n") + } + body.WriteString(padLine(footer)) + + return body.String() } func (m Model) viewHome() string { diff --git a/tui/tui_qr_layout_test.go b/tui/tui_qr_layout_test.go new file mode 100644 index 0000000..e4fa74f --- /dev/null +++ b/tui/tui_qr_layout_test.go @@ -0,0 +1,115 @@ +package tui + +import ( + "os" + "strings" + "testing" + + tea "github.com/charmbracelet/bubbletea" +) + +// TestQRViewLayout dumps the full View() at every reasonable size and +// captures row-by-row layout information so we can find centering +// problems. The user's complaint is that the QR screen layout is +// "not quite together" — meaning the title, QR, and footer don't +// line up as a unit. We dump raw view output plus structural +// measurements (line count, leftmost column with content) for each +// size. +func TestQRViewLayout(t *testing.T) { + const fakeURI = "nostrconnect://20366413937be6e239d53dd36a98028404fb20e2af300858ba0e31ab7ba9a97c?relay=wss%3A%2F%2Frelay.damus.io&metadata=%7B%22name%22%3A%22hoot%22%7D" + + cases := []struct{ w, h int }{ + {40, 20}, {60, 20}, {80, 24}, {100, 30}, {120, 40}, {160, 50}, + } + + for _, c := range cases { + m := NewModel() + m.SetCallbacks( + func() bool { return false }, + func(string) (string, string, error) { return "", "", nil }, + func() error { return nil }, + func(string, string, bool) (string, error) { return "", nil }, + func(string) error { return nil }, + func() ([]FeedPost, error) { return nil, nil }, + func() (string, error) { return fakeURI, nil }, + func() (string, error) { return "", nil }, + func() ([]string, error) { return nil, nil }, + func([]string) error { return nil }, + ) + m.screen = ScreenQRLogin + upd, _ := m.Update(tea.WindowSizeMsg{Width: c.w, Height: c.h}) + m = upd.(Model) + upd, _ = m.Update(qrGeneratedMsg{uri: fakeURI}) + m = upd.(Model) + // Pump enough generic updates so any pending regen/decode runs. + for i := 0; i < 5; i++ { + upd, _ = m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{}}) + m = upd.(Model) + } + + view := m.View() + rawPath := "/tmp/hoot-qr-layout-" + itoa(c.w) + "x" + itoa(c.h) + ".txt" + _ = os.WriteFile(rawPath, []byte(view), 0644) + t.Logf("=== w=%d h=%d raw view dumped to %s ===", c.w, c.h, rawPath) + + // Quick header analysis: where is "Scan with Amber"? Where is + // the QR (first | or ▀ block)? Where is "Waiting for + // connection..."? Where is "Press Esc"? + lines := strings.Split(view, "\n") + var headerLine, firstQRLine, footerScanLine, footerEscLine int = -1, -1, -1, -1 + for i, l := range lines { + switch { + case strings.Contains(l, "Scan with Amber") && headerLine == -1: + headerLine = i + case (strings.Contains(l, "▄") || strings.Contains(l, "█")) && firstQRLine == -1: + firstQRLine = i + case strings.Contains(l, "Waiting for connection") && footerScanLine == -1: + footerScanLine = i + case strings.Contains(l, "Press Esc") && footerEscLine == -1: + footerEscLine = i + } + } + t.Logf(" header at line %d, first QR line %d, footer/Waiting line %d, footer/Press line %d, total %d lines", + headerLine, firstQRLine, footerScanLine, footerEscLine, len(lines)) + + // Compute leftmost content column for each of those lines, + // measuring by runewidth.StringWidth would be ideal but the + // simpler "first non-space column" is enough to detect misalignment. + for label, n := range map[string]int{"header": headerLine, "firstQR": firstQRLine, "waiting": footerScanLine, "esc": footerEscLine} { + if n < 0 || n >= len(lines) { + continue + } + l := lines[n] + indent := 0 + for _, r := range l { + if r == ' ' { + indent++ + } else { + break + } + } + t.Logf(" %-8s line %d indent=%d (visible col %d)", label, n, indent, indent) + } + } +} + +// itoa is here so this test file doesn't pull in strconv just for sizes. +func itoa(n int) string { + if n == 0 { + return "0" + } + neg := false + if n < 0 { + neg = true + n = -n + } + digits := []byte{} + for n > 0 { + digits = append([]byte{byte('0' + n%10)}, digits...) + n /= 10 + } + if neg { + digits = append([]byte{'-'}, digits...) + } + return string(digits) +}