Skip to content

[docs] Fix size column filtering in files tree demo #17952

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[docs] Fix size column filtering in files tree demo #17952

wants to merge 1 commit into from
+12 −30

Conversation

cherniavskii
Copy link
Member

@cherniavskii cherniavskii commented May 21, 2025 •
edited
Loading

I added valueGetter and kept only one unit for the size column to make filtering and sorting work reliably.

Before: https://stackblitz.com/edit/bbwp47fm?file=src%2FDemo.tsx
After: https://deploy-preview-17952--material-ui-x.netlify.app/x/react-data-grid/aggregation/#usage-with-tree-data

@cherniavskii cherniavskii added component: data grid This is the name of the generic UI component, not the React module! plan: Pro Impact at least one Pro user feature: Filtering Related to the data grid Filtering feature feature: Tree data Related to the data grid Tree data feature feature: Aggregation Related to the data grid Aggregation feature labels May 21, 2025
@github-actions GitHub Actions
Copy link

Please add one type label to categorize the purpose of this PR appropriately:

bug, regression, enhancement, new feature, release or dependencies

2 similar comments
@github-actions GitHub Actions
Copy link

Please add one type label to categorize the purpose of this PR appropriately:

bug, regression, enhancement, new feature, release or dependencies

@github-actions GitHub Actions
Copy link

Please add one type label to categorize the purpose of this PR appropriately:

bug, regression, enhancement, new feature, release or dependencies

@cherniavskii cherniavskii added the docs Improvements or additions to the documentation label May 21, 2025
@mui-bot
Copy link

mui-bot commented May 21, 2025

Deploy preview: https://deploy-preview-17952--material-ui-x.netlify.app/

Generated by 🚫 dangerJS against 1bf8a06

@cherniavskii cherniavskii mentioned this pull request May 21, 2025
Open
@cherniavskii cherniavskii requested a review from a team May 22, 2025 14:45
@cherniavskii cherniavskii marked this pull request as ready for review May 22, 2025 14:45
michelengelen
michelengelen approved these changes May 22, 2025
View reviewed changes
Copy link
Member

@michelengelen michelengelen left a comment

Choose a reason for hiding this comment

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

LGTM :shipit:

@cherniavskii
Copy link
Member Author

@michelengelen BTW, shouldn't we also add docs label to this list?
I don't think any of the listed labels make sense for PRs like this one.

@michelengelen
Copy link
Member

@michelengelen BTW, shouldn't we also add docs label to this list?

I don't think any of the listed labels make sense for PRs like this one.

It's a fix, so the correct label would be a "bug" label. For others this would be the upcoming "improvement" or "internal" label.

This is still up for discussion on notion though, so feel free to reference this there.

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

@michelengelen michelengelen michelengelen approved these changes

Assignees
No one assigned
Labels
component: data grid This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation feature: Aggregation Related to the data grid Aggregation feature feature: Filtering Related to the data grid Filtering feature feature: Tree data Related to the data grid Tree data feature plan: Pro Impact at least one Pro user
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@cherniavskii @mui-bot @michelengelen