EnxDev commented on PR #41135: URL: https://github.com/apache/superset/pull/41135#issuecomment-4809451007
## EnxDev's Review Agent โ apache/superset#41135 ยท HEAD f055e94 **comment** โ correct, minimal fix for the reported whitespace collapse (#36042); one edge case worth a conscious call. ### ๐ก Should-fix - **`superset-frontend/src/components/GridTable/index.tsx:175`** โ `white-space: pre` preserves newlines too, not just runs of spaces. A cell value containing `\n` (e.g. multi-line text or JSON in SQL Lab results) will now break across lines and get clipped by the fixed `rowHeight` (`:141`/`:150`), where it previously collapsed to a single line. There's no pure-CSS way to keep intra-line spaces but collapse newlines, so this may well be acceptable โ just worth making it a deliberate trade-off rather than incidental. (test: a Playwright check that a `'a\nb'` value still renders on one line, or an explicit note that multi-line clipping is fine.) ### ๐ต Nits - The Bito bot's horizontal-overflow flag isn't a real concern: `.ag-cell` already clips (`overflow: hidden`) and single-line values still truncate with ellipsis, so `pre` causes no layout overflow. No action needed. ### ๐ Praise - `:170-175` โ comment is timeless and explains the *why*; picking `pre` over the issue's suggested `pre-wrap` correctly keeps row heights and column sizing unchanged. <!-- enxdev-review-agent:f055e94 --> _Reviewed by EnxDev's Review Agent โ @EnxDev ยท HEAD f055e94._ -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
