[TreeView] New API method: `setItemExpansion` #12595

flaviendelangle · 2024-03-28T14:34:36Z

Closes #9960

I would like to merge #12428 to write the new tests in the right place instead of moving them in 2 days

mui-bot · 2024-03-28T14:39:37Z

Deploy preview: https://deploy-preview-12595--material-ui-x.netlify.app/

Updated pages:

Generated by 🚫 dangerJS against faa8bca

flaviendelangle · 2024-03-28T14:36:52Z

packages/x-tree-view/src/internals/plugins/useTreeViewExpansion/useTreeViewExpansion.types.ts

@@ -2,10 +2,20 @@ import * as React from 'react';
 import { DefaultizedProps, TreeViewPluginSignature } from '../../models';
 import { UseTreeViewItemsSignature } from '../useTreeViewItems';

-export interface UseTreeViewExpansionInstance {
+export interface UseTreeViewExpansionPublicAPI {


@noraleonte I propose you to switch the logic for the definition of the instance / publicAPI and to instead define the method on XXXPublicAPI and use extend on XXXInstance.
That way it's easy to be sure that every public method has a good JSDoc.

That's probably a much better way to do it 👍

flaviendelangle · 2024-03-28T14:37:55Z

packages/x-tree-view/src/internals/plugins/useTreeViewExpansion/useTreeViewExpansion.types.ts

+   * @param {string} itemId The id of the item to modify.
+   * @param {boolean} isExpanded The new expansion status of the given item.
+   */
+  setItemExpansion: (event: React.SyntheticEvent, itemId: string, isExpanded: boolean) => void;


The main DX question here is about the event

Do we require the user to pass an event? This is needed right now to call the onItemExpansionToggle prop.
If we don't, do we only call onItemExpansionToggle when the event is defined or do we make the event nullable on onItemExpansionToggle?

Do we require the user to pass an event

I personally don't see a problem with keeping the event required here 🤔 I'm trying to think of a scenario where users would want to specifically avoid passing the event here 🤔

We could imagine scenarios where there is no event (expand an item when receiving some data from the server for instance).
But I'd be in favor of keeping the event mandatory and making it nullable if we have feedback of valid use-cases that ask for it.

flaviendelangle · 2024-03-28T14:39:55Z

packages/x-tree-view/src/internals/plugins/useTreeViewExpansion/useTreeViewExpansion.ts

  params,
  models,
 }) => {
+  const expandedItemsMap = React.useMemo(() => {


I took the opportunity to do this performance improvement now because in order to avoid code duplication between toggleItemExpansion and setItemExpansion, I need to check the expansion status twice and doing it with a linear complexity with awefull.

@romgrk do you have a preference on the data structure to use for this model? Knowing that for now it's purely internal and we keep the array for the expandedItems prop (we can reconsider in the future).

flaviendelangle · 2024-03-28T15:09:55Z

packages/x-tree-view/src/internals/useTreeView/useTreeViewModels.ts

-      const value = model.isControlled
-        ? props[modelName as keyof DefaultizedParams]
-        : modelsState[modelName];
+      const value = props[modelName as keyof DefaultizedParams] ?? modelsState[modelName];


This change makes sure that if we switch a model from controlled to uncontrolled, we will still have a valid value on the model.
This is a bad pattern and a warning will be shown (defined below in this file), but at least it does not crash with in some random place of the code and people can understand what is the problem.

flaviendelangle · 2024-04-02T11:33:03Z

test/utils/tree-view/describeTreeView/describeTreeView.tsx

@@ -177,13 +187,14 @@ const innerDescribeTreeView = <TPlugin extends TreeViewAnyPluginSignature>(
        );

        const result = render(
-          <SimpleTreeView slots={slots} {...other}>
+          <SimpleTreeView slots={slots} apiRef={apiRef} {...other}>


I don't want to add ! on every single test when we know the ref will never be undefined

noraleonte

Nice work, LGTM 👍 🎉

noraleonte · 2024-04-03T10:11:26Z

packages/x-tree-view/src/internals/plugins/useTreeViewExpansion/useTreeViewExpansion.types.ts

+   * @param {string} itemId The id of the item to modify.
+   * @param {boolean} isExpanded The new expansion status of the given item.
+   */
+  setItemExpansion: (event: React.SyntheticEvent, itemId: string, isExpanded: boolean) => void;


Do we require the user to pass an event

I personally don't see a problem with keeping the event required here 🤔 I'm trying to think of a scenario where users would want to specifically avoid passing the event here 🤔

noraleonte · 2024-04-03T10:11:50Z

packages/x-tree-view/src/internals/plugins/useTreeViewExpansion/useTreeViewExpansion.types.ts

@@ -2,10 +2,20 @@ import * as React from 'react';
 import { DefaultizedProps, TreeViewPluginSignature } from '../../models';
 import { UseTreeViewItemsSignature } from '../useTreeViewItems';

-export interface UseTreeViewExpansionInstance {
+export interface UseTreeViewExpansionPublicAPI {


That's probably a much better way to do it 👍

LukasTy

Nice improvement! 👍

docs/data/tree-view/rich-tree-view/expansion/ChangeItemExpansion.tsx

docs/data/tree-view/rich-tree-view/expansion/ChangeItemExpansion.js

docs/data/tree-view/rich-tree-view/expansion/expansion.md

docs/data/tree-view/simple-tree-view/expansion/ChangeItemExpansion.js

docs/data/tree-view/simple-tree-view/expansion/ChangeItemExpansion.tsx

packages/x-tree-view/src/internals/plugins/useTreeViewExpansion/useTreeViewExpansion.types.ts

docs/data/tree-view/simple-tree-view/expansion/expansion.md

…on.tsx Co-authored-by: Lukas <llukas.tyla@gmail.com> Signed-off-by: Flavien DELANGLE <flaviendelangle@gmail.com>

…on.js Co-authored-by: Lukas <llukas.tyla@gmail.com> Signed-off-by: Flavien DELANGLE <flaviendelangle@gmail.com>

Co-authored-by: Lukas <llukas.tyla@gmail.com> Signed-off-by: Flavien DELANGLE <flaviendelangle@gmail.com>

…sion.tsx Co-authored-by: Lukas <llukas.tyla@gmail.com> Signed-off-by: Flavien DELANGLE <flaviendelangle@gmail.com>

…sion.js Co-authored-by: Lukas <llukas.tyla@gmail.com> Signed-off-by: Flavien DELANGLE <flaviendelangle@gmail.com>

[TreeView] New API method: setItemExpansion

324c15c

flaviendelangle self-assigned this Mar 28, 2024

flaviendelangle added the component: tree view TreeView, TreeItem. This is the name of the generic UI component, not the React module! label Mar 28, 2024

flaviendelangle commented Mar 28, 2024

View reviewed changes

flaviendelangle added 2 commits March 28, 2024 15:47

Fix CI

a87c964

Fix

066bd84

flaviendelangle changed the title ~~[TreeView] New API method: setItemExpansion~~ [TreeView] New API method: setItemExpansion Mar 28, 2024

flaviendelangle commented Mar 28, 2024

View reviewed changes

flaviendelangle marked this pull request as ready for review March 29, 2024 07:23

flaviendelangle requested review from noraleonte and LukasTy March 29, 2024 07:23

flaviendelangle added 2 commits April 2, 2024 13:05

Merge branch 'master' into publicAPI-setItemExpansion

6546d40

Fix

bef4f91

flaviendelangle commented Apr 2, 2024

View reviewed changes

noraleonte approved these changes Apr 3, 2024

View reviewed changes

flaviendelangle mentioned this pull request Apr 3, 2024

[TreeView] Add JSDoc to all publicAPI methods #12649

Merged

LukasTy approved these changes Apr 3, 2024

View reviewed changes

flaviendelangle and others added 6 commits April 3, 2024 16:37

Update docs/data/tree-view/rich-tree-view/expansion/ChangeItemExpansi…

9f8678c

…on.tsx Co-authored-by: Lukas <llukas.tyla@gmail.com> Signed-off-by: Flavien DELANGLE <flaviendelangle@gmail.com>

Update docs/data/tree-view/rich-tree-view/expansion/ChangeItemExpansi…

5330824

…on.js Co-authored-by: Lukas <llukas.tyla@gmail.com> Signed-off-by: Flavien DELANGLE <flaviendelangle@gmail.com>

Update docs/data/tree-view/rich-tree-view/expansion/expansion.md

b918f4a

Co-authored-by: Lukas <llukas.tyla@gmail.com> Signed-off-by: Flavien DELANGLE <flaviendelangle@gmail.com>

Update docs/data/tree-view/simple-tree-view/expansion/expansion.md

564b722

Co-authored-by: Lukas <llukas.tyla@gmail.com> Signed-off-by: Flavien DELANGLE <flaviendelangle@gmail.com>

Update docs/data/tree-view/simple-tree-view/expansion/ChangeItemExpan…

11c9481

…sion.tsx Co-authored-by: Lukas <llukas.tyla@gmail.com> Signed-off-by: Flavien DELANGLE <flaviendelangle@gmail.com>

Update docs/data/tree-view/simple-tree-view/expansion/ChangeItemExpan…

faa8bca

…sion.js Co-authored-by: Lukas <llukas.tyla@gmail.com> Signed-off-by: Flavien DELANGLE <flaviendelangle@gmail.com>

flaviendelangle merged commit b44c954 into mui:master Apr 3, 2024
15 checks passed

flaviendelangle deleted the publicAPI-setItemExpansion branch April 3, 2024 14:59

flaviendelangle mentioned this pull request Apr 5, 2024

v7.1.1 #12681

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[TreeView] New API method: `setItemExpansion` #12595

[TreeView] New API method: `setItemExpansion` #12595

flaviendelangle commented Mar 28, 2024 •

edited

mui-bot commented Mar 28, 2024 •

edited

flaviendelangle Mar 28, 2024

noraleonte Apr 3, 2024

flaviendelangle Mar 28, 2024

noraleonte Apr 3, 2024 •

edited

flaviendelangle Apr 3, 2024

flaviendelangle Mar 28, 2024

flaviendelangle Mar 28, 2024

flaviendelangle Apr 2, 2024

noraleonte left a comment

noraleonte Apr 3, 2024 •

edited

noraleonte Apr 3, 2024

LukasTy left a comment

[TreeView] New API method: setItemExpansion #12595

[TreeView] New API method: setItemExpansion #12595

Conversation

flaviendelangle commented Mar 28, 2024 • edited

mui-bot commented Mar 28, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

noraleonte Apr 3, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

noraleonte left a comment

Choose a reason for hiding this comment

noraleonte Apr 3, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LukasTy left a comment

Choose a reason for hiding this comment

[TreeView] New API method: `setItemExpansion` #12595

[TreeView] New API method: `setItemExpansion` #12595

flaviendelangle commented Mar 28, 2024 •

edited

mui-bot commented Mar 28, 2024 •

edited

noraleonte Apr 3, 2024 •

edited

noraleonte Apr 3, 2024 •

edited