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] gridFilteredDescendantCountLookupSelector is not sufficient to complete user story #14491

Open
ashburnham opened this issue Sep 5, 2024 · 4 comments
Labels
feature: Row grouping Related to the data grid Row grouping feature feature: Tree data Related to the data grid Tree data feature new feature New feature or request support: pro standard Support request from a Pro standard plan user. https://mui.com/legal/technical-support-sla/

Comments

@ashburnham
Copy link

ashburnham commented Sep 5, 2024

Summary

Current behavior
if you have a parent group which has two rows inside that are in the same group, then it will count this as 3 descendants (it counts the 2 rows and then adds another for the group).

This is confusing for the user who only sees 2 descendant rows which just happen to be grouped.

Expected behavior
if you have a parent group which has two rows inside that are in the same group, then I should be able to show the user the number of rows rather than the number of nodes. i.e. it should only show 2 descendants because there are only 2 descendant rows.

image

Examples

this is an example of trying to use gridFilteredDescendantCountLookupSelector and only being able to deliver something nonsensical to the user

image

There should be a new feature like gridFilteredDescendantCountLookupSelector but different that allows me to show there are 5 items. Maybe gridFilteredUltimateDescendantCountLookupSelector

Motivation

just want the numbers to add up so it makes sense to the user

Search keywords: gridFilteredDescendantCountLookupSelector, tree data
Order ID: 92825

@ashburnham ashburnham added new feature New feature or request status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Sep 5, 2024
@github-actions github-actions bot added the support: pro standard Support request from a Pro standard plan user. https://mui.com/legal/technical-support-sla/ label Sep 5, 2024
@michelengelen michelengelen changed the title gridFilteredDescendantCountLookupSelector is not sufficient to complete user story [data grid] gridFilteredDescendantCountLookupSelector is not sufficient to complete user story Sep 5, 2024
@michelengelen
Copy link
Member

Hey @ashburnham and thanks for opening this.
The most logical step here would be to include a parameter to the selector to exclude grouping rows. Not sure if the state structure allows for something like that tough. 🤔

@github-project-automation github-project-automation bot moved this to 🆕 Needs refinement in MUI X Data Grid Sep 5, 2024
@michelengelen michelengelen added feature: Tree data Related to the data grid Tree data feature feature: Row grouping Related to the data grid Row grouping feature and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Sep 5, 2024
@michelengelen
Copy link
Member

I just checked myself and can achieve this with a simple added param to the getTreeNodeDescendants util:

diff --git a/packages/x-data-grid/src/hooks/features/rows/gridRowsUtils.ts b/packages/x-data-grid/src/hooks/features/rows/gridRowsUtils.ts
index b088a3fb6..8dc4f943a 100644
--- a/packages/x-data-grid/src/hooks/features/rows/gridRowsUtils.ts
+++ b/packages/x-data-grid/src/hooks/features/rows/gridRowsUtils.ts
@@ -210,6 +210,7 @@ export const getTreeNodeDescendants = (
   tree: GridRowTreeConfig,
   parentId: GridRowId,
   skipAutoGeneratedRows: boolean,
+  skipParentRows = false,
 ) => {
   const node = tree[parentId];
   if (node.type !== 'group') {
@@ -219,7 +220,11 @@ export const getTreeNodeDescendants = (
   const validDescendants: GridRowId[] = [];
   for (let i = 0; i < node.children.length; i += 1) {
     const child = node.children[i];
-    if (!skipAutoGeneratedRows || !isAutogeneratedRowNode(tree[child])) {
+    const childNode = tree[child];
+    if (
+      !(skipParentRows && childNode.type === 'group') &&
+      !(skipAutoGeneratedRows && isAutogeneratedRowNode(tree[child]))
+    ) {
       validDescendants.push(child);
     }
     const childDescendants = getTreeNodeDescendants(tree, child, skipAutoGeneratedRows);

Usage:

const descendantsWithoutParentRows = getTreeNodeDescendants(gridRowTreeSelector(apiRef), id, true, true);

@michelengelen
Copy link
Member

michelengelen commented Sep 5, 2024

We might want to consider putting the last parameters into a new options object:

type GetTreeNodeDescendantOptions = {
  skipAutoGeneratedRows?: boolean;
  skipParentRows?: boolean;
};

export const getTreeNodeDescendants = (
  tree: GridRowTreeConfig,
  parentId: GridRowId,
  options: GetTreeNodeDescendantOptions = {
    skipAutoGeneratedRows: false,
    skipParentRows: false,
  },
) => {
...

Thoughts @mui/xgrid ?

@ashburnham
Copy link
Author

This would be really nice and seems an easy enough improvement @mui/xgrid

Can it be implemented?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature: Row grouping Related to the data grid Row grouping feature feature: Tree data Related to the data grid Tree data feature new feature New feature or request support: pro standard Support request from a Pro standard plan user. https://mui.com/legal/technical-support-sla/
Projects
Status: 🆕 Needs refinement
Development

No branches or pull requests

2 participants