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

Make EditorProperty and its child EditorProperty behave like sibling nodes when handling mouse events #103316

Conversation

Rindbee
Copy link
Contributor

@Rindbee Rindbee commented Feb 26, 2025

Let EditorProperty::gui_input() no longer handle mouse events that occur in the child EditorProperty rect (bottom_child_rect).

This is another way to solve issues like #88848 , but avoids #100695 .

Bugsquad edit:
Fix #103578

@Rindbee Rindbee requested review from a team as code owners February 26, 2025 11:29
@Rindbee Rindbee force-pushed the make-EditorProperty-and-its-children-behave-like-sibling-nodes branch from df322b8 to 5b2d73f Compare February 26, 2025 11:52
@Rindbee Rindbee changed the title Make editor property and its children behave like sibling nodes Make EditorProperty and its child EditorProperty behave like sibling nodes Feb 26, 2025
@AThousandShips AThousandShips added this to the 4.5 milestone Feb 26, 2025
@JekSun97
Copy link
Contributor

Why is this chosen for 4.5? This is a fix for a common issue, not a new feature, #101768 did not solve the issue completely.

@KoBeWi
Copy link
Member

KoBeWi commented Feb 26, 2025

What case wasn't solved?
The original fix was meant as workaround, this is a proper fix, but it's more risky.

@AThousandShips
Copy link
Member

Also we are in the RC phase and we merge only critical fixes currently

@JekSun97
Copy link
Contributor

What case wasn't solved?

I can still see the inspector shifting abruptly when expanding some properties, such as when editing a material.

0226.mp4

@KoBeWi
Copy link
Member

KoBeWi commented Feb 27, 2025

Tested this PR and while folding now works properly, toggling some properties can still cause jumps:

godot.windows.editor.dev.x86_64_FeyJMyZthI.mp4

It's caused by inspector refresh though, and the scroll ends up in the same position, so it's fine.

@@ -918,6 +918,9 @@ void EditorProperty::gui_input(const Ref<InputEvent> &p_event) {

if (me.is_valid()) {
Vector2 mpos = me->get_position();
if (bottom_child_rect.has_point(mpos)) {
return; // Makes child EditorProperties behave like sibling nodes.
Copy link
Member

Choose a reason for hiding this comment

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

How does that work exactly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just when handling mouse events.

In this case, when clicking on the Label of a child EditorProperty, we don't want the parent EditorProperty's menu to pop up (RMB), but we want the root inspector to have focus (LMB).

… sibling nodes

Let `EditorProperty::gui_input()` no longer handle mouse events that
occur in the child EditorProperty rect.
@Rindbee Rindbee force-pushed the make-EditorProperty-and-its-children-behave-like-sibling-nodes branch from 5b2d73f to 62aa4a6 Compare February 27, 2025 23:40
@Rindbee Rindbee changed the title Make EditorProperty and its child EditorProperty behave like sibling nodes Make EditorProperty and its child EditorProperty behave like sibling nodes when handling mouse events Feb 27, 2025
Copy link
Member

@KoBeWi KoBeWi left a comment

Choose a reason for hiding this comment

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

Seems to work correctly.

This also reverts #101768 and #102775

@Repiteo Repiteo merged commit 0544ed3 into godotengine:master Mar 5, 2025
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Mar 5, 2025

Thanks!

@Rindbee Rindbee deleted the make-EditorProperty-and-its-children-behave-like-sibling-nodes branch March 6, 2025 00:30
@akien-mga akien-mga added the cherrypick:4.4 Considered for cherry-picking into a future 4.4.x release label Mar 17, 2025
@akien-mga
Copy link
Member

Cherry-picked for 4.4.1.

@akien-mga akien-mga removed the cherrypick:4.4 Considered for cherry-picking into a future 4.4.x release label Mar 17, 2025
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.

Material Inspector Panel Scrolls Down Erratically
6 participants