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

mouse operations respect scrolloff #5255

Merged

Conversation

hunterliao29
Copy link
Contributor

close #5254

Some(pos) => pos,
None => return EventResult::Ignored(None),
};
if let Some((pos, view_id)) = pos_and_view(editor, row, column) {
Copy link
Member

Choose a reason for hiding this comment

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

Why the change from pos_at_screen_coords on the current view to pos_and_view here? Dragging should only affect the currently focused view: with this change, if you start a drag on a focused view and then drag across an unfocused view, you modify the selection on the unfocused view but you can't see the selection because selections are hidden on unfocused views.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did not realize that there are different between pos_at_screen_coords and pos_and_view beside beside pos_and_view also return view.id. I will change it.

@@ -1213,6 +1216,7 @@ impl EditorView {
commands::scroll(cxt, offset, direction);

cxt.editor.tree.focus = current_view;
cxt.editor.ensure_cursor_in_view(current_view);
Copy link
Member

Choose a reason for hiding this comment

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

Is this change necessary? commands::scroll already respects scrolloff

Copy link
Contributor Author

@hunterliao29 hunterliao29 Dec 22, 2022

Choose a reason for hiding this comment

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

Currently the commands::scroll does respects scrolloff. However, if the file like this:

----------------------------| visible at this point--------------| cursor here
longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglong[]onglonglonglong
longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong
longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong
longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong
short or empty
short or empty
short or empty
short or empty
short or empty

after scroll down

--------------| cursor here | still visible at this point
longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglong[]onglonglonglong
longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong
longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong
longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong
short or empty
short or empty
short or empt[]
short or empty
short or empty

the cursor will not be visible and not respects scrolloff.

change pos_and_view back to  pos_at_screen_coords
@the-mikedavis the-mikedavis merged commit df1830e into helix-editor:master Dec 23, 2022
@hunterliao29 hunterliao29 deleted the mouse_respect_scrolloff branch December 23, 2022 03:12
hadronized pushed a commit to hadronized/helix that referenced this pull request Jan 4, 2023
freqmod pushed a commit to freqmod/helix that referenced this pull request Feb 8, 2023
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.

Mouse actions does not trigger scrolloff.
2 participants