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

extensible key bindings #47

Open
dlech opened this issue Mar 21, 2022 · 5 comments
Open

extensible key bindings #47

dlech opened this issue Mar 21, 2022 · 5 comments
Assignees

Comments

@dlech
Copy link
Contributor

dlech commented Mar 21, 2022

It would be useful to be able to add additional key bindings to trees. For example I would like to implement a delete action when the delete key is pressed that receives the focused item and additional state.

It could also be useful to allow 3rd party libraries to implement their own keybindings handling instead of using the built-in one here. This is useful when the 3rd-party library is used to display all keybinding in an app and/or allow the user to customize the key bindings.

@dlech
Copy link
Contributor Author

dlech commented Mar 21, 2022

A nice way to do this could be to add a renderKeybindingsContainer similar to the other render functions. The default implementation would just call useTreeKeyboardBindings() and custom implementations could chose to call this and add additional key bindings or could skip calling it and completely implement their own key binding system.

@lukasbach
Copy link
Owner

Hi @dlech, thank you so much for your contributions, I really appreciate them and your effort! I will look into them later this week and make sure to make them available in a new release.

@lukasbach
Copy link
Owner

Hm I thought about your suggestion, but at the moment you can also pretty much write custom keyboard handlers by just listening to keydown events yourself and use an imperative handle of the tree to manipulate the tree state yourself (https://rct.lukasbach.com/docs/guides/refs) or directly changing the state of a controlled tree environment. And if custom keybindings clash with existing ones, you can disable them by passing [] as keybinding to them. Would this be enough for your scenario?

@dlech
Copy link
Contributor Author

dlech commented Apr 1, 2022

but at the moment you can also pretty much write custom keyboard handlers by just listening to keydown events yourself

The problem I had with this is that there is no access to the internal state of each item to know when an item should respond to a key event or not. Most of the internal keybindings use isActiveTree && !dnd.isProgrammaticallyDragging && !isRenaming which is quite a chore to try to recreate and would need to be kept up-to-date with any internal changes to the react-complex-tree library.

Would this be enough for your scenario?

The reason I suggested an extra render hook is that because (as stated above) the key bindings need to have access to the tree state. In other words, they need to be able to use the useTree() hook. What I did for now is use the renderTreeContainer hook to wrap the default renderTreeContainer with the key bindings.

    return <div onKeyDown={handleKeyDown}>{renderers.renderTreeContainer(props)}</div>;

So, in summary, exposing a state of when to enable/disable the key bindings is the major missing feature. A cleaner way to be able to inject the key binding element as a child of the Tree element would helpful but is not as critical.

@lukasbach
Copy link
Owner

there is no access to the internal state of each item to know when an item should respond to a key event or not

You should be able to get everything from the treeref, with ref.isRenaming, ref.isProgramaticallyDragging and ref.isRenaming you have access to all pieces of data as is internally used for internal keybindings. So you can use existing outwards-facing APIs to compose your hotkey logic rather than using internal logic which is likely more convinent.

which is quite a chore to try to recreate and would need to be kept up-to-date with any internal changes to the react-complex-tree library

The thing is, which properties you want to use to enable or disable depends on the keybinding you're implementing. Yes, for a lot of keybindings it makes sense to listen for those properties specifically and then I agree that it might be inconvenient to repeat this check everytime, but if I provide predefined logic for writing custom hotkeys that specifies this check automatically, you will not be able to write keybindings that e.g. work while renaming or during drag events, which might be important for specific use cases.

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