Skip to content

DataTableForQuery doesn't freeze its header / fill its container the way DataTableForObservableQuery does #195

Description

@einari

What was expected, and what happened instead

DataTableForObservableQuery and DataTableForQuery are documented as twin components - one for IObservableQueryFor<T>, one for IQueryFor<T> - with the same props surface and the same intended behavior. In practice their internal rendering diverges:

  • DataTableForObservableQuery (Source/DataTables/DataTableForObservableQuery.tsx) unconditionally passes scrollable={true} and scrollHeight="100%" to the underlying DataTableCore, and sizes its table region with a ResizeObserver so the header freezes and only the body scrolls, filling whatever height its container gives it.
  • DataTableForQuery (Source/DataTables/DataTableForQuery.tsx) does not set scrollable/scrollHeight on DataTableCore at all. It wraps the table in a div with overflow: 'auto', so a snapshot-query table taller than its container scrolls the header away with the body instead of freezing it.

Neither component exposes scrollable/scrollHeight as a consumer-facing prop, either - so a caller genuinely cannot opt a snapshot-query table into the same frozen-header, fill-to-container behavior the observable variant gets for free, even by hand.

Smallest reproduction

import { DataTableForQuery, Column } from '@cratis/components/DataTables';
import { AllAuthors } from './AllAuthors'; // IQueryFor<Author[]>, i.e. a snapshot query

<div style={{ height: '400px' }}>
    <DataTableForQuery query={AllAuthors} emptyMessage="No authors">
        <Column field="name" header="Name" />
    </DataTableForQuery>
</div>

With more rows than fit in 400px, the whole table (header included) scrolls inside the wrapping overflow: auto div - the header is not frozen. Swap AllAuthors for an equivalent IObservableQueryFor<Author[]> query and render the same columns through DataTableForObservableQuery instead, and the header freezes correctly (confirmed by reading DataTableForObservableQuery.js's compiled output in @cratis/components@3.6.0: it hardcodes scrollable: true, scrollHeight: '100%' on DataTableCore, plus a ResizeObserver to compute the table region's pixel height).

Why it matters beyond the immediate annoyance

Any list page backed by a snapshot query (IQueryFor<T>, not observable) that can grow taller than its container has no way to get a frozen header - not through DataTableForQuery, not through DataPage (which auto-selects DataTableForQuery for snapshot queries via context.query.prototype instanceof QueryFor), and not through a documented pass-through prop. The only workaround is dropping to the low-level DataTableCore directly and rebuilding paging/selection by hand, which defeats the purpose of the higher-level component. This is not obviously broken in casual testing - it only shows up once a snapshot-query table has enough rows to overflow its container, so it is easy to ship and only notice once real data volume arrives.

Suggested direction

Not prescribing the implementation, but two independent fixes would both close the gap:

  1. Bring DataTableForQuery's internal rendering in line with DataTableForObservableQuery (scrollable/scrollHeight + the same fill-to-container sizing), so the two "twin" components actually behave the same way for the same shape of usage.
  2. Expose scrollable/scrollHeight as pass-through props on both DataTableForQuery and DataTableForObservableQuery (and DataPage), so a consumer can opt in/out or override the value explicitly rather than relying on whichever behavior happens to be hardcoded.

Either would unblock a page-filling, internally-scrolling table with a frozen header for a query-backed list page taller than the viewport - the shape this was found from: a settings-style page with three tabs, each holding a query-backed table meant to fill the remaining viewport height.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions