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

[DataGrid] Add density change event #10080

Closed
2 tasks done
aress31 opened this issue Aug 18, 2023 · 4 comments
Closed
2 tasks done

[DataGrid] Add density change event #10080

aress31 opened this issue Aug 18, 2023 · 4 comments
Labels
component: data grid This is the name of the generic UI component, not the React module! duplicate This issue or pull request already exists new feature New feature or request

Comments

@aress31
Copy link

aress31 commented Aug 18, 2023

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Summary 💡

It seems like there is still no onDensityChange handler, listening for the entire state change is an overkill and will get triggered at every change and therefore to stick with the current logic of variable + handler I would love to see an onDensityChange.

Examples 🌈

No response

Motivation 🔦

Improving DX and providing more control over preference persistence.

Order ID 💳 (optional)

No response

@aress31 aress31 added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Aug 18, 2023
@romgrk romgrk added component: data grid This is the name of the generic UI component, not the React module! new feature New feature or request and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Aug 18, 2023
@romgrk
Copy link
Contributor

romgrk commented Aug 18, 2023

I would suggest that you keep listening to the state changes but only trigger your logic when specific parts of it change. This is flexible enough to cover all use cases, and is exactly what we do inside the grid to trigger targetted re-renders.

// Select the parts of the state you care about
// NOTE: make sure to use the public selectors from the docs, the state's inner structure isn't public
const selector = state => [state.a, state.b, state.c]

const cachedValues = useRef([])
useEffect(() => {
  return apiRef.current.subscribeEvent('stateChange', state => {
    const values = selector(state)
    if (!values.every((v, i) => v === cachedValues.current[i])) {
      // [insert save state code here]
      cachedValues.current = values
    } 
  })
}, [])

I don't like the idea of adding a prop for this specifically, it's too specific and doesn't have a significant advantage over using the implementation above. Does that solve your issue?

@romgrk romgrk added the status: waiting for author Issue with insufficient information label Aug 18, 2023
@aress31
Copy link
Author

aress31 commented Aug 18, 2023

From an ease of use and DX perspective I would still prefer the onDensityChange handler to remain consistent with all the other handlers.

@github-actions github-actions bot removed the status: waiting for author Issue with insufficient information label Aug 18, 2023
@romgrk romgrk changed the title [MUI Grid] onDensityChange [DataGrid] Add density change event Aug 18, 2023
@romgrk
Copy link
Contributor

romgrk commented Aug 18, 2023

@mui/xgrid Any other opinions on adding a prop here?

@romgrk romgrk added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Aug 18, 2023
@MBilalShafi
Copy link
Member

@aress31 Thank you for using MUI X Data Grid and providing useful feedback. 😊
The request you made makes sense and is already made previously in #5647
Fortunately it's already in our backlog and should be prioritized as some bandwidth is available.
Please feel free to checkout the discussion or hop in with your feedback on the mentioned ticket.

I'll close this one just to keep all the conversation on the same place.
Thank you.

@MBilalShafi MBilalShafi added duplicate This issue or pull request already exists and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Aug 29, 2023
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! duplicate This issue or pull request already exists new feature New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants