grapp-dev / nui-components.nvim

A feature-rich and highly customizable library for creating user interfaces in Neovim.
https://nui-components.grapp.dev
MIT License
303 stars 6 forks source link

feat: add `Renderer:get_component_by_direction` #8

Closed willothy closed 5 months ago

willothy commented 5 months ago

Adds support for directional queries in forms via a utility function.

This function just returns the nearest component in a given direction from the given component or the focused one.

mobily commented 5 months ago

@willothy thanks for contributing again! ❤️ what's the use case of that feature?

willothy commented 5 months ago

I prefer to use <C-h>/<C-j>/<C-h>/<C-l> for directional movement in forms for more granular control over focus.

I added the ability to query starting from non-current nodes just as a utility but I don't have an actual use case for that part atm.

mobily commented 5 months ago

awesome! If I understand correctly, we could give our devs/users some choices for gaining focus on components; how about adding a new option to the Renderer called focus_strategy?

willothy commented 5 months ago

awesome! If I understand correctly, we could give our devs/users some choices for gaining focus on components

Exactly! So one could use <Tab> to traverse the form sequentially, and the mappings I mentioned to switch between inputs directly.

how about adding a new option to the Renderer called focus_strategy?

Imo it'd be best to allow both options simultaneously - the way I have it at the moment just uses mappings in my config which makes it simple. That could work as a way to opt-in to directional movement too though, as long as it's possible to use both.

Maybe the options for focus_strategy could be sequential, directional, and all?

focus_strategy

I like the idea, but what do you think about calling it focus_mappings instead? I think it's not super apparent from the word strategy that the option has to do with mappings, and focus_mappings makes it super clear to the user that this just affects mappings.

mobily commented 5 months ago

but what do you think about calling it focus_mappings instead?

@willothy that sounds good to me! mind updating your PR accordingly to what we just agreed on? 🙏

willothy commented 5 months ago

Definitely! Didn't have time to update it last night but will do sometime today :)

willothy commented 5 months ago

Actually, I think it may be better to just add focus_up, focus_down, etc. to the global mappings, but unmapped by default. This way users can just add focus_right = "<C-l>" to their mappings tables.

Edit: @mobily I've updated the branch - thoughts?

willothy commented 5 months ago

@mobily changes made and docs updated!

Also I'm sure this can be optimized by doing a tree traversal instead of iteration, but this works for now and I'll try to PR in a faster solution later if I find one :)