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 hovered tabs be drawn with the unselected's width at minimum #82384

Merged
merged 1 commit into from
Sep 27, 2023

Conversation

YeldhamDev
Copy link
Member

Fixes #81836. Supersedes #82344.

This PR also fixes a small bug I found that makes tabs still be hovered if mouse exits the window.

@YeldhamDev YeldhamDev added bug crash regression topic:gui cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release labels Sep 26, 2023
@YeldhamDev YeldhamDev added this to the 4.2 milestone Sep 26, 2023
@YeldhamDev YeldhamDev requested review from a team as code owners September 26, 2023 17:17
doc/classes/TabBar.xml Outdated Show resolved Hide resolved
Copy link
Contributor

@YuriSizov YuriSizov left a comment

Choose a reason for hiding this comment

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

Looks good to me. I wonder what other UI toolkits do in this case.

There is a small issue with the fact that we still use hover stylebox's margins, so the text alignment shifts. But I think this is fine for now so we fix the crash. Perhaps a bit of refactoring in TabBar in the future would make hover behave more like focus to avoid these issues.
image

@akien-mga akien-mga merged commit 048abcb into godotengine:master Sep 27, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks!

@YeldhamDev YeldhamDev deleted the corner_cases_man_i_swear branch September 27, 2023 13:57
@YeldhamDev
Copy link
Member Author

YeldhamDev commented Sep 27, 2023

@YuriSizov This is less an issue and more of "how it works". The text on tabs aren't drawn centered, they just take into account the content_margin of the styles, so if you have a hover style that doesn't have margins and an unselected one that does, this happens.

@YuriSizov
Copy link
Contributor

@YeldhamDev I know, but if we are enforcing some styling from another stylebox, we should probably do it consistently and apply margins as well. This shouldn't be an issue for most, of course, since making them different size is pretty user-unfriendly in the first place. Which is why I'm only leaving it as a passing comment. But still, keeping some consistency would be nice.

@YeldhamDev
Copy link
Member Author

Remember when I said that _draw_tab() is only aware of the style you give it and nothing more, and that changing it for such a corner case would not be worth the extra complexity? 🙃

@YuriSizov
Copy link
Contributor

Cherry-picked for 4.1.2.

@YuriSizov YuriSizov removed the cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release label Sep 27, 2023
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.

TabBar: mouse events crash when tab size changes depending on hover state
4 participants