File Explorer Keyboard Navigation - Part 1 #1086
Conversation
| @@ -4,16 +4,20 @@ type t = { | |||
| tree: option(FsTreeNode.t), | |||
| isOpen: bool, | |||
| scrollOffset: [ | `Start(float) | `Middle(float)], | |||
| focus: option(string) // path | |||
| active: option(string), // path | |||
| focus: option(string) // node id | |||
There was a problem hiding this comment.
Note to self: path, not id
There was a problem hiding this comment.
That would be one nice thing about using a path library like Path/fp - we could have a type for it
| type t = { | ||
| id: int, |
There was a problem hiding this comment.
I've removed id entirely in favor of using paths as identifiers instead. For a few reasons:
- it's simpler.
- Paths don't change (and if they do it's because their identity changes). No more stale ids, they're valid across workspace changes and can even be persisted.
- Searching by path is most likely faster since nodes that aren't in the path can be eliminated from the search. Hashing also makes individual comparisons as fast as on ids.
| path | ||
| |> String.split_on_char(Filename.dir_sep.[0]) | ||
| |> List.map(Hashtbl.hash); | ||
| let equals = (a, b) => a.hash == b.hash && a.path == b.path; |
There was a problem hiding this comment.
Hashes aren't guaranteed to be unique, so use this instead of comparing hashes directly, and especially instead of comparing nodes structurally.
| let onNodeClick = node => { | ||
| Option.iter(Revery.UI.Focus.focus, containerRef); | ||
| onNodeClick(node); | ||
| }; | ||
|
|
||
| let onKeyDown = (event: NodeEvents.keyEventParams) => { | ||
| switch (event.keycode) { | ||
| // Enter | ||
| | v when v == 13 => | ||
| GlobalContext.current().dispatch(Actions.FileExplorer(Select)) | ||
|
|
||
| // arrow up | ||
| | v when v == 1073741906 => | ||
| GlobalContext.current().dispatch(Actions.FileExplorer(FocusPrev)) | ||
|
|
||
| // arrow down | ||
| | v when v == 1073741905 => | ||
| GlobalContext.current().dispatch(Actions.FileExplorer(FocusNext)) | ||
|
|
||
| | _ => () | ||
| }; | ||
| }; | ||
|
|
||
| <View | ||
| onKeyDown style=Styles.container ref={ref => setContainerRef(Some(ref))}> |
There was a problem hiding this comment.
This should all be removed in favor of handling input "bottom-up".
|
Woo! What is this new and amazing CI we got? Didn't even take 30 mins to finish! 🎉 |
| module Path = { | ||
| // Not very robust path-handling utilities. | ||
| // TODO: Make good | ||
|
|
||
| let toRelative = (~base, path) => { | ||
| let base = base == "/" ? base : base ++ Filename.dir_sep; | ||
| Str.replace_first(Str.regexp_string(base), "", path); | ||
| }; | ||
|
|
||
| let explode = String.split_on_char(Filename.dir_sep.[0]); | ||
| }; |
There was a problem hiding this comment.
We could consider using the Fp/Path library from reason-native: https://github.com/facebookexperimental/reason-native/tree/master/src/fp
(if it makes sense)
There was a problem hiding this comment.
Yeah, that looks pretty good. It doesn't seem to have any way of destructuring it like explode though. But we could contribute that.
I like the way you divided the work - seems like a good chunk to bring in. I tried it out and it's so nice to have some keyboard input in the file explorer! |
I tried to go through and fix most of the hangs / issues... hopefully it works better now 🤞 |
|
This is awesome. Looking forward to part 2 and 3! Once it's "nerdtree-like" I will be able to use oni2 for real I think. |
|
Any update on the 'nerdtree-like' behaviour. I am also eagerly waiting for this behaviour to switch to onivim2. |
This implements basic keyboard navigation (up arrow/down arrow) and selection (enter) within the file explorer. There's no way to use the keyboard to focus the file explorer, however, so it's not very useful for end-users yet. And because of the mechanism used to capture keyboard input it also blocks all keybindings. But it's useful for testing keyboard input and relatively self-contained, so I'm submitting this now to try to keep PRs manageable.
Addresses #528