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

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

Open
marchingband opened this issue Apr 17, 2024 · 12 comments
Open

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

marchingband opened this issue Apr 17, 2024 · 12 comments
Assignees

Comments

@marchingband
Copy link

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
Copy link
Owner

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

@marchingband
Copy link
Author

Thanks!

@lukasbach
Copy link
Owner

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
Copy link
Author

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
Copy link
Author

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
Copy link
Author

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
Copy link
Owner

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.

@lukasbach lukasbach reopened this May 18, 2024
@marchingband
Copy link
Author

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
Copy link
Author

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
Copy link
Author

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
Copy link
Author

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

@lukasbach
Copy link
Owner

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 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants