feat(table): 🎸 add isResponsive prop to control mobile layout behavior by punkbit · Pull Request #881 · ClickHouse/click-ui

punkbit · 2026-03-06T17:15:16Z

Why?

Adds a new isResponsive prop to the Table component. When set to true, the table maintains its standard layout with horizontal scroll on narrow screens instead of automatically converting to a mobile list view. The default behavior (isResponsive={false}) remains unchanged.

The implementation uses a data-responsive-mode data attribute to conditionally apply mobile list styles.

🍰 Original contribution by @maxknv maxknv (#877), retained credit/commit history.

How?

Add isResponsive?: boolean prop (default: true)
Set data-responsive-mode attribute on TableOuterContainer
Wrap all mobile @media queries with [data-responsive-mode="list"] & selector
When isResponsive=false, mobile styles don't apply

Preview?

demo-table-is-responsive-control.mov

…nversion Add new prop to Table component that disables the automatic conversion to mobile list view on narrow screens, keeping the table layout with horizontal scroll instead. Changes: - Add disableMobileListView?: boolean prop (default: false) - Set data-responsive-mode attribute on TableOuterContainer - Wrap all mobile @media queries with [data-responsive-mode="list"] & selector - When disableMobileListView=true, mobile styles don't apply Benefits: - Simple implementation (3 main changes) - No prop threading through nested components - Clean CSS selector approach - Backward compatible (default behavior unchanged) Use case: When you want tables to scroll horizontally on mobile instead of converting to a list view format.

changeset-bot · 2026-03-06T17:15:19Z

🦋 Changeset detected

Latest commit: ac188f5

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@clickhouse/click-ui	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

github-actions · 2026-03-06T17:15:37Z

Claude finished @punkbit's task in 1m 34s —— View job

Code Review

Tasks

Gather context
Read changed files
Analyze conventions and guidelines
Provide review feedback

Nice work on migrating from hardcoded 768px to the theme breakpoint token and using a data-* attribute for the styling hook — that follows the repo's conventions well.

There are a few things worth addressing before merging:

🚨 Must Fix

TableWrapper missing overflow-x: auto: When isResponsive={true}, all the mobile list-view styles are correctly suppressed, but no horizontal scroll is enabled. TableWrapper has overflow: visible, so on narrow screens the table will silently overflow rather than scroll. The PR description promises "horizontal scroll instead" — this is the missing half of that. You'll need to either add overflow-x: auto to TableWrapper conditionally (perhaps via a prop), or add it unconditionally since scroll only activates when content overflows. Fix this →

⚠️ Missing Changeset

The changeset bot already flagged this: a new public prop (isResponsive) was added to TableProps, so a minor changeset is needed. Per .llm/CONVENTIONS.md, run yarn changeset:add and include a usage example. Fix this →

💡 Suggestions (non-blocking)

Prop semantics are a bit inverted: isResponsive={false} (the default) is actually the responsive behavior (it adapts to mobile by showing a list view). isResponsive={true} opts into scroll-only. Naming like responsiveMode?: 'list' | 'scroll' or mobileLayout?: 'list' | 'scroll' would make the intent unambiguous. The current name could confuse future users who expect isResponsive={true} to enable responsive behavior.
Inconsistent quote style in CSS selectors: Some styled components use [data-responsive-mode='list'] (single quotes) while others use [data-responsive-mode="list"] (double quotes). Both are valid CSS but picking one is cleaner.
No tests or stories for the new prop: The test file has no coverage for isResponsive, and the story file has no new story for the scroll-mode behavior. Per conventions, new behavioral states should be tested and have Storybook stories.
Pre-existing: ActionsList still uses a hardcoded border-left: 1px solid (line 482) instead of theme.click.table.cell.stroke — not introduced by this PR but worth a follow-up.

github-actions · 2026-03-06T17:18:16Z

Claude finished @punkbit's task in 1m 10s —— View job

Tasks

Gather context
Read changed files
Analyze conventions and guidelines
Provide review feedback

Nice clean approach using a data-* attribute as a styling hook — that's the right pattern for this repo. The theme breakpoint migration is good too. A few things still need attention before merging:

🚨 Must Fix

TableWrapper missing overflow-x: auto (Table.tsx:501): TableWrapper still has overflow: visible. When isResponsive={true} (scroll mode), the table will silently overflow its container on narrow screens rather than scrolling. The PR description promises "horizontal scroll instead" — this is the missing half of that. Adding overflow-x: auto to TableWrapper unconditionally is safe since scroll only activates when content overflows. Fix this →

⚠️ Missing Changeset

A new public prop (isResponsive) was added to TableProps — a minor changeset is required. Per .llm/CONVENTIONS.md, run yarn changeset:add and include a usage example. Fix this →

💡 Suggestions (non-blocking)

Inverted prop semantics (Table.tsx:968): isResponsive={false} (the default) produces data-responsive-mode="list" — which IS the responsive behavior. isResponsive={true} disables it. This is counterintuitive: users expect isResponsive={true} to mean "this is responsive." The original disableMobileListView or a union like mobileLayout?: 'list' | 'scroll' would make intent unambiguous.
Inconsistent CSS attribute quote style: Some styled components use single quotes ([data-responsive-mode='list'], e.g. lines 354, 398, 413) while others use double quotes ([data-responsive-mode="list"], e.g. lines 386, 426, 449). Both are valid CSS — pick one and be consistent.
No tests or stories: Per conventions (Component.test.tsx, Storybook stories for all states), the new isResponsive behavior should have a test asserting the data-responsive-mode attribute value, and a Storybook story demonstrating the scroll-mode layout.

github-actions · 2026-03-06T17:22:44Z

Claude finished @punkbit's task in 1m 7s —— View job

Tasks

Gather context
Read changed files
Analyze conventions and guidelines
Provide review feedback

Good progress — the changeset is now present and well-formed. The hardcoded 768px → theme breakpoint migration is solid. One must-fix from the previous two reviews is still open:

🚨 Must Fix

TableWrapper still has overflow: visible (Table.tsx:504): When isResponsive={true} (scroll mode), the table will silently overflow its container on narrow screens — there's nothing to enable horizontal scrolling. The PR description promises "horizontal scroll instead," but that half was never implemented. Adding overflow-x: auto to TableWrapper is safe unconditionally (scroll only activates when content overflows). Fix this →

💡 Suggestions (non-blocking)

Inverted prop semantics (Table.tsx:965): isResponsive={false} (default) → data-responsive-mode="list", which IS the responsive behavior. isResponsive={true} → data-responsive-mode="scroll", which opts out of it. The changeset example <Table isResponsive> also reads as "make this table responsive," yet it disables responsive layout. Consider mobileLayout?: 'list' | 'scroll' or keeping the original disableMobileListView name for clarity.
Inconsistent CSS attribute quote style: The [data-responsive-mode=...] selector alternates between single ('list') and double ("list") quotes — e.g. TableRow/StyledColGroup/StyledThead use single, TableData/MobileHeader/SelectData/ActionsList use double. Pick one and be consistent.
No test coverage: Table.test.tsx has no assertions for isResponsive. A test verifying data-responsive-mode="list" vs "scroll" is set on the outer container would be quick to add and guard against regressions.

github-actions · 2026-03-06T17:23:30Z

Claude finished @punkbit's task in 1m 0s —— View job

Tasks

Gather context
Read changed files
Analyze conventions and guidelines
Provide review feedback

Nice approach using a data-* attribute as a styling hook — that's the right pattern for this repo, and migrating from 768px to the theme breakpoint is a good improvement. The changeset is present with a usage example. ✅

One functional gap still needs to be addressed before merging:

🚨 Must Fix

TableWrapper missing overflow-x: auto (Table.tsx:501): TableWrapper has overflow: visible. When isResponsive={true} (scroll mode), the mobile list-view styles are correctly suppressed, but there's nothing enabling horizontal scroll — the table will silently overflow its container on narrow screens. The PR description explicitly promises "horizontal scroll instead." Adding overflow-x: auto to TableWrapper unconditionally is safe since scroll only activates when content overflows. Fix this →

💡 Suggestions (non-blocking)

Inverted prop semantics (Table.tsx:968): isResponsive={false} (the default) produces data-responsive-mode="list" — which IS the responsive behavior. isResponsive={true} disables it. This is counterintuitive; future consumers will likely expect isResponsive={true} to mean "this adapts to mobile." A union prop like mobileLayout?: 'list' | 'scroll' or keeping the original disableMobileListView name would make intent unambiguous.
Inconsistent CSS attribute quote style: Some selectors use single quotes ([data-responsive-mode='list'], e.g. lines 354, 398, 413) and others use double quotes ([data-responsive-mode="list"], e.g. lines 386, 426, 449). Both are valid CSS — pick one throughout.
No tests or stories: Per conventions, the new isResponsive behavior should have a test asserting the data-responsive-mode attribute value, and a Storybook story demonstrating the scroll-mode layout.
Pre-existing: ActionsList still has a hardcoded border-left: 1px solid (line 481) instead of ${theme.click.table.cell.stroke} — not introduced by this PR, but worth a follow-up.

github-actions · 2026-03-06T17:43:41Z

Claude finished @punkbit's task in 1m 19s —— View job

Tasks

Gather context
Read changed files
Analyze conventions and guidelines
Provide review feedback

Good pattern overall — using data-responsive-mode as a CSS styling hook is the right approach for this repo. The changeset is well-formed and the theme breakpoint migration is a nice improvement. Two issues need addressing before merge:

🚨 Must Fix

isResponsive logic is inverted vs. documentation (Table.tsx:968): The code is data-responsive-mode={isResponsive ? 'list' : 'scroll'}, which maps isResponsive={true} → "list" (the old mobile list-view behavior). But the changeset, Storybook story description, and PR description all say isResponsive={true} should enable horizontal scroll (no mobile list view). This is a direct code/docs mismatch — one of them is wrong. The Storybook story also confirms the intent: isResponsive: true is the default arg, and the description reads "maintains standard layout with horizontal scroll." Fix by swapping the ternary: data-responsive-mode={isResponsive ? 'scroll' : 'list'}. Fix this →
TableWrapper still has overflow: visible (Table.tsx:504): When the mobile list-view styles are suppressed (scroll mode), there's nothing enabling horizontal scroll — the table will silently overflow its container on narrow screens. The Storybook story papers over this by wrapping in <div style={{ overflow: 'auto' }}>, but that's an undocumented requirement for consumers. Add overflow-x: auto to TableWrapper unconditionally (safe — scroll only activates when content overflows). Fix this →

💡 Suggestions (non-blocking)

Inconsistent CSS attribute quote style: [data-responsive-mode='list'] (single quotes, e.g. TableRow, StyledColGroup, StyledThead) and [data-responsive-mode="list"] (double quotes, e.g. TableData, MobileHeader, SelectData) are mixed. Both are valid CSS — pick one and be consistent.
No test coverage: Table.test.tsx has no assertions for isResponsive. A quick test verifying data-responsive-mode="list" vs "scroll" on the outer container would guard against regressions (and catch the inversion bug above at CI time).

github-actions · 2026-03-06T17:45:39Z

Claude finished @punkbit's task in 1m 24s —— View job

Tasks

Gather context
Read changed files
Analyze conventions and guidelines
Provide review feedback

Good pattern and solid approach using data-responsive-mode as a CSS hook. The changeset is present and well-formed. The theme breakpoint migration is a clean improvement. Two blocking issues remain open from previous rounds of review — both need to land before merging.

🚨 Must Fix

Inverted ternary AND wrong default (Table.tsx:771,968): The code has isResponsive = true as the default and data-responsive-mode={isResponsive ? 'list' : 'scroll'}. This maps isResponsive={true} → 'list' (mobile list view ON), directly contradicting the changeset, PR description, and Storybook story description which all say isResponsive={true} should produce horizontal scroll (no mobile list view). The fix is two lines:
1. Change default to isResponsive = false
2. Swap the ternary to data-responsive-mode={isResponsive ? 'scroll' : 'list'}
Result: default (false) → 'list' (unchanged old behavior), isResponsive={true} → 'scroll' (matching all docs). Fix this →
TableWrapper still has overflow: visible (Table.tsx:504): When isResponsive={true} suppresses the mobile list-view styles, nothing enables horizontal scroll — the table silently overflows its container on narrow screens. The Storybook story papers over this by wrapping in <div style={{ overflow: 'auto' }}>, but that's an undocumented consumer requirement. Adding overflow-x: auto to TableWrapper unconditionally is safe (scroll only activates when content overflows). Fix this →

💡 Suggestions (non-blocking)

Inconsistent CSS attribute quote style: [data-responsive-mode='list'] (single quotes, e.g. TableRow line 354, StyledColGroup line 398, StyledThead line 413) and [data-responsive-mode="list"] (double quotes, e.g. TableData line 386, MobileHeader line 426, SelectData line 449) are mixed. Both are valid CSS — pick one throughout.
No test coverage: Table.test.tsx has no assertions for isResponsive. A quick test verifying data-responsive-mode="list" vs "scroll" on the outer container would catch the inversion bug above at CI time and guard against future regressions.
Pre-existing: ActionsList has a hardcoded border-left: 1px solid (line 481) instead of ${theme.click.table.cell.stroke} — not introduced by this PR, worth a follow-up issue.

github-actions · 2026-03-06T17:55:29Z

Claude finished @punkbit's task in 1m 51s —— View job

Tasks

Gather context
Read changed files
Analyze conventions and guidelines
Provide review feedback

Good progress overall — the data-responsive-mode hook pattern is correct, the changeset describes the API accurately, and the theme breakpoint migration is clean. Note: previous automated reviews flagged the ternary at line 968 as "inverted," but it is actually correct — isResponsive={true} (default) maps to 'list' (mobile list view on), isResponsive={false} maps to 'scroll' (mobile list view off). This is consistent with both changeset files and the story description.

One functional issue still needs to be addressed before merging:

Must Fix

TableWrapper still has overflow: visible (Table.tsx:504): When isResponsive={false}, the mobile list-view styles are correctly suppressed, but no horizontal scroll is enabled — the table will silently overflow its container on narrow screens. The Storybook story reveals this by working around it with div style={{ overflow: 'auto' }} (story lines 548, 551), but consumers would have no way to know they need this wrapper. Add overflow-x: auto to TableWrapper — safe unconditionally since scroll only activates when content overflows.

Suggestions (non-blocking)

Duplicate changeset files: Both .changeset/fuzzy-laws-know.md and .changeset/shy-toes-feel.md describe the same minor change with nearly identical content. Remove one before merging to avoid a double entry in the changelog.
Inconsistent CSS attribute quote style: [data-responsive-mode='list'] (single quotes, e.g. lines 354, 398, 413, 433, 492, 520) and [data-responsive-mode="list"] (double quotes, e.g. lines 386, 426, 449, 471) are mixed throughout. Both are valid CSS — pick one.
No test coverage: Table.test.tsx has no assertions for isResponsive. A quick test verifying the data-responsive-mode attribute value on the outer container would guard against regressions.

github-actions · 2026-03-06T17:56:44Z

Claude finished @punkbit's task in 1m 21s —— View job

Tasks

Gather context
Read changed files
Analyze conventions and guidelines
Provide review feedback

Good iteration — the changeset semantics are now correct and consistent with the implementation, and the Storybook story docs match the code. Two issues remain before merging.

🚨 Must Fix

Duplicate changeset files: Both .changeset/fuzzy-laws-know.md and .changeset/shy-toes-feel.md describe the exact same change. This will produce two separate changelog entries and a double minor version bump. Delete one of them. Fix this →
TableWrapper missing overflow-x: auto (Table.tsx:504): TableWrapper still has overflow: visible. The Storybook Responsive story works around this by wrapping the table in <div style={{ overflow: 'auto' }}> — but that's an undocumented requirement that consumers won't know about. When isResponsive={false} (scroll mode), the table will silently overflow its container on narrow screens with no horizontal scrollbar. Add overflow-x: auto to TableWrapper unconditionally (safe — scroll only activates when content overflows). Fix this →

💡 Suggestions (non-blocking)

Inconsistent CSS attribute quote style: [data-responsive-mode='list'] (single quotes — TableRow, StyledColGroup, StyledThead, Tbody, ActionsContainer, MobileActions, StyledTable) and [data-responsive-mode="list"] (double quotes — TableData, MobileHeader, SelectData, ActionsList) are mixed throughout the file. Both are valid CSS but pick one for consistency.
No tests: Table.test.tsx has no assertions for isResponsive. A quick test verifying the data-responsive-mode attribute on the outer container would prevent regressions.
Pre-existing: ActionsList still has a hardcoded border-left: 1px solid (line 481) instead of ${theme.click.table.cell.stroke} — not introduced by this PR, worth a follow-up issue.

…nce scroll only activates when content overflows

github-actions · 2026-03-06T18:01:47Z

Claude finished @punkbit's task in 1m 3s —— View job

Tasks

Gather context
Read changed files
Analyze conventions and guidelines
Provide review feedback

Good progress! The latest commit (b764371d) resolved the long-standing overflow-x: auto issue — TableWrapper at line 504 now correctly enables horizontal scroll unconditionally. The changeset content and implementation are now consistent: isResponsive={true} (default) → 'list' (mobile list view on), isResponsive={false} → 'scroll' (horizontal scroll mode). ✅

One blocking issue remains before merge:

🚨 Must Fix

Duplicate changeset files: Both .changeset/fuzzy-laws-know.md and .changeset/shy-toes-feel.md describe the exact same minor change. This will produce two changelog entries and a double minor version bump on release. Delete one. Fix this →

💡 Suggestions (non-blocking)

Inconsistent CSS attribute quote style: [data-responsive-mode='list'] (single quotes — lines 354, 398, 413, 433, 492, 520, 1162) and [data-responsive-mode="list"] (double quotes — lines 386, 426, 449, 471) are mixed throughout the file. Both are valid CSS; pick one for consistency.
Prop semantics are counterintuitive: isResponsive={true} (the default) enables the mobile list view — which is the adaptive/responsive behavior. isResponsive={false} opts into scroll-only — arguably the less responsive behavior. A future reader would likely expect isResponsive={true} to mean "this adapts to viewport width." Consider mobileLayout?: 'list' | 'scroll' for unambiguous intent, or at minimum a JSDoc comment on the prop explaining the inversion.
No test coverage: Table.test.tsx has no assertions for isResponsive. A quick test verifying data-responsive-mode attribute value on the outer container would guard against regressions.

github-actions · 2026-03-06T18:03:53Z

Claude finished @punkbit's task in 1m 22s —— View job

Tasks

Gather context
Read changed files
Analyze conventions and guidelines
Provide review feedback

All previously flagged blockers have been resolved in this round of commits. The PR is in good shape. 🙌

Resolved since last review:

✅ TableWrapper now has overflow-x: auto unconditionally (commit b764371d)
✅ Duplicate changeset removed — only fuzzy-laws-know.md remains
✅ CSS attribute quote style is now consistent throughout (all single quotes)
✅ Changeset accurately documents the API (isResponsive={false} → scroll mode)

💡 Suggestions (non-blocking)

Story default is unintuitive: The Responsive story defaults to isResponsive: true (list view — the old behavior), while its description text explains isResponsive={false} (the new scroll behavior). A user opening this story in Storybook sees the old behavior by default and has to toggle the control to see what the story is actually about. Consider defaulting to isResponsive: false with maxWidth: '400px' so the scroll behavior is immediately visible.
No test coverage: Table.test.tsx has no assertions for isResponsive. A quick test verifying data-responsive-mode="list" vs "scroll" on the outer container would guard against regressions.
Prop semantics remain inverted (per previous reviews): isResponsive={true} enables the responsive/adaptive list view; isResponsive={false} opts into fixed scroll. Future readers will likely expect the inverse. This is now well-documented in the changeset, so it's workable — but worth a JSDoc comment on the prop for IDE discoverability.

No blockers remain. ✅

github-actions · 2026-03-06T18:05:12Z

Claude finished @punkbit's task in 1m 19s —— View job

Tasks

Gather context
Read changed files
Analyze conventions and guidelines
Provide review feedback

Great iteration! All blocking issues from the previous review rounds are now resolved. Here's the current state:

✅ Fixed

TableWrapper now has overflow-x: auto (line 504) — horizontal scroll works correctly
Duplicate changeset file removed — only fuzzy-laws-know.md remains
CSS attribute selector quotes are now consistent (all single-quoted [data-responsive-mode='list'])
Tests added: Table.test.tsx lines 128–138 cover both isResponsive={true} (default list mode) and isResponsive={false} (scroll mode)
Changeset accurately documents the behavior: default isResponsive={true} → list mode, isResponsive={false} → scroll mode

💡 Suggestions (non-blocking)

Prop semantics are counterintuitive: isResponsive={true} (default) enables the mobile list view — which is the adaptive/responsive behavior. isResponsive={false} opts into scroll-only mode. A future reader would likely expect isResponsive={true} to mean "use the responsive layout" and isResponsive={false} to mean "disable it." Consider mobileLayout?: 'list' | 'scroll' or a JSDoc comment to make the inversion explicit:
```
/** When false, disables mobile list view and uses horizontal scroll instead. Defaults to true (list view enabled). */
isResponsive?: boolean;
```
Storybook story default arg: The Responsive story defaults to isResponsive: true (list mode), which doesn't showcase the new feature out of the box. Setting isResponsive: false as the default arg would make the scroll behavior the first thing viewers see, better communicating what the prop adds.
Pre-existing: ActionsList has a hardcoded border-left: 1px solid (line 481) instead of a theme token — not introduced by this PR, worth a follow-up.

This is ready to merge in my view — the implementation is sound, tests are in place, docs are accurate. 🚀

github-actions · 2026-03-06T18:11:12Z

Claude finished @punkbit's task in 57s —— View job

Tasks

Gather context
Read changed files
Analyze conventions and guidelines
Provide review feedback

All the blocking issues from previous review rounds are resolved. This is in good shape to merge. 🙌

✅ Resolved

TableWrapper has overflow-x: auto — horizontal scroll works correctly
No duplicate changeset files (only fuzzy-laws-know.md)
CSS attribute selector quotes are consistent (all single-quoted [data-responsive-mode='list'])
Tests cover both modes (isResponsive={true} → "list", isResponsive={false} → "scroll")
Changeset accurately documents the API and includes a usage example
JSDoc on the prop explains the inversion clearly

💡 Suggestions (non-blocking)

Responsive story default is unintuitive (Table.stories.tsx:536): The story defaults to isResponsive: true (list mode — the old behavior), so a Storybook user opening it won't see the new feature. The story description explains isResponsive={false}, but the default arg shows the opposite. Defaulting to isResponsive: false with maxWidth: '400px' would make the scroll behavior immediately visible and better communicate what the prop adds.
Prop semantics remain inverted: isResponsive={true} (default) = mobile list view ON, isResponsive={false} = scroll only. The JSDoc comment does explain this, so it's workable. Worth a follow-up to consider mobileLayout?: 'list' | 'scroll' if/when the API is revisited.
Pre-existing: ActionsList has border-left: 1px solid (line 481) with a hardcoded 1px instead of ${theme.click.table.cell.stroke} — not introduced by this PR, worth a follow-up issue.

No blockers. ✅

github-actions · 2026-03-06T18:12:17Z

Claude finished @punkbit's task in 40s —— View job

Tasks

Gather context
Read changed files
Analyze conventions and guidelines
Provide review feedback

All previously flagged blockers are now resolved. The PR is in great shape. 🙌

✅ Resolved

TableWrapper now has overflow-x: auto (line 504) — horizontal scroll works correctly
Duplicate changeset removed — only fuzzy-laws-know.md remains
CSS attribute selector quotes are consistent (all single-quoted [data-responsive-mode='list'])
Tests added at Table.test.tsx:128–138 covering both isResponsive={true} (list mode) and isResponsive={false} (scroll mode)
Changeset accurately documents the API with correct default behavior
Story default is isResponsive: false with maxWidth: 400px, showing the scroll behavior out-of-the-box

💡 Suggestions (non-blocking)

Prop semantics (Table.tsx:583): isResponsive={true} (default) enables the mobile list view — which is the adaptive behavior. isResponsive={false} opts into fixed horizontal scroll. The JSDoc comment on the prop now documents this inversion, which is workable. A mobileLayout?: 'list' | 'scroll' union could make intent unambiguous for future consumers, but that's a nice-to-have.
Pre-existing: ActionsList has a hardcoded border-left: 1px solid (line 481) instead of ${theme.click.table.cell.stroke} — not introduced by this PR, worth a follow-up issue.

No blockers remain. ✅ Ready to merge.

maxknv · 2026-03-09T09:22:40Z

LGTM! Thank you

github-actions · 2026-03-09T10:06:36Z

Claude finished @punkbit's task in 1m 2s —— View job

Tasks

Gather context
Read changed files
Analyze conventions and guidelines
Provide review feedback

All previously flagged blockers from prior review rounds are resolved. The implementation is clean and well-considered. 🙌

✅ Confirmed Good

TableWrapper has overflow-x: auto — horizontal scroll works correctly without requiring a consumer wrapper
Single changeset file (fuzzy-laws-know.md) with accurate description and usage example
CSS attribute selectors are consistent throughout (all single-quoted [data-responsive-mode='list'])
Theme breakpoint token (theme.breakpoint.sizes.md) replaces all hardcoded 768px values
Tests cover both modes: default isResponsive={true} → "list", isResponsive={false} → "scroll"
JSDoc on the prop documents the semantics clearly
Responsive story defaults to isResponsive: false with maxWidth: 400px, showcasing the new behavior immediately

💡 Suggestions (non-blocking)

Committed TODO comment (Table.tsx:582): The inline // TODO: Should this be renamed... comment in the interface is a bit unusual for a published library — it exposes internal deliberation in the shipped source. If the naming question is unresolved, track it in a follow-up issue instead.
Pre-existing: ActionsList still has a hardcoded border-left: 1px solid (line 481) instead of ${theme.click.table.cell.stroke} — not introduced by this PR, worth a follow-up issue.

No blockers. ✅ Ready to merge.

workflow-authentication-public · 2026-03-09T10:08:13Z

📚 Storybook Preview Deployed

✅ Preview URL: https://click-efaw99x2v-clickhouse.vercel.app

Built from commit: bde8dabbbd143eb489f6b96a087fab2abd2b548f

elizabetdev

LGTM! Tested in Google Chrome.

elizabetdev · 2026-03-09T10:39:13Z

src/components/Table/Table.tsx

  showHeader?: boolean;
  rowHeight?: string;
  resizableColumns?: boolean;
+  // TODO: Should this be renamed to mobileLayout?: 'list' | 'scroll' instead for clarity?


I agree that the prop name mobileLayout would be more descriptive. But for now isResponsive works and keeps the API consistent with other components.

#881) * feat(Table): add disableMobileListView prop to prevent mobile list conversion Add new prop to Table component that disables the automatic conversion to mobile list view on narrow screens, keeping the table layout with horizontal scroll instead. Changes: - Add disableMobileListView?: boolean prop (default: false) - Set data-responsive-mode attribute on TableOuterContainer - Wrap all mobile @media queries with [data-responsive-mode="list"] & selector - When disableMobileListView=true, mobile styles don't apply Benefits: - Simple implementation (3 main changes) - No prop threading through nested components - Clean CSS selector approach - Backward compatible (default behavior unchanged) Use case: When you want tables to scroll horizontally on mobile instead of converting to a list view format. * Use theme breakpoint instead of hardcoded 768px * refactor: 💡 rename prop to isResponsive * chore: 🤖 format * chore: 🤖 update changeset with how to * fix: 🐛 isResponsive * chore: 🤖 add table responsive story * chore: 🤖 add table responsive story * refactor: 💡 changeset, default behaviour amends * chore: 🤖 default isresponsive true * style: 💄 Add overflow-x: auto to TableWrapper safe unconditionally since scroll only activates when content overflows * chore: 🤖 delete duplicate changeset * chore: 🤖 format * test: 💍 table isResponsive * chore: 🤖 add jsdoc and TODO * chore: 🤖 default to isresponsive false --------- Co-authored-by: Max Kainov <max.kainov@clickhouse.com>

maxknv and others added 5 commits March 5, 2026 22:25

Merge branch 'main' into feat/table-disable-mobile-list-view

d09f922

Merge branch 'main' into feat/table-disable-mobile-list-view

7088b26

Use theme breakpoint instead of hardcoded 768px

8a93b54

refactor: 💡 rename prop to isResponsive

40acf4b

chore: 🤖 format

34f55cc

punkbit changed the title ~~feat(table): 🎸 table disable mobile list view (reviewed)~~ feat(table): 🎸 add isResponsive prop to control mobile layout behavior Mar 6, 2026

chore: 🤖 update changeset with how to

a15ba63

Merge branch 'main' into feat/table-disable-mobile-list-view-reviewed

cfefce4

punkbit added 2 commits March 6, 2026 17:30

fix: 🐛 isResponsive

69d6b68

chore: 🤖 add table responsive story

2ddbefa

chore: 🤖 add table responsive story

f4e3939

refactor: 💡 changeset, default behaviour amends

0dcf8e2

chore: 🤖 default isresponsive true

b11ec9c

style: 💄 Add overflow-x: auto to TableWrapper safe unconditionally si…

b764371

…nce scroll only activates when content overflows

punkbit added 2 commits March 6, 2026 18:02

chore: 🤖 delete duplicate changeset

6d18681

chore: 🤖 format

320bdf0

test: 💍 table isResponsive

ee3ed1a

punkbit mentioned this pull request Mar 6, 2026

feat(Table): add disableMobileListView prop to prevent mobile list conversion #877

Closed

punkbit marked this pull request as ready for review March 6, 2026 18:08

chore: 🤖 add jsdoc and TODO

3a9cb5b

chore: 🤖 default to isresponsive false

2420da6

Merge branch 'main' into feat/table-disable-mobile-list-view-reviewed

ac188f5

elizabetdev approved these changes Mar 9, 2026

View reviewed changes

punkbit merged commit 0f32e3f into main Mar 9, 2026
9 checks passed

punkbit deleted the feat/table-disable-mobile-list-view-reviewed branch March 9, 2026 10:40

github-actions bot mentioned this pull request Mar 9, 2026

refactor: 💡 use mobile layout prop instead of is responsive for mobile view control #884

Merged

Conversation

punkbit commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why?

How?

Preview?

Uh oh!

changeset-bot bot commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

github-actions bot commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review

Tasks

Uh oh!

github-actions bot commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Tasks

Uh oh!

github-actions bot commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Tasks

Uh oh!

github-actions bot commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Tasks

Uh oh!

github-actions bot commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Tasks

Uh oh!

github-actions bot commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Tasks

Uh oh!

github-actions bot commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Tasks

Uh oh!

github-actions bot commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Tasks

Uh oh!

github-actions bot commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Tasks

Uh oh!

github-actions bot commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Tasks

Uh oh!

github-actions bot commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Tasks

Uh oh!

github-actions bot commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Tasks

Uh oh!

github-actions bot commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Tasks

Uh oh!

maxknv commented Mar 9, 2026

Uh oh!

github-actions bot commented Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Tasks

Uh oh!

workflow-authentication-public bot commented Mar 9, 2026

📚 Storybook Preview Deployed

Uh oh!

elizabetdev left a comment

Choose a reason for hiding this comment

Uh oh!

elizabetdev Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

punkbit commented Mar 6, 2026 •

edited

Loading

changeset-bot bot commented Mar 6, 2026 •

edited

Loading

github-actions bot commented Mar 6, 2026 •

edited

Loading

github-actions bot commented Mar 6, 2026 •

edited

Loading

github-actions bot commented Mar 6, 2026 •

edited

Loading

github-actions bot commented Mar 6, 2026 •

edited

Loading

github-actions bot commented Mar 6, 2026 •

edited

Loading

github-actions bot commented Mar 6, 2026 •

edited

Loading

github-actions bot commented Mar 6, 2026 •

edited

Loading

github-actions bot commented Mar 6, 2026 •

edited

Loading

github-actions bot commented Mar 6, 2026 •

edited

Loading

github-actions bot commented Mar 6, 2026 •

edited

Loading

github-actions bot commented Mar 6, 2026 •

edited

Loading

github-actions bot commented Mar 6, 2026 •

edited

Loading

github-actions bot commented Mar 6, 2026 •

edited

Loading

github-actions bot commented Mar 9, 2026 •

edited

Loading