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

[marchingband]

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]

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

[marchingband]

Thanks!

[lukasbach]

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]

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]

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]

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]

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]

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]

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]

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]

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

[lukasbach]

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

[marchingband]

Just checking in to see if there was progress on this.
I have a slow burn feature request waiting on it ...

[marchingband]

I discussed the matter with my client and they are happy to contribute some $ to the project if that helps hurry it along some.

[lukasbach]

Hi @marchingband. Sorry for the late reply, I was mostly focusing on other things and didn't get to prioritize RCT recently. Financial support for the development of this library is always appreciated :) I'll try to look into this further and provide you with an update until end of next week.

[marchingband]

@lukasbach thank you! Client made a donation, name is Argumentation_LLC.

[lukasbach]

Hi @marchingband, I've adressed some of the points you mentioned and released the changes. This thread has become fairly long, so I hope I haven't missed anything, please let me know otherwise.

small lingering bug where one drop target is directly under a directory, but not in the directory, which is not a real position

I hope I understood that bug correctly, but the keyboard-targeted drop-target at thje bottom of an expanded folder, which would drop the dragged items on the same level as the folder, directly below the folder item (and thus also below the contents of the expanded folder), will not show up anymore, so this should be fixed.

Your keypress API is also not working perfectly, I am setting them to [] but the defaults still persist. [...] It was alt-space on windows

I found the issue for that, the error was that pressing the space bar also triggers a click event on a focused button element as default behavior by the browser, so this caused the same behavior as clicking on a button while pressing control. I've added special handling so that this doesn't happen when triggering a click event on a button while pressing control. The normal "space triggers click event" behavior remains unchanged in other cases, since this browser-default behavior makes sense otherwise. This can be further customized by library consumers by overwriting the onClick handler completely with a custom interaction mode. Sample for a tree with all hotkeys disabled: Story

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.

I'm not really sure what you mean or what causes this, I can't reproduce this. I have used this sample as baseline, if I move focus from the button to the tree, hotkeys like "End" to jump to the tree bottom, or space to select work fine for me. Do I misunderstand the issue and you can reproduce it on one of the storybook instances, or can you otherwise provide more details on your context that produces this issue, like a sandbox that reproduces the issue or some code excerpt maybe?

[marchingband]

amazing thank you so so much!
The last issue is unfortunately the one I was unable to work around.
It has been a while, so I have to examine it and refresh.
I will make an example and share it soon!

[lukasbach]

Sure! I'll just dump some info from what I found from digging through the RCT code that affects the initial focus, that might be helpful:

• The focused item is always the button with tabindex="0", all other buttons should have tabindex="-1". There should always be exactly one item with tabindex 0, even if no focused item is defined explicitly in the viewState, in which case it should default to the first item in the tree
• There is a method focusItem: (itemId: TreeItemIndex, setDomFocus?: boolean) => void on the tree ref, when calling it with the second param set to true, it should force the DOM focus to the chosen item. Maybe that's helpful, though the proper solution shouldn't have to rely on that
• The tabindex is defined in the interaction manager, in the case that you are implementing an interaction manager entirely on your own without extending an existing one, make sure that the tabIndex is set there correctly similar to how it is done in the default interaction managers: https://github.com/lukasbach/react-complex-tree/blob/main/packages/core/src/interactionMode/ClickItemToExpandInteractionManager.ts#L64

[marchingband]

so I have these elements to my Component (this is highly abridged).

const LoadModal = () => {
    const [focusedItem, setFocusedItem] = useState();
    const [selectedItems, setSelectedItems] = useState([]);
    useFolderKeypress({selectedItems, setSelectedItems, focusedItem, setFocusedItem})
    return(
        <ControlledTreeEnvironment
            onFocusItem={item => setFocusedItem(item.index)}
            onSelectItems={items => setSelectedItems(items)}
        >
            <Tree treeId="tree-1" rootItem="root" treeLabel="Tree Example" ref={tree}/>
        </ControlledTreeEnvironment>
    )
}

And I have a keypress listener

export function useFolderKeypress({selectedItems, setSelectedItems, focusedItem, setFocusedItem}){
    const onFolderKeyPress = useCallback(e=>{
        if(!e.shiftKey || !e.altKey) return;
        switch(e.code){
            ...
            case "KeyG":{
                e.preventDefault()
                if(selectedItems.includes(focusedItem)){
                    setSelectedItems(items => items.filter(item=>item != focusedItem))
                } else {
                     setSelectedItems(items => [...items, focusedItem])
                }
                break;
            }
            ...
        }
    },[ selectedItems, setSelectedItems, focusedItem, setFocusedItem ])
}

If I use tab to navigate to the tree, it focuses the first item in the tree, it is outlined in blue.
If I then press your default ctrl+space, then selection works, it turns grey.
However, if instead, right after navigating to the tree, I use my keybinding via the listener, which is shift+option+g, the selection does not work, presumably because onFocusItem did not yet fire.
If I use the arrow keys once, then shift+option+g now works.

Looking at this, I see it is possible that this is a react quirk, maybe not your problem at all.
I also realize that my initial report did not explain this in any detail at all, and I do apologize for that.
Let me know if you can reproduce this, or if it is indeed unrelated to your library.

I guess I might name it "onFocusItem doesn't fire upon tab-to-tree"

[lukasbach]

I see. I guess the idea behind onFocusItem was a bit different, though I get your intend. onFocusItem triggers whenever the focused item in RCT changes. I don't really track DOM focus in this case, I just track user behavior that should change which tree item should be focused, and communicate that outwards with onFocusItem to wherever this information is stored in the app. As I mentioned above, there is always an item that is focused in RCT, this is important since this element needs to be rendered as the one focusable DOM element in the tree, if there is no focused item defined in the initial viewState, RCT implicitly assumes the first item in the tree is focused. This is why moving DOM focus to the tree initially doesn't trigger the onFocusItem event: From the point of view of RCT, nothing changed, that item was already focused before.

From your excerpt, it looks like this is primarily an issue because you want to keep track of the focused item and act on it even at a point when the initial focus is still on the first item. Would it help, if, instead of RCT implicitly assuming the first item is focused if no items are focused, RCT would trigger onFocusItem explicitly when it renders without a focused item defined in the viewState prop? That would allow you to set this value consistently with onFocusItem. This behavior is released in prerelease 2.5.1-alpha.0 if you want to test if that would work for you.

[marchingband]

That patch certainly works for me, do you think it is ok to add to RCT?
With your help understanding this issue above, I can probably find another workaround, if you don't think its a good change to your lib, but this is the easiest from my perspective.

[marchingband]

I am not sure if this is a bug, but, when dragging an item using key commands, when at the bottom of the tree, pressing down again makes the drop indicator disappear, and renderDragBetweenLine is not called.
If I press up-arrow again, it re-appears at the bottom of the tree, and renderDragBetweenLine is called.

This happens in 2.4.3 and in 2.5.1-alpha.0

The nomenclature suggests it would not be called when at the bottom, so I suspect this is again an issue with the way I am using your lib. Here is how I am using drag and drop, the yellow line represents the drop target position:

Maybe there is a way, using canDropAt to deny this last position, but because of the mechanics of target.linearPosition and friends, I havn't been able to find a good hack...

[lukasbach]

I think the patch makes sense for RCT, I've merged it into the main branch.

The additional drop target when dragging via keyboards is an off-by-one error in RCT I believe, unless what you experienced is something else then what I reproduced, thanks for pointing that out. I've fixed it in the latest version.

Both that and the patch regarding the initial item focus is now released as of 2.6.0.

[marchingband]

That's working beautifully for me thank you so much.

I am noticing now that when doing a keypress drag and drop, then the right/left arrow keys do not expand/collapse branches of the tree. Is there a way i can enable that? It seems like that should work, otherwise I have to expand target sub-directories before starting the drag/drop.

[lukasbach]

Hm, no there currently is no way for doing so. RCT currently evaluates all viable drag positions when the programmatic drag is started, so right now there are no mutations to the tree structure possible while moving the drag position with the keyboard, at least not in an easy way.

[marchingband]

I understand, and I now remember this part about RCT. My impression is that it is a decision to make async lazy-loading of branches possible. It seems to me that in many cases, the full tree would be loaded in RAM already, so this pattern would not apply. I wonder if an option to disable this behaviour would be useful to others as well ... it would be very confusing for a non-sighted user to drag and drop into a folder, or between folders, and we would have to publish special documentation describing this limitation.

[lukasbach]

It's partially about that the tree might not be loaded yet, but also that when doing a keyboard-bound drag, the user uses arrow keys to navigate a flattened/linear list, and the structure of this flattened list depends on which folders are expanded or not. The behavior is actually mostly consistent with other drag use cases, since you can also not expand or collapse items while doing a mouse-bound drag.

[marchingband]

Thanks, I understand. Hopefully this will not be an issue for our auditors.
I am noticing that with NVDA, and Jaws, the arrow keys do not navigate the tree.
This is typically due to an issue in the aria markup.
The arrow keys work when there is no screen reader, and also with macos Voice Over, which is often much more forgiving with markup.
I am examining your markup, and I don't see any obvious flaws, but setting role=tree on a <div> is certainly less standard then doing so on a <ul>, I'm not sure this is the issue, and I'm not sure how to debug without deconstructing your library.

[lukasbach]

Interesting. I'll try to experiment with a screen reader myself during next week. At least the accessibility tool in Chrome Devtools seems to interpret it as proper tree element, it should also be possible to change how that is rendered specifically by customizing the render props of RCT, you can use the default ones as inspiration if you want to experiment with this: https://github.com/lukasbach/react-complex-tree/blob/main/packages/core/src/renderers/createDefaultRenderers.tsx

Btw, I have been working for a longer time now on a successor library for react-complex-tree, which approaches some aspects of tree features a bit more sensibly than RCT. Don't worry, I will still continue maintaining react-complex-tree and fix bugs and issues that come up, but maybe that new library is of interest to you. I didn't want to bring it up yet since it didn't have feature parity with RCT yet and particularly the keyboard-based drag-and-drop interactions weren't implemented at all, but I've taken this opportunity to bring that functionality to the new library as well, and it pretty much has feature parity now with react-complex-tree. I approached the keyboard-drag there with the aspects in mind you mentioned in this thread, so it handles expanding-while-keyboard-dragging properly and also handles other special cases better than RCT. I can of course understand if you don't want to completely exchange the library you are using, but if you are interested, the library is https://headless-tree.lukasbach.com/ and I would be very glad for feedback on that library. Details on keyboard-controlled drag are available here: https://headless-tree.lukasbach.com/features/kdnd