mui / mui-x

MUI X: Build complex and data-rich applications using a growing list of advanced React components, like the Data Grid, Date and Time Pickers, Charts, and more!
https://mui.com/x/
4.52k stars 1.31k forks source link

[tree view] Can't use `itemId` with escaping characters in `SimpleTreeView` #13414

Closed CarlosAmaral closed 4 months ago

CarlosAmaral commented 5 months ago

Steps to reproduce

Link to live example: (required)

Current behavior

The itemId provided is not resolved thus throwing an error.

Expected behavior

Expected to resolved the tree item id as it did in v6.

Context

No response

Your environment

npx @mui/envinfo ``` Don't forget to mention which browser you used. Output from `npx @mui/envinfo` goes here. ```

Search keywords: SimpleTreeView, TreeItem

michelengelen commented 5 months ago

Hey @CarlosAmaral ... that's a tricky one. '\' is an escaping character, so I assume you want to have 'C:\' as a value for the treeitem, correct?

CarlosAmaral commented 5 months ago

@michelengelen Yes, I do. Escaping the \ should work, this use case was included in the v6 of tree view.

romgrk commented 5 months ago

We have an escape function for this in the grid, using CSS.escape will run into this issue: https://github.com/mui/mui-x/pull/13069#discussion_r1614687024

michelengelen commented 5 months ago

Thanks @romgrk

applying this diff would resolve the issue:

diff --git a/packages/x-tree-view/src/internals/TreeViewProvider/TreeViewChildrenItemProvider.tsx b/packages/x-tree-view/src/internals/TreeViewProvider/TreeViewChildrenItemProvider.tsx
index 48afd8f36..212741143 100644
--- a/packages/x-tree-view/src/internals/TreeViewProvider/TreeViewChildrenItemProvider.tsx
+++ b/packages/x-tree-view/src/internals/TreeViewProvider/TreeViewChildrenItemProvider.tsx
@@ -1,5 +1,6 @@
 import * as React from 'react';
 import PropTypes from 'prop-types';
+import { escapeOperandAttributeSelector } from '@mui/x-tree-view/internals/utils/utils';
 import { useTreeViewContext } from './useTreeViewContext';
 import type { UseTreeViewJSXItemsSignature } from '../plugins/useTreeViewJSXItems';
 import type { UseTreeViewItemsSignature } from '../plugins/useTreeViewItems';
@@ -46,9 +47,11 @@ export function TreeViewChildrenItemProvider(props: TreeViewChildrenItemProvider
       return;
     }

+    const escapedIdAttr = escapeOperandAttributeSelector(idAttr);
+
     const previousChildrenIds = instance.getItemOrderedChildrenIds(itemId ?? null) ?? [];
     const childrenElements = rootRef.current.querySelectorAll(
-      `${itemId == null ? '' : `*[id="${idAttr}"] `}[role="treeitem"]:not(*[id="${idAttr}"] [role="treeitem"] [role="treeitem"])`,
+      `${itemId == null ? '' : `*[id="${escapedIdAttr}"] `}[role="treeitem"]:not(*[id="${escapedIdAttr}"] [role="treeitem"] [role="treeitem"])`,
     );
     const childrenIds = Array.from(childrenElements).map(
       (child) => childrenIdAttrToIdRef.current.get(child.id)!,
diff --git a/packages/x-tree-view/src/internals/utils/utils.ts b/packages/x-tree-view/src/internals/utils/utils.ts
index 5401ae664..661b34206 100644
--- a/packages/x-tree-view/src/internals/utils/utils.ts
+++ b/packages/x-tree-view/src/internals/utils/utils.ts
@@ -12,3 +12,9 @@ export const getActiveElement = (root: Document | ShadowRoot = document): Elemen

   return activeEl;
 };
+
+// TODO, eventually replaces this function with CSS.escape, once available in jsdom, either added manually or built in
+// https://github.com/jsdom/jsdom/issues/1550#issuecomment-236734471
+export function escapeOperandAttributeSelector(operand: string): string {
+  return operand.replace(/["\\]/g, '\\$&');
+}

WDYT @flaviendelangle ?

flaviendelangle commented 5 months ago

If it fixes the issue, I think it's a great solution :+1:

michelengelen commented 5 months ago

I'll open this as good first issue. Would be great to have some tests accompanying this change. 👍🏼

github-actions[bot] commented 4 months ago

:warning: 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.

@CarlosAmaral: 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.