[tree view] Fix collapsed children not selected#22711
Conversation
Deploy previewhttps://deploy-preview-22711--material-ui-x.netlify.app/ Bundle size
Check out the code infra dashboard for more information about this PR. |
…terial to 9.1.0 Bump @mui/material family (material, system, utils, icons-material) to 9.1.0. 9.1.0's Collapse mounts children in a deferred render, so SimpleTreeView's child-discovery effect never re-ran; force a re-render on child register/unregister to fix collapsed-parent descendant selection.
5e9a3dd to
856d7f7
Compare
LukasTy
left a comment
There was a problem hiding this comment.
Review: PR 22711 -- [tree view] Fix collapsed children not selected
Verdict: Approve. Correct, minimal fix for a real 9.1.0 regression: Collapse now mounts unmountOnExit children a commit later, so SimpleTreeView's DOM child-discovery never re-ran; forcing a re-render on register/unregister re-arms the existing selection-recovery path. CI green.
Findings:
- [Medium] Confirm with the material team whether 9.1.0
Collapsemounting children a commit later was intentional. If not, report upstream -- but keep this fix regardless, since it also hardens the tree against any deferred-mount transition. - [Low] Optional: guard
rerender()on an actual map change -- skip it whenregisterChildgets an identical(idAttr, itemId), and useif (map.delete(idAttr)) rerender()inunregisterChild. Trims redundant renders and neutralizes the pre-existing duplicateunregisterChildcall (itemPlugin.tsx:44-45).--- a/packages/x-tree-view/src/internals/TreeViewProvider/TreeViewChildrenItemProvider.tsx +++ b/packages/x-tree-view/src/internals/TreeViewProvider/TreeViewChildrenItemProvider.tsx registerChild: (childIdAttribute, childItemId) => { + // Only re-render when the child set actually changes. + if (childrenIdAttrToIdRef.current.get(childIdAttribute) === childItemId) { + return; + } childrenIdAttrToIdRef.current.set(childIdAttribute, childItemId); rerender(); }, unregisterChild: (childIdAttribute) => { - childrenIdAttrToIdRef.current.delete(childIdAttribute); - rerender(); + // Map.delete returns true only when an entry existed, so duplicate/no-op calls skip the re-render. + if (childrenIdAttrToIdRef.current.delete(childIdAttribute)) { + rerender(); + } }, parentId: itemId,
Perf is a non-issue: the re-render hits only the provider (stable context value + same children element, so no subtree re-render), batches per commit, and this path is SimpleTreeView-only -- RichTreeView never mounts it, so large datasets are unaffected.
| fireEvent.click(view.getItemCheckboxInput('1')); | ||
| fireEvent.click(view.getItemContent('1')); | ||
| expect(view.getSelectedTreeItems()).to.deep.equal(['1', '1.1', '1.2']); | ||
| await waitFor(() => { |
There was a problem hiding this comment.
This is technically a small BC worth flagging in the release notes.
Bumps
@mui/materialfamily (material, system, utils, icons-material) to 9.1.0.9.1.0's
Collapsemounts children in a deferred render, soSimpleTreeView's child-discovery effect never re-ran. Force a re-render on child register/unregister so collapsed-parent descendant selection works. Full jsdom suite green on 9.1.0.