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 TreeItem button handling #90839

Merged
merged 1 commit into from
Apr 22, 2024
Merged

Conversation

timothyqiu
Copy link
Member

@timothyqiu timothyqiu commented Apr 18, 2024

Fixes #79629

This PR mainly fixes get_tooltip() and get_button_id_at_position():

  • They did not work when the control is too narrow and when RTL layout is used.
  • They did not take button_margin, h_separation, and item_margin into account.

TestProject.zip

@timothyqiu timothyqiu added this to the 4.3 milestone Apr 18, 2024
@timothyqiu timothyqiu requested a review from a team as a code owner April 18, 2024 08:56
@akien-mga
Copy link
Member

akien-mga commented Apr 18, 2024

Would it make sense to do the revert part only for 4.1 and 4.2, while the rest of the more involved fix gets assessed for 4.3?
Or is the regression it caused less bad than the bug it worked around?

@timothyqiu
Copy link
Member Author

timothyqiu commented Apr 18, 2024

Would it make sense to do the revert part only for 4.1 and 4.2, while the rest of the more involved fix gets assessed for 4.3?

Ah yeah, the bug it worked around only exists if you make a tree too narrow, adjusting window size solves the problem. But the regression it caused has no workaround, many tree buttons in the editor lost their tooltips.

@akien-mga
Copy link
Member

Could you make a revert PR for 4.2 with cherrypick:4.1 explaining the rationale?

@akien-mga
Copy link
Member

akien-mga commented Apr 18, 2024

Or well actually, maybe do a revert PR for master to merge first and cherry-pick (reopening the original issue), and then rebase this one on top.

@timothyqiu
Copy link
Member Author

OK, done. #90842.

I'll rebase this one after it's merged.

- Fix incorrect tooltip and `get_button_id_at_position()` when column
  title is visible and when RTL layout is used
- Take `button_margin`, `h_separation`, and `item_margin` into account
@akien-mga akien-mga merged commit 9498753 into godotengine:master Apr 22, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

@timothyqiu timothyqiu deleted the tree-button branch April 22, 2024 11:09
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.

Editor : Scene Tree icons' tooltips dont match icon when narrow
3 participants