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

feat(preview): Scroll Preview window command and bindings #1470

Merged
merged 2 commits into from
May 15, 2024

Conversation

alexveden
Copy link
Contributor

I'd like to extend #675 with scrolling capabilities.

  1. I took telescope's code for preview scrolling. Code simply sends and , nothing complex.
  2. Scrolling is performed without focusing the preview window (for me, it's important)
  3. Scroll step is configurable

@pysan3
Copy link
Collaborator

pysan3 commented May 14, 2024

Thanks for your PR!

This is really interesting and tbh I didn't imagine this feature could be implemented this easily.

However I believe a lot of users already use <C-d>, <C-u> to scroll (jump half page) the neo-tree buffer itself and I think we should prioritize that over scrolling the preview window.

I would suggest implementing a fallback when if not Preview:is_active() to scroll neo-tree window (so don't always hijack the keybind), OR, comment out your suggested keymap in defaults.lua by default and let the user choose whether to add it and which key to map to. (For example IMO I'll map it to <C-f>, <C-b> cuz I rarely use them to scroll neo-tree).
What do you think?

FYI you should be able to use state.winid but be careful that this valve may be nil.

@alexveden
Copy link
Contributor Author

Thanks for your PR!

I would suggest implementing a fallback when if not Preview:is_active() to scroll neo-tree window (so don't always hijack the keybind), OR, comment out your suggested keymap in defaults.lua by default and let the user choose whether to add it and which key to map to. (For example IMO I'll map it to <C-f>, <C-b> cuz I rarely use them to scroll neo-tree). What do you think?

I don't mind about <C-f> / <C-b> mappings, actually. I don't use <C-d>/<C-u> neither, just saw people were requesting this in the #675.

comment out your suggested keymap in defaults.lua by default and let the user choose whether to add it and which key to map to.

I would stick to alternative mapping then. I'll push changes tommorrow.

@pysan3
Copy link
Collaborator

pysan3 commented May 14, 2024

comment out your suggested keymap in defaults.lua by default and let the user choose whether to add it and which key to map to.

I would stick to alternative mapping then. I'll push changes tommorrow.

Awesome. Please notify that you added the feature in the regarding issues as well so that more people can notice the new feature ;)

And please ping me again after you added the change (my notifications gets flooded pretty quickly 😅).

@alexveden
Copy link
Contributor Author

@pysan3 I've changed the default bindings. Please take a look.

@alexveden
Copy link
Contributor Author

This pull request also related to #672

@pysan3
Copy link
Collaborator

pysan3 commented May 15, 2024

LGTM! Thanks for your contribution.

@pysan3 pysan3 self-requested a review May 15, 2024 15:22
@pysan3 pysan3 changed the title Scroll Preview window command and bindings feat(preview): Scroll Preview window command and bindings May 15, 2024
@pysan3 pysan3 merged commit 6e20108 into nvim-neo-tree:main May 15, 2024
4 checks passed
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

Successfully merging this pull request may close these issues.

None yet

2 participants