Skip to content

fix(chat): restore table vertical scroll and cell word wrapping - #248

Merged
mrohan-sq merged 2 commits into
block:mainfrom
mrohan-sq:fix/table-text-cutoff
Sep 1, 2026
Merged

mrohan-sq merged 2 commits into
block:mainfrom
mrohan-sq:fix/table-text-cutoff

Conversation

@mrohan-sq

@mrohan-sq mrohan-sq commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

Summary

Tables in chat responses were cutting off text. Two CSS issues combined to cause this:

  1. overflow-y: hidden on the table scroll container overrode Streamdown's default overflow-y: auto, completely disabling vertical scrolling. Combined with Streamdown's default maxHeight: 300px, any table taller than 300px had its bottom rows clipped with no way to scroll.

  2. No overflow-wrap on td cells — long unbreakable strings (URLs, file paths, code identifiers) caused horizontal overflow instead of wrapping within cells.

PR #207 (smaller wrapping headers) made this worse: wrapping headers narrow columns → body text wraps more → rows get taller → more content falls below the 300px cutoff.

Changes

  • overflow-y: hiddenoverflow-y: auto on the table scroll container (globals.css)
  • Added overflow-wrap: break-word to td cells (globals.css)
  • Updated globals.streamdown-table.test.ts to assert the new behavior

Related issue

none found

Testing

  • All existing table-related tests pass (2 table style tests + 2 scrollbar tests + 9 message tests)
  • Pre-commit hooks passed (biome, typecheck, i18n, design-system checks, berdctl contract)

The table scroll container had overflow-y: hidden, which clipped rows
below Streamdown's 300px max-height with no way to scroll. This was
worsened by PR block#207 (smaller wrapping headers → narrower columns →
taller body rows). Long unbreakable strings in td cells also caused
horizontal overflow instead of wrapping.

- Change overflow-y: hidden → auto on the table scroll container
- Add overflow-wrap: break-word to td cells
- Update tests to match
@mrohan-sq
mrohan-sq requested a review from a team August 28, 2026 21:34
Comment thread src/shared/styles/globals.css Outdated

[data-streamdown="table"] td {
font-size: 0.8125rem;
overflow-wrap: break-word;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 overflow-wrap: break-word does not reduce a table cell’s intrinsic minimum width under automatic table layout, so long unbroken URLs, paths, or identifiers can still expand the table and scroll horizontally instead of wrapping. I reproduced this in Chromium with a 100-character token: the table remained 749px wide inside a 300px container. overflow-wrap: anywhere kept it at 300px and wrapped the token. Could we use anywhere here and, ideally, cover the rendered behavior with a browser/layout test? The current test only verifies that the CSS declaration exists.

Per review feedback: overflow-wrap: break-word does not reduce a table
cell's intrinsic minimum width under automatic table layout, so long
unbreakable strings still expand the table. Switch to overflow-wrap:
anywhere, which participates in min-content sizing.

Add a Playwright browser test (table-overflow.spec.ts) that verifies:
- Long unbreakable strings wrap within the cell width (no horizontal
  table expansion)
- Tables taller than max-height scroll vertically (overflow-y: auto)
@mrohan-sq

Copy link
Copy Markdown
Collaborator Author

🤖 Good catch — updated in 084d619.

Changed overflow-wrap: break-wordoverflow-wrap: anywhere, which participates in min-content sizing so long unbreakable strings wrap within the cell instead of expanding the table.

Also added a Playwright browser layout test (table-overflow.spec.ts) that verifies:

  1. A 200-char unbreakable string in a td wraps within a 300px container (no horizontal expansion)
  2. A 50-row table scrolls vertically (scrollHeight > clientHeight, overflow-y: auto)

The browser test uses page.setContent() with inline CSS so it runs from a standalone Playwright config (playwright.table-overflow.config.ts) without needing a built dist/.

@mrohan-sq
mrohan-sq merged commit a8c08fc into block:main Sep 1, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants