Skip to content

MinWidth config#238

Merged
philcunliffe merged 5 commits intomasterfrom
minheight-config
Aug 6, 2025
Merged

MinWidth config#238
philcunliffe merged 5 commits intomasterfrom
minheight-config

Conversation

@philcunliffe
Copy link
Copy Markdown
Contributor

Adds an optional minWidth to the column configuration

(cherry picked from commit 259d874be2bcdc2efb097da2397f800d0ff36adb)
@philcunliffe philcunliffe requested review from platypii and severo August 5, 2025 18:15
Copy link
Copy Markdown
Contributor

@severo severo left a comment

Choose a reason for hiding this comment

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

Overall, I wonder: maybe we could create a context for the columns configuration, and its provider would be the outermost:

function HighTableData(props: PropsData) {
  const { data, key, version } = useData()
  const { numRows } = data
  // TODO(SL): onError could be in a context, as we might want to use it everywhere
  const { cacheKey, orderBy, onOrderByChange, selection, onSelectionChange, onError } = props

  return (
    <ColumnConfigurationProvider value={props.columnConfiguration}>
      /* Create a new set of widths if the data has changed, but keep it if only the number of rows changed */
      <ColumnStatesProvider key={cacheKey ?? key} localStorageKey={cacheKey ? `${cacheKey}${columnStatesSuffix}` : undefined} numColumns={data.header.length} minWidth={minWidth}>
        {/* Create a new context if the dataframe changes, to flush the cache (ranks and indexes) */}
        <OrderByProvider key={key} orderBy={orderBy} onOrderByChange={onOrderByChange} disabled={!data.sortable}>
          {/* Create a new selection context if the dataframe has changed */}
          <SelectionProvider key={key} selection={selection} onSelectionChange={onSelectionChange} data={data} onError={onError}>
            {/* Create a new navigation context if the dataframe has changed, because the focused cell might not exist anymore */}
            <CellsNavigationProvider key={key} colCount={data.header.length + 1} rowCount={numRows + 1} rowPadding={props.padding ?? defaultPadding}>
              <PortalContainerProvider>
                <HighTableInner version={version} {...props} />
              </PortalContainerProvider>
            </CellsNavigationProvider>
          </SelectionProvider>
        </OrderByProvider>
      </ColumnStatesProvider>
    </ColumnConfigurationProvider>
  )
}

and use that configuration inside the ColumnStates context, so that the rest of the code would not have to touch the minWidth parameter.

Alternatively, consume columnConfiguration as a prop of ColumnStatesProvider directly, which expands the scope of the context, but would be simpler, I guess


Otherwise, only some comments.

Comment thread src/hooks/useColumnStates.tsx
Comment thread src/components/ColumnHeader/ColumnHeader.test.tsx
Comment thread src/components/ColumnHeader/ColumnHeader.tsx Outdated
@philcunliffe philcunliffe requested a review from severo August 5, 2025 19:44
@philcunliffe philcunliffe merged commit a0ef888 into master Aug 6, 2025
10 checks passed
@philcunliffe philcunliffe deleted the minheight-config branch August 6, 2025 18:52
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