lukasbach / react-complex-tree

Unopinionated Accessible Tree Component with Multi-Select and Drag-And-Drop
https://rct.lukasbach.com
MIT License
961 stars 77 forks source link

Illegal drag-between and drag-over is allowed with key commands #363

Open marchingband opened 5 months ago

marchingband commented 5 months ago

When I use the mouse to drag and drop, illegal moves are not allowed, and the indicator does not show. For example if I try to drag a parent onto its own child. When I use the key commands to do the same, the indicators do show, I am allowed to drop, and can easily break the tree.

lukasbach commented 5 months ago

Hi! Thanks for the report. I can reproduce the issue, and will look into a solution once I have some availability.

marchingband commented 5 months ago

Thanks!

lukasbach commented 5 months ago

Hi, thanks for the report. The issue should be fixed as of v2.4.3. Please let me know or reopen if something doesn't work as expected.

marchingband commented 4 months ago

Thanks! This seems to work, but there is a small lingering bug where one drop target is directly under a directory, but not in the directory, which is not a real position. The drop zone for immediately-under-the-directory also exists where it should (under all the children) but there is a zombie drop point above all the children, and above position 0 of the children (top of the list), which should not exist.

marchingband commented 4 months ago

I solve it with

canDropAt={(item, target) => {
    const isFolder = indexIsUnderFolder(item, target)
    return !isFolder
}}

const indexIsUnderFolder = useCallback((item, target) => {
        // the library renders a line under the directory root but it drops the item under the whole directory :-/ so we have to weed these out.
        // console.log({items, item, target})
        const { parentItem, childIndex, linearIndex, targetType, linePosition } = target
        if(
            targetType == "between-items" &&
            linePosition == "bottom" &&
            childIndex > 0
        ){
            const parent = items[parentItem]
            const itemAboveId = parent.children[childIndex - 1]
            const itemAbove = items[itemAboveId]

            // if its a folder, and its expanded, and its not empty, then we have the bad one
            return (itemAbove.isFolder && (itemAbove.children.length > 0) && expandedItems.includes(itemAbove.index))
    } else {
            // not a folder
            return false
        }
    }, [items, expandedItems])

Which is funny and 1/2 crazy but does work.

marchingband commented 4 months ago

Your keypress API is also not working perfectly, I am setting them to [] but the defaults still persist. Is there another way to remove your listeners?

lukasbach commented 4 months ago

I'll have another look at the redundant drop target.

Setting all elements in the keyboardBindings prop to an empty arrow should disable all hotkeys, i.e. setting

keyboardBindings={{
  primaryAction: [],
  moveFocusToFirstItem: [],
  moveFocusToLastItem: [],
  expandSiblings: [],
  renameItem: [],
  abortRenameItem: [],
  toggleSelectItem: [],
  abortSearch: [],
  startSearch: [],
  selectAll: [],
  startProgrammaticDnd: [],
  abortProgrammaticDnd: [],
  completeProgrammaticDnd: [],
}}

as prop. If what you mean is that the arrow keys still work, the arrow key keybindings are handled differently because they have a different behavior, so in the past, there was no way of disabling them. I've added an additional disableArrowKeys prop on the tree environment in the latest version which you can use to also disable arrow hotkeys. Or was it a different issue you had with disabling arrow hotkeys?

Also, I've seen that you mentioned in a comment that default keys conflict Windows system hotkeys. I guess you removed the comment, but do you mind sharing details on the conflict? I would be very interest if there is an actual conflict, since this should be avoided in the defaults.

marchingband commented 4 months ago

It was alt-space on windows, I was confused. I test my apps on a windows machine but have a Mac keys on my keyboard so it's confusing.

I am not referring to arrow keys. CTRL-space for example persists, although my code looks exactly like yours.

marchingband commented 4 months ago

Another issue I am facing is that when I use tab to navigate to the tree, the first item that gets focussed is not really focussed, so select, dnd, etc., will not work until I use the arrow keys at least once.

marchingband commented 4 months ago

I'm really sorry to bombard you here. I am assuming you want to know all this. Let me know if I can improve these reports, or if they are needed. I am able to debug/patch/workaround most of these issues myself, I am reporting just to be helpful.

marchingband commented 4 months ago

Also my client is making significant use of this, and I can suggest he make a donation at some point.

lukasbach commented 4 months ago

No worries for bombarding, thanks for the details! I will have another look at the problems you describe once I have some more time. I would be happy for a donation notice :)