Skip to content

DataTableForObservableQuery's internal table-container never actually becomes scrollable (max-height:100% resolves to none) #196

Description

@einari

What was expected, and what happened instead

DataTableForObservableQuery is meant to give a page-filling, internally-scrolling table with a frozen header - it hardcodes scrollable={true} and scrollHeight="100%" on the underlying DataTableCore, and sizes its wrapper with a ResizeObserver (see Source/DataTables/DataTableForObservableQuery.tsx). In practice the internal scroll region never actually becomes scrollable: it silently grows to fit every row instead of being clipped to the available height, so content beyond the visible area is invisibly cut off by the outer wrapper's overflow: hidden with no scrollbar ever appearing anywhere - the table looks page-filling but cannot be scrolled.

Root cause, traced through the compiled output of @cratis/components@3.6.0 and @primereact/headless@11:

  • DataTableForObservableQuery passes style={{ minWidth: '100%' }} (no height) and scrollHeight="100%" to DataTableCore, which forwards both straight to DataTable.Root.
  • @primereact/headless's useDataTable applies scrollHeight to the table-container part as style.maxHeight = scrollHeight (i.e. max-height: 100%) whenever scrollHeight !== "flex" (see @primereact/headless/datatable's useMemo building the table-container part props).
  • Per the CSS spec, a percentage max-height against an ancestor whose own height is not explicitly specified (i.e. auto) resolves to none - no constraint at all. DataTable.Root's own rendered <div data-part="root"> (the immediate parent of table-container) never receives an explicit height from DataTableCore/DataTableForObservableQuery - only className/style merge with no height. So the percentage chain breaks exactly one DOM level above where it is applied, and max-height: 100% is silently ignored.
  • The ResizeObserver-computed pixel height that DataTableForObservableQuery applies (or its hardcoded 600 fallback before the first measurement) lands on its own outer wrapper div, which is a grandparent of table-container, with DataTable.Root's unconstrained div sandwiched in between - so even a correctly-measured JS height never reaches the CSS property that actually needs it.

Net effect: table-container's scrollHeight always equals its clientHeight (verified directly - see repro below), so overflow: auto never has anything to scroll, and the only thing that visually caps the table is the grandparent wrapper's overflow: hidden, which clips without offering any way to scroll to the clipped content.

This is a different, more specific defect than #195: #195 is about DataTableForQuery (the snapshot variant) lacking the scrollable/frozen-header mechanism entirely. This issue is about DataTableForObservableQuery - the variant #195 describes as already working correctly - having that mechanism in place but non-functional once real data overflows the container, because of the percentage-height chain break described above. #195's own repro never exercised actual scroll interaction on the observable variant with enough rows to overflow, which is why the two didn't surface together.

Smallest reproduction

Render DataTableCore (or DataTableForObservableQuery) inside a height-bounded ancestor with enough rows to overflow it, exactly as it renders internally:

import { DataTableCore, Column } from '@cratis/components/DataTables';

// Any ancestor chain that bounds this div's height, e.g. a flex column with min-h-0/flex-1
<div style={{ height: '564px', overflow: 'hidden' }}>
    <DataTableCore
        data={thirtyOrMoreRows}
        dataKey="id"
        emptyMessage="No rows"
        scrollable
        scrollHeight="100%"
        style={{ minWidth: '100%' }}>
        <Column field="name" header="Name" />
    </DataTableCore>
</div>

Inspect the rendered [data-part="table-container"] element: element.scrollHeight === element.clientHeight always holds, regardless of row count, and element.scrollTop = 100000 never moves past 0 - there is nothing to scroll. The rows beyond the visible area are simply invisible, clipped by the outer wrapper.

Confirmed workaround (does not require touching @cratis/components internals): passing a pt override that gives DataTable.Root's own div an explicit height restores the percentage chain and makes the table genuinely scrollable:

<DataTableCore
    /* ...same props as above... */
    pt={{ root: { style: { height: '100%' } } }}>

With this, table-container's scrollHeight correctly exceeds its clientHeight for overflowing content, and scrollTop moves as expected. This confirms the diagnosis: the only thing missing is an explicit height on DataTable.Root's own rendered element.

Why it matters beyond the immediate annoyance

DataTableForObservableQuery is the documented, recommended way to get a page-filling, internally-scrolling, frozen-header table for a live/observable list. Every consumer that follows that guidance and ends up with more rows than fit in the container gets a table that looks right (frozen header, correct fit) right up until someone tries to scroll past the visible rows - which will not happen in casual testing or an empty/near-empty dev dataset, only once real data volume arrives. It presents as "scrolling doesn't work" with no console error, no layout warning, and a component tree that otherwise looks entirely correctly wired - very easy to ship and very expensive to diagnose downstream, since every visible signal (frozen header, correct outer sizing, no overflow spilling onto the page) suggests the height chain is fine.

Suggested direction

Not prescribing the implementation, but any of the following would close the gap:

  1. Have DataTableForObservableQuery (and DataTableCore generally, when scrollable/scrollHeight are set) pass an explicit height: '100%' (or equivalent) through to DataTable.Root's own rendered element by default, so scrollHeight="100%"'s percentage resolves against a real height instead of relying on the consumer to know about the pt.root.style.height workaround.
  2. Default to scrollHeight="flex" internally instead of "100%" - @primereact/headless already special-cases "flex" to apply flex: '1 1 auto' to table-container instead of a percentage max-height, which only needs the immediate parent to be a flex container rather than needing an unbroken chain of explicit heights - a more robust default for exactly this "fill a flex-managed height chain" use case DataTableForObservableQuery is designed for.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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