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

[TreeView] Add label editing feature #13388

Merged
merged 110 commits into from
Aug 13, 2024
Merged

Conversation

noraleonte
Copy link
Contributor

@noraleonte noraleonte commented Jun 5, 2024

Preview:

https://deploy-preview-13388--material-ui-x.netlify.app/x/react-tree-view/rich-tree-view/editing/

To do:

  • Finetune the behavior on double click
  • add callback method
  • finetune api for controlling the editing state (and limiting it to icon buttons as opposed to keyboard interactions)
  • create docs
  • styling of the editing state
  • tests

@noraleonte noraleonte added new feature New feature or request component: tree view TreeView, TreeItem. This is the name of the generic UI component, not the React module! labels Jun 5, 2024
@noraleonte noraleonte self-assigned this Jun 5, 2024
@mui-bot
Copy link

mui-bot commented Jun 5, 2024

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jun 10, 2024
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Jun 17, 2024
Copy link
Member

@LukasTy LukasTy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome feature, massive effort and superb result! 🎉 👏
Congrats! 💙

Leaving some open questions and nitpicks. 😉

import EditOutlinedIcon from '@mui/icons-material/EditOutlined';
import CloseRoundedIcon from '@mui/icons-material/CloseRounded';
import CheckIcon from '@mui/icons-material/Check';
import { TreeItem2LabelInput } from '@mui/x-tree-view/TreeItem2/TreeItem2';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have any idea why ESLint didn't catch these? 🤔

docs/data/tree-view/rich-tree-view/editing/editing.md Outdated Show resolved Hide resolved
response.getItemRoot('1').focus();
});
fireEvent.doubleClick(response.getItemLabel('1'));
fireEvent.change(response.getItemLabelInput('1'), { target: { value: 'new value' } });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you considered using the newly available userEvent on the response and the Keyboard API? 🤔
https://testing-library.com/docs/user-event/keyboard

packages/x-tree-view/src/useTreeItem2/useTreeItem2.ts Outdated Show resolved Hide resolved
@LukasTy LukasTy changed the title [Tree View] Add label editing feature [TreeView] Add label editing feature Aug 1, 2024
Copy link
Member

@flaviendelangle flaviendelangle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For me we are almost good to go

I think we need to make the feature unstable like the drag & drop one, I would follow the same pattern and just add an experimentalFeature={{ labelEditing }} flag.

Copy link
Member

@LukasTy LukasTy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, awesome work! 🎉💯
Leaving a couple final comments regarding demos. 👍

Copy link
Member

@LukasTy LukasTy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, congrats! 👍 💯 💙 🚀

Copy link
Member

@flaviendelangle flaviendelangle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the terrific work on this feature! 👏

@noraleonte noraleonte merged commit 471b118 into mui:master Aug 13, 2024
18 checks passed
@noraleonte noraleonte deleted the label-editing branch August 13, 2024 12:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: tree view TreeView, TreeItem. This is the name of the generic UI component, not the React module! new feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants