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

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

Closed
CarlosAmaral opened this issue Jun 7, 2024 · 7 comments · Fixed by #13487
Closed

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

CarlosAmaral opened this issue Jun 7, 2024 · 7 comments · Fixed by #13487
Labels
bug 🐛 Something doesn't work component: tree view TreeView, TreeItem. This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process.

Comments

@CarlosAmaral
Copy link

CarlosAmaral commented Jun 7, 2024

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

@CarlosAmaral CarlosAmaral added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jun 7, 2024
@michelengelen
Copy link
Member

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?

@michelengelen michelengelen changed the title Failed to execute 'querySelectorAll' on 'Element': '*[id=":r1:-C:\"] [tree view] Failed to execute 'querySelectorAll' on 'Element': '*[id=":r1:-C:\"] Jun 7, 2024
@michelengelen michelengelen added bug 🐛 Something doesn't work status: waiting for author Issue with insufficient information component: tree view TreeView, TreeItem. This is the name of the generic UI component, not the React module! and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Jun 7, 2024
@CarlosAmaral
Copy link
Author

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

@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 Jun 7, 2024
@romgrk
Copy link
Contributor

romgrk commented Jun 7, 2024

We have an escape function for this in the grid, using CSS.escape will run into this issue: #13069 (comment)

@michelengelen
Copy link
Member

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
Copy link
Member

If it fixes the issue, I think it's a great solution 👍

@michelengelen
Copy link
Member

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

@michelengelen michelengelen added good first issue Great for first contributions. Enable to learn the contribution process. and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Jun 7, 2024
@flaviendelangle flaviendelangle changed the title [tree view] Failed to execute 'querySelectorAll' on 'Element': '*[id=":r1:-C:\"] [tree view] Can't use itemId with escaping characters in SimpleTreeView Jun 10, 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.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: tree view TreeView, TreeItem. This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants