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 move RichTreeView editable label input caret when itemsReordering is enabled (Firefox) #15533

Open
kzimins-arvatosystems opened this issue Nov 21, 2024 · 5 comments · May be fixed by #15636
Labels
bug 🐛 Something doesn't work component: tree view TreeView, TreeItem. This is the name of the generic UI component, not the React module! support: pro standard Support request from a Pro standard plan user. https://mui.com/legal/technical-support-sla/

Comments

@kzimins-arvatosystems
Copy link

kzimins-arvatosystems commented Nov 21, 2024

Steps to reproduce

Steps:

  1. Open this link to live example in Firefox: https://codesandbox.io/p/sandbox/silly-wing-j5fcdq
  2. Double-click on any tree item
  3. When input appears, you couldn't move caret inside input by clicking anywhere in text

Current behavior

In Firefox you couldn't click inside any input text place to move caret, but could in Chrome

Firefox:
https://github.com/user-attachments/assets/bcd75d2a-2887-45ba-bb0d-36842eebb1d2

Chrome:
https://github.com/user-attachments/assets/584ac8f2-f5a1-43c4-88db-b9045478d956

Expected behavior

No response

Context

Caret moving by click works if I just do return false in isItemReorderable and canMoveItemToNewPosition, but If I'm doing some checks inside then it's not working

Your environment

No response

Search keywords: RichTreeView, labelEditing, itemsReordering
Order ID: 101720

@kzimins-arvatosystems kzimins-arvatosystems added bug 🐛 Something doesn't work status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Nov 21, 2024
@github-actions github-actions bot added component: tree view TreeView, TreeItem. This is the name of the generic UI component, not the React module! support: pro standard Support request from a Pro standard plan user. https://mui.com/legal/technical-support-sla/ labels Nov 21, 2024
@kzimins-arvatosystems kzimins-arvatosystems changed the title Couldn't move RichTreeView editable label input caret when itemsReordering is enabled (Firefox) Can't move RichTreeView editable label input caret when itemsReordering is enabled (Firefox) Nov 21, 2024
@michelengelen
Copy link
Member

Hey @kzimins-arvatosystems ... first of all thanks for opening an issue here.

It seems you are using the canMoveItemToNewPosition prop a bit wrong. The function signature looks like this:

  canMoveItemToNewPosition?: (params: {
    itemId: string;
    oldPosition: TreeViewItemReorderPosition;
    newPosition: TreeViewItemReorderPosition;
  }) => boolean;

So in your case you would need to apply this diff:

-         canMoveItemToNewPosition={(itemId) => {
+         canMoveItemToNewPosition={({ itemId }) => {

Even with that the method will not be of help as it will always only return the item you are currently dragging. Its better to compare the old and new position for a valid drop action like in the example in the docs.

So when using a correct canMoveItemToNewPosition function it works in Firefox as well!

@michelengelen michelengelen added status: waiting for author Issue with insufficient information and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Nov 21, 2024
@michelengelen michelengelen changed the title Can't move RichTreeView editable label input caret when itemsReordering is enabled (Firefox) [tree view] Can't move RichTreeView editable label input caret when itemsReordering is enabled (Firefox) Nov 21, 2024
@kzimins-arvatosystems
Copy link
Author

Hey @kzimins-arvatosystems ... first of all thanks for opening an issue here.

It seems you are using the canMoveItemToNewPosition prop a bit wrong. The function signature looks like this:

  canMoveItemToNewPosition?: (params: {
    itemId: string;
    oldPosition: TreeViewItemReorderPosition;
    newPosition: TreeViewItemReorderPosition;
  }) => boolean;

So in your case you would need to apply this diff:

-         canMoveItemToNewPosition={(itemId) => {
+         canMoveItemToNewPosition={({ itemId }) => {

Even with that the method will not be of help as it will always only return the item you are currently dragging. Its better to compare the old and new position for a valid drop action like in the example in the docs.

So when using a correct canMoveItemToNewPosition function it works in Firefox as well!

Hi @michelengelen, thanks for the answer! I've mistaken in my example, but I use callbacks correctly in my project.

image

I don't know what's wrong, because if I just return false without any check in those callback caret position changes even in firefox.
I've updated my demo and disable reordering for item with label @mui/x-data-grid. Same result.

Didn't working: https://codesandbox.io/p/sandbox/silly-wing-j5fcdq
Works: https://codesandbox.io/p/sandbox/upbeat-matsumoto-wc8jrr

@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 Nov 21, 2024
@michelengelen
Copy link
Member

Hey @kzimins-arvatosystems ... your second sandbox is private. Could you make it public so i can view it as well?
I did play around with your first example a bit and if you try this:

isItemReorderable={(itemId) => {
  const item = apiRef.current?.getItem(itemId);

  return item.label.endsWith("-pro");
}}
canMoveItemToNewPosition={({ newPosition }) => {
  return newPosition.index > 1;
}}

it works like a charm.

For some reason the method you passed does not produce the correct results.

@michelengelen michelengelen added status: waiting for author Issue with insufficient information and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Nov 22, 2024
@kzimins-arvatosystems
Copy link
Author

Hey @kzimins-arvatosystems ... your second sandbox is private. Could you make it public so i can view it as well? I did play around with your first example a bit and if you try this:

isItemReorderable={(itemId) => {
  const item = apiRef.current?.getItem(itemId);

  return item.label.endsWith("-pro");
}}
canMoveItemToNewPosition={({ newPosition }) => {
  return newPosition.index > 1;
}}

it works like a charm.

For some reason the method you passed does not produce the correct results.

I made it public, thanks!

With your example editing works good only for those items that have no pro in the end, for others I can't move caret as It was before. So that was my question: how to disable reordering for items I edit and don't lose edit functionality in FF

@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 Nov 22, 2024
@michelengelen
Copy link
Member

Ah, I get it now ... when editing you are not able to select using the mouse, since it starts the dragging. 👍🏼

But that's also happening on chrome.

I found a "solution" to the problem, but it leads to another one I cannot find:

--- a/packages/x-tree-view-pro/src/internals/plugins/useTreeViewItemsReordering/useTreeViewItemsReordering.itemPlugin.ts
+++ b/packages/x-tree-view-pro/src/internals/plugins/useTreeViewItemsReordering/useTreeViewItemsReordering.itemPlugin.ts
@@ -7,6 +7,7 @@ import {
   isTargetInDescendants,
   useSelector,
 } from '@mui/x-tree-view/internals';
+import { selectorIsItemBeingEdited } from '@mui/x-tree-view/internals/plugins/useTreeViewLabel/useTreeViewLabel.selectors';
 import {
   UseTreeItemDragAndDropOverlaySlotPropsFromItemsReordering,
   UseTreeItemRootSlotPropsFromItemsReordering,
@@ -34,6 +35,7 @@ export const useTreeViewItemsReorderingItemPlugin: TreeViewItemPlugin = ({ props
     itemId,
   );
   const isValidTarget = useSelector(store, selectorItemsReorderingIsValidTarget, itemId);
+  const isBeingEdited = useSelector(store, selectorIsItemBeingEdited, itemId);

   return {
     propsEnhancers: {
@@ -44,6 +46,7 @@ export const useTreeViewItemsReorderingItemPlugin: TreeViewItemPlugin = ({ props
       }): UseTreeItemRootSlotPropsFromItemsReordering => {
         if (
           !itemsReordering.enabled ||
+          isBeingEdited ||
           (itemsReordering.isItemReorderable && !itemsReordering.isItemReorderable(itemId))
         ) {
           return {};

@flaviendelangle when I try to select text with the cursor it leaves the edit mode. Otherwise this works as expected. Could you have a look? Thanks!

@noraleonte noraleonte linked a pull request Nov 27, 2024 that will close this issue
1 task
@michelengelen michelengelen removed the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Nov 28, 2024
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! support: pro standard Support request from a Pro standard plan user. https://mui.com/legal/technical-support-sla/
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants