Width: stepper, NAWS diagnostics, honor high MinColumns (font floor + WebView min-font)#13
Merged
Merged
Conversation
1. Min columns is now a typeable −/value/+ stepper instead of a slider — far easier to set an exact width on a phone. Range widened to 20–500 (clamped on every change). Max text size stays a slider. 2. Added diagnostic logging around the column-fit math so wrap issues can be traced from the exported log. measureGrid now returns the values it bases its decision on (clientW, contentW, padX, advanceRatio, fitted fontPx, targetCols, and actualColsThatFit — the count that genuinely fits at the fitted font), and SessionScreen logs them at Information (captured by FileLoggerProvider -> the on-device log). When actualColsThatFit < targetCols, the advertised width can't be shown and lines wrap — now visible in the log with fontPx (vs the 6px floor) and contentW (vs clientW, showing scrollbar/inset loss). Verified: Web + Android build clean; UI bUnit 47/47; stepper works in the Web preview (buttons adjust, old slider gone); the "NAWS fit: ..." line is emitted. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HHCRc5CJ6595iMzEgYYdQp
There was a problem hiding this comment.
Pull request overview
This PR improves layout settings usability by replacing the Min columns slider with a numeric stepper, and adds richer NAWS/column-fit diagnostics by returning measurement details from measureGrid and logging them from SessionScreen for exportable on-device troubleshooting.
Changes:
- Replace Min columns slider with a typeable
− / value / +stepper (range 20–500) and update/add bUnit tests. - Extend
measureGridJS interop to return diagnostic metrics (actual cols that fit, padding, widths, scrollbar presence, etc.). - Log NAWS fit diagnostics from
SessionScreento support wrap/fit issue investigation via exported device logs.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/SharpClient.UI.Tests/SettingsViewTests.cs | Updates Min columns UI tests for the new stepper and adds button-adjustment coverage. |
| src/SharpClient.UI/wwwroot/sc-interop.js | Adds computed diagnostic fields to measureGrid output for downstream logging. |
| src/SharpClient.UI/wwwroot/app.css | Introduces styling for the new stepper control. |
| src/SharpClient.UI/Components/SettingsView.razor | Replaces the Min columns slider with a stepper and clamps user input. |
| src/SharpClient.UI/Components/SessionScreen.razor | Injects ILogger and logs NAWS fit diagnostics based on measureGrid results. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+117
to
+127
| // Diagnostics for column/wrap issues — captured to the on-device log (FileLoggerProvider) so it | ||
| // can be exported. actualCols < targetCols means the advertised width can't be shown and lines | ||
| // wrap; compare fontPx against the 6px floor and contentW against clientW (scrollbar/inset loss). | ||
| Logger.LogInformation( | ||
| "NAWS fit: minCols(setting)={MinColumns} maxFont(cap)={Cap} probeW={ProbeWidth} probeH={ProbeHeight} " + | ||
| "clientW={ClientW} contentW={ContentW} padX={PadX} advanceRatio={AdvanceRatio} fontPx={FontPx} " + | ||
| "targetCols={TargetCols} actualColsThatFit={ActualCols} reportedCols(NAWS)={Cols} rows={Rows} vScroll={VScroll} fits={Fits}", | ||
| Settings.MinColumns, cap, widthPx, heightPx, | ||
| grid.ClientW, grid.ContentW, grid.PadX, grid.AdvanceRatio, grid.FontPx, | ||
| grid.TargetCols, grid.ActualCols, grid.Cols, grid.Rows, grid.VScroll, | ||
| grid.ActualCols >= grid.TargetCols); |
Comment on lines
+109
to
+111
| // Typeable/stepped bounds for Min columns. Wider than the old 60–120 slider so unusual widths | ||
| // are reachable; the value is clamped on every change. | ||
| private const int MinColumnsMin = 20; |
Comment on lines
+173
to
+175
| const advancePx = advanceRatio * fontPx; | ||
| const actualCols = advancePx > 0 ? Math.floor(contentW / advancePx) : 0; | ||
| const vscroll = element.scrollHeight > element.clientHeight; |
…rap) Diagnosis from the exported device log: at MinColumns=90 on a 384px screen (contentW=376), the font floored at 6px and 90 still wrapped, yet the log said actualColsThatFit=104/fits=True. Two bugs: 1. The min-font floor was 6px. Fitting 90 columns in 376px needs ~5.9–6.9px; the closed-loop fit was clamped at 6px and stopped short, so 90 didn't fit and wrapped. Lowered the floor to 4px (passed from SessionScreen) so the font can shrink enough to honor a high MinColumns on a narrow screen. 2. actualColsThatFit was computed with the LINEAR advanceRatio (sampled at 100px). Glyph advance is proportionally larger at tiny sizes (hinting), so it over-counted (104 at 6px when ~78 truly fit) — which is why the diagnostic wrongly said it fit. measureGrid now measures the REAL advance at the fitted size and reports the honest column count; the returned cols (NAWS) is min(targetCols, whatActuallyFits), so it never advertises more than is visible and the client never double-wraps. Verified at ~device width: MinColumns 90 now fits at ~6.7px with no wrap (actualCols 90); 78 stays comfortable at 7.7px; an extreme 200 floors at 4px and honestly reports 150 (server wraps to that, no client wrap). Web + Android build clean; UI bUnit 47/47. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HHCRc5CJ6595iMzEgYYdQp
HarryCordewener
changed the title
Follow-up to the honest-column reporting: on device, NAWS was pinned at ~78 for any Min columns >= ~79. Root cause is the Android WebView MinimumFontSize / MinimumLogicalFontSize (both default 8px): the terminal fits the font down to a few px to pack e.g. 90 columns onto a 376px-wide screen, but the WebView clamped that to 8px, so only ~78 columns actually fit — and measureGrid (now honest) correctly reported ~78, hence the pin. Drop both floors to 1px on the BlazorWebView's Android WebView (next to the insets bridge). The fitted sub-8px size now renders for real, so 90 columns fit (~6.5px) and NAWS reports 90. measureGrid still measures the real rendered advance and reports only what fits, so a genuinely-too-large value degrades gracefully instead of wrapping. Android-only change; builds clean. The existing "NAWS fit: …" log will confirm on the next export: with the clamp gone, fontPx drops below 8 and actualColsThatFit rises to the requested value. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HHCRc5CJ6595iMzEgYYdQp
HarryCordewener
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
1. Min columns: slider → stepper
The slider was hard to set precisely on a phone. Replaced with a typeable
−/ value /+stepper (range 20–500, clamped). Max text size stays a slider.2. Diagnostic logging
measureGridreturns the values it fits against, andSessionScreenlogs aNAWS fit: …line at Information → the on-device log (exportable via Settings → Export log): clientW, contentW, padX, advanceRatio, fontPx, targetCols, actualColsThatFit, reportedCols(NAWS), rows, vScroll.3. Fix: 90-column width wrapped (diagnosed from the exported log)
The log showed
minCols=90 contentW=376 fontPx=6 actualColsThatFit=104 fits=True— yet it wrapped. Two bugs:actualColsThatFitused the linear advance ratio (sampled at 100px). Glyph advance is proportionally larger at tiny sizes (hinting), so it over-counted (104 at 6px when ~78 truly fit) — which is why it wrongly reportedfits=True.measureGridnow measures the real advance at the fitted size and reports the honest count;cols(NAWS) = min(targetCols, whatActuallyFits), so it never advertises more than is visible and the client never double-wraps.Verified (at ~device width)
actualCols=90, no wrap.NAWS fit:line emits.🤖 Generated with Claude Code