Skip to content

feat: draw the comment indicator without colour - #67

Merged
handlename merged 1 commit into
mainfrom
feat/comment-indicator-style
Aug 29, 2026
Merged

feat: draw the comment indicator without colour#67
handlename merged 1 commit into
mainfrom
feat/comment-indicator-style

Conversation

@handlename

Copy link
Copy Markdown
Owner

Why this change is necessary

The comment indicator read 💬 N while any thread on a block was open, and ✅ N once they were all resolved. Both are colour emoji — whatever font ships them draws them in their own hues — which made the indicator the one place on the review screen carrying a colour other than the single accent. That is the thing UI_DESIGN §2.1 exists to prevent: hierarchy is supposed to come from opacity, weight and whitespace, and a green check beside a blue chip competes for attention with the specification the reviewer came to read. The tension was already known and deferred in as many words — the CSS beside the indicator said the chip's palette was "not this change's to settle".

Approach

Both symbols were the right ones; they only had to be drawn rather than typed. An open indicator is now a filled accent chip carrying a speech bubble stroked in currentColor; an all-resolved one gives up its fill for an accent hairline outline with a . That is the same fill-versus-outline channel §2.5 already uses to step a badge down, so the two read as one control in two states rather than as two different marks — and neither spends a hue.

Position, size, hover and the accent fill are untouched. The interview that preceded this change established that none of those was what felt loud.

Two things fell out of the change rather than being chosen:

  • The chip is appended inside the element it marks, so it inherited that element's text metrics. Measured, the same chip came out 5px shorter and a fraction narrower on a heading than on a paragraph. It now fixes its own line-height and letter-spacing, and both glyphs sit in a box of the same size, so two chips carrying the same number of digits are the same size wherever they land.
  • For the same reason the indicator lands in the element's textContent, and three regexes existed to strip 💬 back out of it. With the emoji gone those regexes match nothing, and the count and the would leak into table-of-contents entries, heading slugs and the composer's context line instead. A visibleText() helper replaces them, reading an element with .comment-indicator removed rather than pattern-matching whatever the chip happens to render.

Design Documents

The design documents live in this repository and are part of this diff, so they are linked rather than reproduced here.

  • UI_DESIGN.md — normative for the review screen. The decision is recorded as a new §2.6, together with the alternatives that were weighed and dropped: removing the open/resolved distinction, hiding the chip once everything is resolved, and / or /, which need no SVG and pair more neatly but say "checklist item" and "in progress" rather than "there is discussion here".
  • GLOSSARY.md, DESIGN.md, AGENTS.md — updated to describe the control as it now is.

Review Points

  • Is §2.6 the principle you would have written? UI_DESIGN is normative, so this is the part of the change that outlives the CSS. The claim it makes is that the open/resolved distinction is worth keeping and belongs on the fill-versus-outline channel — not that the emoji were merely ugly.
  • visibleText() covers three call sites — the composer's context line, heading slugs and the rail's table-of-contents labels. It is worth checking whether anything else reads an element's text and would now see a stray count.
  • Two 💬 remain in UI_DESIGN.md on purpose. They quote what the UI used to show, inside paragraphs explaining why something was changed; rewriting them would falsify the record.
  • Verified in a browser, per the repository's own rule for review-screen changes: the built examples/sample.md and examples/sample.diff rendered in headless Chrome, in light and dark. Across h1, h2, p, li, figure, div, tr and pre, in both states, every chip measured the same width and height; in diff mode both states match within diff lines and within file headers. No indicator text leaked into the table of contents or into heading ids.

The chip read 💬 N and ✅ N, and both codepoints have colour glyphs in
every font that ships them. That made the indicator the one place on the
page carrying a hue other than the accent — a green check beside a blue
chip competing with the text under review, which is the thing UI_DESIGN
§2.1 exists to prevent.

Open is now a filled accent chip with a stroked speech bubble drawn in
currentColor; all-resolved gives up its fill for an accent hairline
outline with a ✓, the same fill-versus-outline step-down §2.5 already
uses for a badge. The balloon and the check were the right two symbols
all along — they only had to be drawn rather than typed. Position, size,
hover and the accent fill are unchanged, and §2.6 now records why, along
with the alternatives that were weighed and dropped.

Both glyphs sit in a box of the same size, and the chip fixes its own
line-height and letter-spacing. It is appended inside its target, so it
inherited that target's text metrics: measured, the same chip came out
5px shorter and a fraction narrower on a heading than on a paragraph.
Across H1, H2, P, LI, FIGURE, DIV, TR and PRE, in both states, every
chip is now the same size.

Losing the emoji also breaks three regexes that stripped 💬 out of an
element's textContent. The indicator lives inside the element it marks,
so without them the count and the ✓ leak into TOC entries, heading slugs
and the composer's context line. visibleText() replaces them, reading
the element with .comment-indicator removed rather than pattern-matching
whatever the chip happens to render.

User request: comment indicator を全体のスタイルに合わせた落ち着いたもの
にしたい。インタビューで、うるさいのはフルカラー絵文字だけで位置・塗り・
hover は現行維持と確定。未解決側にもグリフを付け、同じ桁数なら未解決と
全解決でチップ幅を揃えること。Verified in a browser over the built
examples, in light and dark, in both Markdown and diff mode.
@handlename
handlename merged commit 1ee6201 into main Aug 29, 2026
2 checks passed
@handlename
handlename deleted the feat/comment-indicator-style branch August 29, 2026 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant