fix(web): the fade follows measured overflow, so text that fits is not dimmed forever#554
Merged
Conversation
…t dimmed forever #545 made the Show more toggle depend on MEASURED overflow instead of the length heuristic, but left the fade on the collapsed state alone: clamped = canClamp && !expanded // fade rode on this showToggle = canClamp && overflows // toggle rode on this A message long enough to trip COLLAPSE_LINE_THRESHOLD / CHAR_THRESHOLD but short enough to fit inside max-h-80 therefore rendered with the bottom of its box faded to transparent and NO toggle to clear it. Before #545 the toggle was gated on the heuristic too, so the same message faded but could be expanded; the fade was escapable. #545's note that this case merely 'no longer offers a toggle that does nothing' understates it -- the toggle was doing something visible: it removed the fade. The constraint has to stay applied while collapsed, because overflow is measured THROUGH it (drop max-h and scrollHeight === clientHeight, so nothing ever reports overflowing and the block never clamps). So the fix splits the two: collapsedClassName keeps the constraint, and a new overflowingClassName carries the hint, applied only when the content actually overflows. Verified in a real browser (jsdom has no layout and cannot see a mask): a fitting message reports mask-image: none where it previously reported linear-gradient(...), while a genuinely overflowing message keeps both its fade and its toggle.
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.
Found while browser-verifying #545 after it landed.
The bug
#545 made the toggle depend on measured overflow instead of the length heuristic — good — but left the fade on the collapsed state alone:
clampedappliescollapsedClassName, and forMessageTextthat string carried the fade mask. So a message long enough to tripCOLLAPSE_LINE_THRESHOLD(16 lines) orCOLLAPSE_CHAR_THRESHOLD(1800 chars) but short enough to fit insidemax-h-80renders with the bottom of its box faded to transparent and no toggle to clear it. The text is simply dimmed, permanently.Before #545 the toggle was gated on the heuristic too, so that message faded but could be expanded — the fade was escapable. #545's note that this case "no longer offers a toggle that does nothing" understates it: the toggle was doing something visible. It removed the fade.
Easy to hit with markdown that renders more compactly than its source — blank-line-padded text is the obvious one.
The fix
The constraint has to stay applied while collapsed, because overflow is measured through it: drop
max-h-80andscrollHeight === clientHeight, so nothing ever reports overflowing and the block never clamps at all. GatingcollapsedClassNameonoverflowswould be circular.So the two concerns split:
collapsedClassName— the size constraint, applied whenever collapsed (needed to measure)overflowingClassName— the hint that there is more to see, applied only when it actually overflowsMessageRow's clamp is unaffected:line-clamp-3is a no-op when the content fits, so it never needed the split.Verification
Real browser (Chromium, built + preview), reading computed style — jsdom has no layout and cannot see a mask:
Show moremask-imagebeforelinear-gradient(rgb(0,0,0) 70%, rgba(0,0,0,0))noneAlso confirmed #545's own win is intact — "Show more" still grows the reply (111px → 725px → 111px), nowhere near the #538 shrink (360 → 87).
pnpm lint✓ ·pnpm -r typecheck✓ · 761 web unit tests ✓cluster-steer-interleave+session-pane-ux+spine-aside: 7 passedscrollHeight/clientHeight+ResizeObserver, so the class-level behaviour is testable even though the visual is not)