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

Cursor blinking is triggered by changes in inactive tabs #437

Open
hougesen opened this issue Jan 28, 2024 · 5 comments
Open

Cursor blinking is triggered by changes in inactive tabs #437

hougesen opened this issue Jan 28, 2024 · 5 comments
Labels
wishlist Features wishlist

Comments

@hougesen
Copy link
Contributor

hougesen commented Jan 28, 2024

Changes in other tabs should not cause the active tab to rerender since it makes the ui seem buggy.

Screencast.from.2024-01-28.16-23-43.webm
@hougesen
Copy link
Contributor Author

Resizing the window also triggers a lot of rerenders.

Screencast.from.2024-01-28.16-40-10.webm

@raphamorim
Copy link
Owner

Good findings, yes those behaviors should be fixed. Now cursor can come either from terminal and state, most of times comes from state, but would be good move part of cursor rendering logic outside of state (state reflects in the current window rendering state).

@raphamorim raphamorim added the wishlist Features wishlist label Jan 28, 2024
@hougesen
Copy link
Contributor Author

hougesen commented Jan 28, 2024

Are the changes required affected by #428?

I also noticed selecting text is a lot slower when blinking-cursor is turned on.

Turned off:

Screencast.from.2024-01-28.23-58-52.webm

Turned on:

Screencast.from.2024-01-28.23-59-41.webm

It is not a problem when using nvim or zellij, which I am guessing is because they handle cursor state themself (?).

Edit:

Adding a check for whether there is a selection eliminates the delay after first render.

if self.state.has_blinking_enabled && has_blinking_enabled {
self.context_manager.schedule_render(800);
}

if self.state.has_blinking_enabled
	&& has_blinking_enabled
	&& self.selection_is_empty()
{
	self.context_manager.schedule_render(800);
}

I am not sure whether this is a desired long term solution, but it might make sense as a quick fix?

@raphamorim
Copy link
Owner

Are the changes required affected by #428?

No shouldn't be affected, #428 should touch mostly sugarloaf crate.

I also noticed selecting text is a lot slower when blinking-cursor is turned on.

Hmm, that would make sense because is stacking rendering and blinking (rendering) in the same time.

Adding a check for whether there is a selection eliminates the delay after first render.

I like the solution for now but shouldn't be the final fix (I guess) because could be that users would expect the cursor to blink if have selected text (?? not sure tbh).

Would you mind create a PR with those changes?

@hougesen
Copy link
Contributor Author

Sure, coming up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wishlist Features wishlist
Projects
None yet
Development

No branches or pull requests

2 participants