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

[Editor] Update SceneUniqueName button text for clarity #98745

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

Conversation

yahkr
Copy link
Contributor

@yahkr yahkr commented Nov 2, 2024

  • Update on hover message for clarity.
  • Remove "Click to disable this." when the button is disabled.

Enabled
image
Disabled
image

@yahkr yahkr requested review from a team as code owners November 2, 2024 00:19
@timothyqiu
Copy link
Member

I think the same button showing different hint text depending on the context might be unexpected by many users. It could be harmonized into more precise language, for example:

This node can be accessed from within anywhere in its owning scene by preceding it with the '%s' prefix in a node path.

@yahkr
Copy link
Contributor Author

yahkr commented Nov 2, 2024

I think the same button showing different hint text depending on the context might be unexpected by many users. It could be harmonized into more precise language, for example:

This node can be accessed from within anywhere in its owning scene by preceding it with the '%s' prefix in a node path.

I do like this combined language, however I fear users may not understand what ownership really is, but maybe that's not the problem to try and solve.

Thoughts on?

This node can be accessed from anywhere within the scene it belongs to by using the '%' prefix in the node path.

I would also be just as happy if the message was the same and we at least removed the "Click to disable this" when disabled as it's not immediately clear why you can't click it.

@AdriaandeJongh
Copy link
Contributor

I think the tooltip is a great place to mention what pressing the button does, because it isn’t really explained anywhere else what it does.

@@ -344,7 +344,9 @@ void SceneTreeEditor::_add_nodes(Node *p_node, TreeItem *p_parent) {
}

if (p_node->is_unique_name_in_owner()) {
item->add_button(0, get_editor_theme_icon(SNAME("SceneUniqueName")), BUTTON_UNIQUE, p_node->get_owner() != EditorNode::get_singleton()->get_edited_scene(), vformat(TTR("This node can be accessed from within anywhere in the scene by preceding it with the '%s' prefix in a node path.\nClick to disable this."), UNIQUE_NODE_PREFIX));
bool is_enabled = p_node->get_owner() == EditorNode::get_singleton()->get_edited_scene();
String button_text = vformat(TTR("This node can be accessed from anywhere within the scene it belongs to by using the '%s' prefix in the node path%s."), UNIQUE_NODE_PREFIX, is_enabled ? "\nClick to disable this." : "");
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
String button_text = vformat(TTR("This node can be accessed from anywhere within the scene it belongs to by using the '%s' prefix in the node path%s."), UNIQUE_NODE_PREFIX, is_enabled ? "\nClick to disable this." : "");
String button_text = vformat(TTR("This node can be accessed from anywhere within the scene it belongs to by using the '%s' prefix in the node path.%s"), UNIQUE_NODE_PREFIX, is_enabled ? ("\n" + TTR("Click to disable this.")) : "");

The additional text needs to be translated, and the period should still be at the end of the first sentence

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