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 placement of ColorPicker in the editor #91757

Merged
merged 1 commit into from
May 10, 2024

Conversation

Chaosus
Copy link
Member

@Chaosus Chaosus commented May 9, 2024

My attempt to Fix #91722

@Chaosus Chaosus requested a review from a team as a code owner May 9, 2024 11:50
@Chaosus Chaosus added this to the 4.3 milestone May 9, 2024
@KoBeWi
Copy link
Member

KoBeWi commented May 9, 2024

I'm looking into that right now and BoxContainer was already using is_visible_in_tree(), so I don't get why the new method wouldn't work.

@Chaosus
Copy link
Member Author

Chaosus commented May 9, 2024

@KoBeWi Ok, if you find the right solution, feel free to close this PR.

@KoBeWi
Copy link
Member

KoBeWi commented May 9, 2024

Ok I missed that BoxContainer used is_visible() for minimum size. And apparently you need to revert both containers to fix the issue.

Aims to solve similar issue for SplitContainer, to allow using is_visible_in_tree() for minimum size. @kitbdev do you think this can be applied to all containers?

Alternate solution is either introducing a separate method or adding argument to as_sortable_control(), with intention to be used for minimum size. The argument could be an enum actually, to handle containers that ignore visibility 🤔

For now this PR is fine though.

scene/gui/box_container.cpp Outdated Show resolved Hide resolved
@Chaosus Chaosus force-pushed the fix_color_picker_placement branch from dd5f17f to 1fd1adc Compare May 9, 2024 12:16
@kitbdev
Copy link
Contributor

kitbdev commented May 9, 2024

I think we should make as_sortable_control use is_visible so it works when the Container is not visible. This way all Containers can calculate minimum size at any time.
To stop Containers from sorting while it is not visible in tree, we can just add a guard for each one when it sorts children.

@akien-mga akien-mga merged commit a294825 into godotengine:master May 10, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

@Chaosus Chaosus deleted the fix_color_picker_placement branch May 10, 2024 08:04
@KoBeWi
Copy link
Member

KoBeWi commented May 10, 2024

I think we should make as_sortable_control use is_visible so it works when the Container is not visible.

I tried that originally and it crashed the editor on startup.

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.

Opening a Color picker in the inspector places it outside the display border
4 participants