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

Improve clarity of Tree's activated/double-clicked signals #70290

Merged
merged 1 commit into from
Jan 24, 2023

Conversation

aaronfranke
Copy link
Member

When double-clicking a Tree's TreeItem, you might expect item_double_clicked to be emitted. However, this signal is badly named. It's only emitted when the icon is double-clicked, not any other part of the item. The actual signal that works when double-clicking on any part of the item is item_activated.

This PR improves the documentation to make this clear and also renames item_double_clicked to item_icon_double_clicked to reflect that it only emits when double-clicking on an item's icon.

@YeldhamDev
Copy link
Member

Maybe item_activated should also be renamed to item_label_double_clicked?

@aaronfranke
Copy link
Member Author

@YeldhamDev item_activated is also emitted when clicking on the icon, or on the empty space to the left or right side.

@YuriSizov
Copy link
Contributor

This PR improves the documentation to make this clear and also renames item_double_clicked to item_icon_double_clicked to reflect that it only emits when double-clicking on an item's icon.

I think if you add it to the project convertor, it would be okay to break compat here.

doc/classes/Tree.xml Outdated Show resolved Hide resolved
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Approved by production team.

Did a thorough byte-wise review of why the heck GitHub shows a space being added after item here:
image

@YuriSizov YuriSizov dismissed their stale review January 24, 2023 16:03

Changes made

@YuriSizov
Copy link
Contributor

Needs a rebase, though :)

Co-authored-by: Yuri Sizov <yuris@humnom.net>
@YuriSizov YuriSizov merged commit a3a4215 into godotengine:master Jan 24, 2023
@YuriSizov
Copy link
Contributor

Thanks!

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.

None yet

4 participants