Repository navigation
fix(chat): size a list's indent to its widest marker in the Chat font - #2096
Merged
Merged
Conversation
Fixes #1993, reported by @Hyd3-14. A list's markers sit outside its items, in the list's indent, and are drawn in the Chat font; a fixed 20px cut the first digit of "1." off in a monospace Chat font, and of "10." and up in the default face. The indent is now the widest marker in cells of the list's own font, never less than 20px: max(20px, 2ch) for bullets and max(20px, (digits + 2)ch) for numbers, the digits of the widest number passed in from Markdown.tsx (a start past a 32-bit integer, which the browser ignores, sets no wider indent). The new demo regression measures every marker in its own font against its list's indent at a phone's and a desktop's width.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 11 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Owner
Author
|
CI notes for the merge pass:
|
Owner
Author
|
Merging 44e550d: omo-native-code-reviewer and Codex both pass this head (LOW notes only), and the required checks are green. CodeRabbit did not review this head. The LOW notes go to a follow-up issue. |
This was referenced Oct 28, 2026
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.
Fixes #1993, reported by @Hyd3-14.
A list's markers sit outside its items, inside
.markdown-list's fixedpadding-left: 20px, and are drawn in the Chat font. In a monospace Chat font each digit and the.after it take a whole cell, so1.is about 27px at the chat size and its first digit was cut off at the transcript's left edge. Lists that reach10.overflowed even in the default Pretendard face (10.is 26px there).Summary
src/components/ChatView.css: the indent is the widest marker measured in cells (ch) of the list's own font, and never less than the 20px it was:max(var(--space-5), 2ch). In the default face at the default size this is still 20px. It grows only where•needs more, for example monospace at 24px, where the bullet was clipped too.max(var(--space-5), calc((var(--markdown-list-digits) + 2) * 1ch)). MonospaceN.is exactlydigits + 2cells, whatever the face's cell width (HackGen35 is a 3:5 face, so its cell is 0.6em), and a proportional face's.is narrower than two cells.--markdown-list-digitsresets to 1 on every list, so a nested list does not inherit its parent's value.src/components/Markdown.tsx: an ordered list with a multi-digit widest number sets--markdown-list-digitsinline. Past a 32-bit integer the browser ignoresstartand counts from 1, so such a start sets no wider indent.src/lib/markdown.test.ts: covers the digit count, including nested lists and the 32-bit limit.scripts/chat-list-markers-demo-regression.ts(new, inscripts/ci-browser.sh): runs the unmodified app over the demo's fixture transport, at 390px and 1440px, in the default face, in monospace, and in monospace at the largest Chat size (24px). It measures every marker of a reply's lists (1-3, 8-12, 99-100, bullets and a nested bullet) as the browser draws it, in the marker's own computed font, and checks that it fits its list's indent. In the default face it also checks that a bullet list keeps its 20px.changelog.d/chat-list-number-indent.md.Visible change
In the default face, a numbered list whose numbers are all one digit now indents
3ch, about 27px at 15px, instead of 20px. Bullet lists keep 20px. CSS cannot give these the old 20px and still fit monospace: any indent built fromchandemthat fits monospace1.is wider than 20px in Pretendard, whosech(0.6em) is wider than a monospace one (0.5em). Telling the face apart would take a runtime measurement, so I did not try.Design precedent
This follows T3 Code. Its chat
olrenderer widens an inherited--list-gutterwith an inline style for lists whose widest marker has more than one character, solist-style-position: outsidedoes not paint the marker past the padding box. Everyolandulresets the variable, so nested lists keep their own value (index.css#L1846-L1868). The difference is that T3's gutter is inrem, while ours is inch: our Chat font is the user's choice, and the marker is drawn in it.Verification
bun scripts/chat-list-markers-demo-regression.tsfails on main at its first case (default face, 390px:10.to12.and99.are 26px and100.is 35.2px, all in a 20px indent). With this fix it prints PASS for all six cases (390/1440 × default/monospace/monospace 24px). The monospace markers fill their indents exactly (100.45.2px in 45.2px).1.as.and the default face clips the first digit of10.and up; with the fix every number is whole.bun test src/lib/markdown.test.ts: the new digit-count case fails when the 32-bit limit is removed (it counts 21 digits) and passes with it.bun run check fastgreen (7720 pass) on the tree before the rebase. Main's commits since then touch none of these files, and typecheck plus the touched test files pass again on the rebased head.check fastrun failed in two files this PR does not touch,scripts/check.test.ts(/tmp/...: No space left on device) andscripts/ci-browser.test.ts(a 5 s timeout at load 93 on 128 cores)./tmpthen had 36G free, and the rerun was green.Reviews
100000000000000000000.) gave a 21-digit indent (about 330px) for a list the browser numbers from 1. Fixed inMarkdown.tsx, with a unit test. Chrome was probed for this:start="2147483648"reads back asstart 1.clip-pathadded to the list would pass).