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 ColorPicker position changing on theme update bug #76567

Conversation

TheSecondReal0
Copy link
Contributor

@TheSecondReal0 TheSecondReal0 commented Apr 29, 2023

Fixes #62937 by no longer adding theme_override_constants/margin to side offsets. It didn't seem to be working correctly anyway, only changing position in unexpected ways. It didn't even increase the space a ColorPicker takes up in a container (ex. HBoxContainer). Inside a container, it will still cause position to change, which can lead to the ColorPicker moving outside of the container's rectangle entirely.

Maybe theme_override_constants/margin should be removed entirely, not sure about how compatibility breaking it would be.

@TheSecondReal0 TheSecondReal0 requested a review from a team as a code owner April 29, 2023 00:14
@KoBeWi KoBeWi added this to the 4.1 milestone Apr 29, 2023
@KoBeWi
Copy link
Member

KoBeWi commented Apr 29, 2023

Yeah setting offsets based on theme value is definitely not correct.
A better solution would be changing ColorPicker to inherit MarginContainer and overriding the margins. But that requires some bigger changes.

@KoBeWi KoBeWi modified the milestones: 4.1, 4.2 Jun 20, 2023
@akien-mga
Copy link
Member

Superseded by #78468.

@akien-mga akien-mga closed this Aug 7, 2023
@AThousandShips AThousandShips removed this from the 4.2 milestone Aug 7, 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.

ColorPicker nodes move when a scene is opened
4 participants