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

Fix scrolling by adding missing NSEventPhaseEnded check #94360

Conversation

adamscott
Copy link
Member

This fix an issue that it is impossible to scroll a autocompletion box using a trackpad on a MacBook Pro. It's because NSEventPhaseNone is considered legacy and was replaced with NSEventPhaseEnded.

The current code makes it so that the mouse scrolling emulation is never triggered because NSEventPhaseNone isn't emitted by the OS.

@adamscott
Copy link
Member Author

Makes me think that we should maybe update InputEventPanGesture to handle when the input is momentum based (created by the OS) or not.

Copy link
Member

@bruvzg bruvzg left a comment

Choose a reason for hiding this comment

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

That's gonna break all pan events (no longer possible to pan 2D view and rotate 3D view, for example - pan gesture will zoom instead, which is wrong, you zoom with pinch gesture). I do not think it should emulate mouse in this case, probably should be fixed in the PopupMenu (or whatever control is used for autocompletion) instead (other controls do handle scroll with the trackpad without issues).

@adamscott adamscott closed this Jul 14, 2024
@bruvzg
Copy link
Member

bruvzg commented Jul 14, 2024

diff --git a/scene/gui/code_edit.cpp b/scene/gui/code_edit.cpp
index c3c4b1d3fb..b43bae6217 100644
--- a/scene/gui/code_edit.cpp
+++ b/scene/gui/code_edit.cpp
@@ -251,6 +251,24 @@ void CodeEdit::_notification(int p_what) {
 }
 
 void CodeEdit::gui_input(const Ref<InputEvent> &p_gui_input) {
+	Ref<InputEventPanGesture> pan_gesture = p_gui_input;
+	if (pan_gesture.is_valid() && code_completion_active && code_completion_rect.has_point(pan_gesture->get_position())) {
+		const real_t delta = pan_gesture->get_delta().y;
+		if (delta < 0) {
+			if (code_completion_current_selected > 0) {
+				code_completion_current_selected--;
+				code_completion_force_item_center = -1;
+				queue_redraw();
+			}
+		} else {
+			if (code_completion_current_selected < code_completion_options.size() - 1) {
+				code_completion_current_selected++;
+				code_completion_force_item_center = -1;
+				queue_redraw();
+			}
+		}
+	}
+
 	Ref<InputEventMouseButton> mb = p_gui_input;
 	if (mb.is_valid()) {
 		// Ignore mouse clicks in IME input mode, let TextEdit handle it.

This won't be usable as is, since it will scroll too fast, but that's the place to fix it. Probably should have some float position variable to accumulate delta value for reasonable scroll speed.

@bruvzg
Copy link
Member

bruvzg commented Jul 14, 2024

Haven't fully tested it (might need resetting in some other places) but seems to be working OK - #94363

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

Successfully merging this pull request may close these issues.

3 participants