Skip to content

Fix some inspector action buttons not updating icon when theme changes #107436

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

timothyqiu
Copy link
Member

This PR adds a EditorInspectorActionButton class to replace create_inspector_action_button(). Thus action button users don't have to add icon updating logic on-site.

@timothyqiu timothyqiu requested review from a team as code owners June 12, 2025 03:11
@lodetrick
Copy link
Contributor

If the point of this is to update the icon automatically, it could make sense to have the p_text argument be optional so you can create a button with just an icon.

@timothyqiu
Copy link
Member Author

@lodetrick There is no such use case of inspector action button in the editor codebase.

@AThousandShips AThousandShips added this to the 4.5 milestone Jun 12, 2025
@akien-mga akien-mga requested a review from KoBeWi June 12, 2025 09:51
@timothyqiu timothyqiu force-pushed the action-button-icon branch from 38831c7 to 2cd3957 Compare June 25, 2025 06:56
@timothyqiu
Copy link
Member Author

Updated to use TTRC() for button text.

The only exception is this one, I did not add NOTIFICATION_TRANSLATION_CHANGED because usually auto-translation won't take place for this property editor.

if (script_editor) {
// This property editor is currently only used inside the font import settings dialog.
// Usually, the dialog needs to be closed in order to change the editor's language.
// So we can ignore the auto-translation here.
// TRANSLATORS: Script refers to a writing system.
button_add = memnew(EditorInspectorActionButton(TTR("Add Script", "Locale"), SNAME("Add")));
button_add->set_auto_translate_mode(AUTO_TRANSLATE_MODE_DISABLED);
} else {

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.

4 participants