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

Refactor completion doc popup and fix positioning #5842

Merged

Conversation

sudormrfbin
Copy link
Member

Refactor the logic for rendering completion documentation popup. The changes include using an early return, renaming some variables (overshadowing area, height, etc tends to be confusing) and making some area calculations more readable. Most importantly this PR also contains a fix for preventing the rendering of the completion doc popup over the completion popup itself and to not draw over the cursor.

Before After
before after

Each commit does a single refactor and the last commit contains the bug fix (The full diff across all the commits might be hard to read since the first commit introduces an early return that dedents a whole block; stepping through the commits should be easier for reviewing).

@sudormrfbin sudormrfbin added A-helix-term Area: Helix term improvements S-waiting-on-review Status: Awaiting review from a maintainer. labels Feb 5, 2023
@pascalkuthe
Copy link
Member

Right now we render the documentation popup on the right of the completion popup when there is enough space. Maybe instead of showing the completion popup on top when there isn't enough space, we should show it in the left instead.
I think vscore does but that way too (and actually defaults to the left but that's not that importent).

@sudormrfbin
Copy link
Member Author

I actually wanted to implement it but forgot about it in the middle of fixing the bug, thanks for the reminder :) Should be a pretty small change.

@pascalkuthe pascalkuthe added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from a maintainer. labels Feb 6, 2023
@pascalkuthe pascalkuthe added this to the next milestone Feb 9, 2023
Copy link
Member

@archseer archseer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work!

@archseer archseer merged commit 425315d into helix-editor:master Feb 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-helix-term Area: Helix term improvements S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants