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

[3.x] Improve user communications in the theme editor #51243

Merged
merged 1 commit into from
Aug 4, 2021

Conversation

YuriSizov
Copy link
Contributor

A 3.x version of #51242. Doesn't include the type variations change because we don't have them in 3.x, obviously.

@@ -2888,7 +2925,7 @@ void ThemeEditor::_theme_save_button_cbk(bool p_save_as) {
}

void ThemeEditor::_theme_edit_button_cbk() {
theme_edit_dialog->popup_centered(Size2(850, 760) * EDSCALE);
theme_edit_dialog->popup_centered(Size2(850, 700) * EDSCALE);
Copy link
Member

Choose a reason for hiding this comment

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

Does a 700 pixel-tall popup still fit on a 1366×768 monitor with the Godot editor maximized, but not fullscreen? Assuming it's run on Windows with a non-autohiding task bar and a visible window border, that is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should, I think. The user who reported it not fitting shared a screenshot where it was almost okay (but they also had a weird combination of Windows and Godot Editor scaling, so I'm not entirely sure how representative it is).

Copy link
Member

@Calinou Calinou 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 just have one question about the popup size. Perhaps popup_centered_minsize() could be used instead, or the popup could be made resizable in a future PR.

@YuriSizov
Copy link
Contributor Author

YuriSizov commented Aug 4, 2021

I don't have anything against making it resizable, but it's still a fairly large popup, so just making it resizable wouldn't solve the problem. I think it should be good now, and if indeed someone wants it to be resizable it can be done in a separate PR.

@akien-mga akien-mga merged commit 2269c2e into godotengine:3.x Aug 4, 2021
@akien-mga
Copy link
Member

Thanks!

@YuriSizov YuriSizov deleted the editor-theme-instructions-3.x branch August 4, 2021 16:35
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.

3 participants