Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[data grid] Support the custom header filter height that is independent from the column header height #12643

Closed
layerok opened this issue Apr 3, 2024 · 9 comments · Fixed by #12666
Labels
component: data grid This is the name of the generic UI component, not the React module! customization: css Design CSS customizability enhancement This is not a bug, nor a new feature feature: Filtering on header Related to the header filtering (Pro) feature support: premium standard Support request from a Premium standard plan user. https://mui.com/legal/technical-support-sla/

Comments

@layerok
Copy link
Contributor

layerok commented Apr 3, 2024

Summary

Modify the behaviour of the columnHeaderHeight prop so that it no longer controls the height of the header filter row and only controls the height of the column header row.
Add a new columnHeaderFilterHeight prop which controls the height of the header filter row.

<DataGrid columnHeaderFilterHeight={52} columnHeaderHeight={40}/>

Examples

I have the following datagrid design in which the header filter height and the column header height are different. The header filter height is 52px and the header column height is 40px

image

Currently there is only one prop to control header row height. It is columnHeaderHeight. But this prop doesn't suit my needs, because it makes both rows the same height.

My current workaround.

const HEADER_ROW_HEIGHT = 40;
const HEADER_FILTER_ROW_HEIGHT = 52;
const headerFilterHeightCssVar = '--DataGrid-headerFilterHeight';

const StyledDataGrid = styled(DataGridPremium)(() => ({
  [headerFilterHeightCssVar]: `${HEADER_FILTER_ROW_HEIGHT}px`,
  [`& .${gridClasses.columnHeaders}`]: {
    maxHeight: `calc(${HEADER_ROW_HEIGHT}px + var(${headerFilterHeightCssVar}))!important`,
    minHeight: `calc(${HEADER_ROW_HEIGHT}px + var(${headerFilterHeightCssVar}))!important`,
  },
  [`& .${gridClasses.headerFilterRow}`]: {
    height: `${HEADER_FILTER_ROW_HEIGHT}px`,
    [`& .${gridClasses.columnHeader}`]: {
      height: `${HEADER_FILTER_ROW_HEIGHT}px!important`,
    },
  },
}));

Motivation

There is no convenient way to change the height of the header filter independently from the height of the column header.

Search keywords: column header filter, custom height
Order ID: 74777

@layerok layerok added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Apr 3, 2024
@layerok layerok changed the title [data grid] Support the custom header filter height that is independent from the header filter height [data grid] Support the custom header filter height that is independent from the column header height Apr 3, 2024
@michelengelen
Copy link
Member

Changing the height on the header filter row can be done like this:

<DataGridPremium
  columns={columns}
  rows={rows}
  headerFilters
  sx={{
    [`& .${gridClasses.headerFilterRow}`]: {
      height: 200,
    },
  }}
/>

This will result in this:

Screenshot 2024-04-03 at 15 48 20

I do agree although that the columnHeaderHeight prop will restrict the column header height when not overwritten with sx as well. If your implementation allows for that you can also do this via sx:

sx={{
  [`& .${gridClasses.columnHeader}`]: {
    height: 100,
  },
  [`& .${gridClasses.headerFilterRow}`]: {
    height: 200,
  },
}}

Does this help your use case?

@michelengelen michelengelen added component: data grid This is the name of the generic UI component, not the React module! customization: css Design CSS customizability feature: Filtering on header Related to the header filtering (Pro) feature support: premium standard Support request from a Premium standard plan user. https://mui.com/legal/technical-support-sla/ status: waiting for author Issue with insufficient information and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Apr 3, 2024
@michelengelen
Copy link
Member

@cherniavskii should we add this to the board to explore 2 separate props columnHeaderHeight and columnHeaderFilterHeight?

@layerok
Copy link
Contributor Author

layerok commented Apr 3, 2024

@michelengelen Thanks for the answer. It kind of works. But the header filter cell does not stretch to fit the height of the parent element.

image

To fix this I had to add one more css rule

sx={{
  [`& .${gridClasses.columnHeader}`]: {
    height: 40,
  },
  [`& .${gridClasses.headerFilterRow}`]: {
    height: 52,
    [`& .${gridClasses.columnHeader}`]: {
      height: "100%!important",
    },
  },
}}

@github-actions github-actions bot added status: waiting for maintainer These issues haven't been looked at yet by a maintainer and removed status: waiting for author Issue with insufficient information labels Apr 3, 2024
@layerok
Copy link
Contributor Author

layerok commented Apr 3, 2024

But there is one problem with the current solution.

Now it is possible to scroll the table content even there is enough space to fit all rows. You can see it in the attached video.

I have autoPageSize option turned on.

mui-scrolling-issue.mov

@layerok
Copy link
Contributor Author

layerok commented Apr 3, 2024

I am guessing this is because we overwrote dimensions only in the CSS, but the dimensions in the internal state were not adjusted accordingly, so features that depend on internal dimensions such as scrolling don't work as expected.

@MBilalShafi
Copy link
Member

MBilalShafi commented Apr 3, 2024

Now it is possible to scroll the table content even there is enough space to fit all rows. You can see it in the attached video.

That's because we use fake scrollbars internally and they use a function called getTotalHeaderHeight to compute when to start a scroll. Even if you increase the height using the sx prop, this method will use the standard height resulting in incorrect behavior related to the scrollbar.

export function getTotalHeaderHeight(
apiRef: React.MutableRefObject<GridApiCommunity>,
headerHeight: number,
) {
const densityFactor = gridDensityFactorSelector(apiRef);
const maxDepth = gridColumnGroupsHeaderMaxDepthSelector(apiRef);
const isHeaderFilteringEnabled = gridHeaderFilteringEnabledSelector(apiRef);
const multiplicationFactor = isHeaderFilteringEnabled ? 2 : 1;
return Math.floor(headerHeight * densityFactor) * ((maxDepth ?? 0) + multiplicationFactor);
}

IMO introducing a separate optional prop like columnHeaderFilterRowHeight could be a possible solution.
It could default to columnHeaderHeight if not provided.

Since it will be kind of an override, I'd prefer not to take into account the densityFactor once it's provided.

Wdyt @mui/xgrid?

@MBilalShafi MBilalShafi added enhancement This is not a bug, nor a new feature and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Apr 3, 2024
@romgrk
Copy link
Contributor

romgrk commented Apr 3, 2024

Yes, columnHeaderFilterHeight sounds good to me. Not sure about density, feels incoherent to have one prop be affected by it and the other not.

@MBilalShafi
Copy link
Member

feels incoherent to have one prop be affected by it and the other not.

Sounds good to me on a rethought. I was relating it more to a fixed height value (just like we set using CSS), but for an internal prop, I agree that honoring the density does make sense.

@romgrk romgrk removed their assignment Apr 4, 2024
Copy link

⚠️ This issue has been closed. If you have a similar problem but not exactly the same, please open a new issue.
Now, if you have additional information related to this issue or things that could help future readers, feel free to leave a comment.

@layerok: How did we do? Your experience with our support team matters to us. If you have a moment, please share your thoughts in this short Support Satisfaction survey.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: data grid This is the name of the generic UI component, not the React module! customization: css Design CSS customizability enhancement This is not a bug, nor a new feature feature: Filtering on header Related to the header filtering (Pro) feature support: premium standard Support request from a Premium standard plan user. https://mui.com/legal/technical-support-sla/
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants