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.07k stars 1.26k forks source link

[tree view] Can't focus input inside TreeItem label #12622

Open IDrumsey opened 5 months ago

IDrumsey commented 5 months ago

Steps to reproduce

Yeah sry, codesandbox was giving me trouble and this is pretty simple I think. I just want to be able to disable the focusing of TreeItems.

The culprit lines I believe are just these.

https://github.com/mui/mui-x/blob/ef40e06213d4721c51d498328f4fadf8e545b3e7/packages/x-tree-view/src/TreeItem/TreeItem.tsx#L292-L297

Maybe we can add a prop or something to just disable running that?

Current behavior

If you do something like

<SimpleTreeView>
  <TreeItem itemId={1} label={<input type="text"/>} />
</SimpleTreeView>

when you click on the input, the TreeItem overrides focusing on the input in favor of the tree item, which essentially means you can't actually use the input.

Expected behavior

Would like to be able to use the input. See suggested solution above.

Context

No response

Your environment

npx @mui/envinfo ``` System: OS: Windows 11 10.0.22631 Binaries: Node: 18.19.0 - C:\Program Files\nodejs\node.EXE npm: 10.5.0 - C:\Program Files\nodejs\npm.CMD pnpm: Not Found Browsers: Chrome: Not Found Edge: Chromium (123.0.2420.65) npmPackages: @emotion/react: ^11.11.4 => 11.11.4 @emotion/styled: ^11.11.0 => 11.11.0 @mui/base: 5.0.0-beta.40 @mui/core-downloads-tracker: 5.15.14 @mui/icons-material: ^5.15.14 => 5.15.14 @mui/material: ^5.15.14 => 5.15.14 @mui/material-nextjs: ^5.15.11 => 5.15.11 @mui/private-theming: 5.15.14 @mui/styled-engine: 5.15.14 @mui/system: 5.15.14 @mui/types: 7.2.14 @mui/utils: 5.15.14 @mui/x-tree-view: ^7.1.0 => 7.1.0 @types/react: ^18 => 18.2.67 react: ^18 => 18.2.0 react-dom: ^18 => 18.2.0 typescript: ^5 => 5.4.3 ```

Search keywords: treeitem

noraleonte commented 5 months ago

Hi, @IDrumsey 👋

Thanks for opening this issue 😄

I double checked and you are right. The focus is always set to the root of the tree item with a specific itemId, and we don't support focusing inner elements of the tree items 😬

A workaround I can think of is using the onItemFocus callback and implement a custom behavior

 onItemFocus={(e, itemId) => {
    const input = document.getElementById(`input-${itemId}`);
    input.focus();
 }}

I will add this to the list of topics to discuss with the team to see if we can come up with a nicer solution 👌

oliviertassinari commented 5 months ago

Is it a regression from https://github.com/mui/material-ui/pull/18894? But the test we added in that PR is still present:

https://github.com/mui/mui-x/blob/ef40e06213d4721c51d498328f4fadf8e545b3e7/packages/x-tree-view/src/TreeItem/TreeItem.test.tsx#L2325-L2350

IDrumsey commented 5 months ago

If you're preventing default then that's probably why it's working.

coxantohi commented 3 weeks ago

Hi, @IDrumsey 👋

Thanks for opening this issue 😄

I double checked and you are right. The focus is always set to the root of the tree item with a specific itemId, and we don't support focusing inner elements of the tree items 😬

A workaround I can think of is using the onItemFocus callback and implement a custom behavior

 onItemFocus={(e, itemId) => {
    const input = document.getElementById(`input-${itemId}`);
    input.focus();
 }}

I will add this to the list of topics to discuss with the team to see if we can come up with a nicer solution 👌

Hi @noraleonte , The suggested workaround does not work when other items are not text inputs. Consider the following example:

<SimpleTreeView
     onItemFocus={(e, itemId) => {
      const input = document.getElementById(`input-${itemId}`);
      input?.focus();
   }}
  >
  <TreeItem itemId="1" label={<input type="text" id="input-1" />} />
  <TreeItem itemId="2" label="B Item" />
  <TreeItem itemId="3" label="C Item" />
</SimpleTreeView>

In this case, as soon as I'll type the B or C letters the focus will switch to another item.

noraleonte commented 3 weeks ago

Hey @coxantohi 👋 I think that in a generic use case, you should be able to work around this issue with a tabIndex and potentially some checks on the callbacks. But I think I would need a more specific reproduction case to figure it out. However, if what you are trying to solve is an editable tree item, we just released a label editing feature available for the RichTreeView and it might be interesting for you 😸

mnixry commented 6 days ago

I've discovered another workaround that utilizes event.stopPropagation to capture event bubbling. This method effectively prevents parent nodes from reacting to click and keydown events.

Here's a working example from my implementation:

<TreeItem
  key={`${nodeId}-empty`}
  itemId={`${nodeId}-empty`}
  label={
    <input
      onKeyDownCapture={(e) => e.stopPropagation()}
      onClickCapture={(e) => e.stopPropagation()}
    />
  }
/>