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.

Sign up for GitHub

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

Jump to bottom

Fix EditorProperty spacing #93089

Merged
merged 1 commit into from
Jun 14, 2024
Merged

Fix EditorProperty spacing #93089

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 23 additions & 12 deletions editor/editor_inspector.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,17 +88,17 @@ Size2 EditorProperty::get_minimum_size() const {

if (keying) {
Ref<Texture2D> key = get_editor_theme_icon(SNAME("Key"));
ms.width += key->get_width() + get_theme_constant(SNAME("hseparator"), SNAME("Tree"));
ms.width += key->get_width() + get_theme_constant(SNAME("h_separation"), SNAME("Tree"));
Copy link
Member

Choose a reason for hiding this comment

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

That would be for a future PR, but we should probably move these and other theme constants to a ThemeCache struct like we have in some other editor controls (e.g. EditorFileDialog).

}

if (deletable) {
Ref<Texture2D> key = get_editor_theme_icon(SNAME("Close"));
ms.width += key->get_width() + get_theme_constant(SNAME("hseparator"), SNAME("Tree"));
ms.width += key->get_width() + get_theme_constant(SNAME("h_separation"), SNAME("Tree"));
}

if (checkable) {
Ref<Texture2D> check = get_theme_icon(SNAME("checked"), SNAME("CheckBox"));
ms.width += check->get_width() + get_theme_constant(SNAME("h_separation"), SNAME("CheckBox")) + get_theme_constant(SNAME("hseparator"), SNAME("Tree"));
ms.width += check->get_width() + get_theme_constant(SNAME("h_separation"), SNAME("Tree"));
}

if (bottom_editor != nullptr && bottom_editor->is_visible()) {
Expand Down Expand Up @@ -179,9 +179,9 @@ void EditorProperty::_notification(int p_what) {
key = get_editor_theme_icon(SNAME("Key"));
}

rect.size.x -= key->get_width() + get_theme_constant(SNAME("hseparator"), SNAME("Tree"));
rect.size.x -= key->get_width() + get_theme_constant(SNAME("h_separation"), SNAME("Tree"));
if (is_layout_rtl()) {
rect.position.x += key->get_width() + get_theme_constant(SNAME("hseparator"), SNAME("Tree"));
rect.position.x += key->get_width() + get_theme_constant(SNAME("h_separation"), SNAME("Tree"));
}

if (no_children) {
Expand All @@ -194,16 +194,27 @@ void EditorProperty::_notification(int p_what) {

close = get_editor_theme_icon(SNAME("Close"));

rect.size.x -= close->get_width() + get_theme_constant(SNAME("hseparator"), SNAME("Tree"));
rect.size.x -= close->get_width() + get_theme_constant(SNAME("h_separation"), SNAME("Tree"));

if (is_layout_rtl()) {
rect.position.x += close->get_width() + get_theme_constant(SNAME("hseparator"), SNAME("Tree"));
rect.position.x += close->get_width() + get_theme_constant(SNAME("h_separation"), SNAME("Tree"));
}

if (no_children) {
text_size -= close->get_width() + 4 * EDSCALE;
}
}

// Account for the space needed on the outer side
// when any of the icons are visible.
if (keying || deletable) {
int separation = get_theme_constant(SNAME("h_separation"), SNAME("Tree"));
rect.size.x -= separation;

if (is_layout_rtl()) {
rect.position.x += separation;
}
}
}

//set children
Expand Down Expand Up @@ -291,7 +302,7 @@ void EditorProperty::_notification(int p_what) {
} else {
draw_texture(checkbox, check_rect.position, color2);
}
int check_ofs = get_theme_constant(SNAME("hseparator"), SNAME("Tree")) + checkbox->get_width() + get_theme_constant(SNAME("h_separation"), SNAME("CheckBox"));
int check_ofs = checkbox->get_width() + get_theme_constant(SNAME("h_separation"), SNAME("Tree"));
Copy link
Member

Choose a reason for hiding this comment

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

Is the removal of the CheckBox separation intentional?

I suppose this checkbox is used for stuff like Theme Overrides in control nodes:

image

Copy link
Contributor Author

@passivestar passivestar Jun 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Just seems excessive in the context of EditorProperty because it's the only button in EditorProperty that you can independently control the "padding" of (i.e why only checkbox and not the revert button, key button, etc?), and also the way it was implemented is it was adding two constants together which I think makes theming less predictable

If we think that the flexibility of specifying the checkbox spacing independently from EditorProperty "global" horizontal spacing is important I'd say it would probably make more sense to only take the checkbox h_separation into the account there

ofs += check_ofs;
text_limit -= check_ofs;
} else {
Expand All @@ -300,7 +311,7 @@ void EditorProperty::_notification(int p_what) {

if (can_revert && !is_read_only()) {
Ref<Texture2D> reload_icon = get_editor_theme_icon(SNAME("ReloadSmall"));
text_limit -= reload_icon->get_width() + get_theme_constant(SNAME("hseparator"), SNAME("Tree")) * 2;
text_limit -= reload_icon->get_width() + get_theme_constant(SNAME("h_separation"), SNAME("Tree"));
revert_rect = Rect2(ofs + text_limit, (size.height - reload_icon->get_height()) / 2, reload_icon->get_width(), reload_icon->get_height());

Color color2(1, 1, 1);
Expand All @@ -320,7 +331,7 @@ void EditorProperty::_notification(int p_what) {

if (!pin_hidden && pinned) {
Ref<Texture2D> pinned_icon = get_editor_theme_icon(SNAME("Pin"));
int margin_w = get_theme_constant(SNAME("hseparator"), SNAME("Tree")) * 2;
int margin_w = get_theme_constant(SNAME("h_separation"), SNAME("Tree"));
int total_icon_w = margin_w + pinned_icon->get_width();
int text_w = font->get_string_size(label, rtl ? HORIZONTAL_ALIGNMENT_RIGHT : HORIZONTAL_ALIGNMENT_LEFT, text_limit - total_icon_w, font_size).x;
int y = (size.height - pinned_icon->get_height()) / 2;
Expand Down Expand Up @@ -350,7 +361,7 @@ void EditorProperty::_notification(int p_what) {
key = get_editor_theme_icon(SNAME("Key"));
}

ofs -= key->get_width() + get_theme_constant(SNAME("hseparator"), SNAME("Tree"));
ofs -= key->get_width() + get_theme_constant(SNAME("h_separation"), SNAME("Tree"));

Color color2(1, 1, 1);
if (keying_hover) {
Expand All @@ -374,7 +385,7 @@ void EditorProperty::_notification(int p_what) {

close = get_editor_theme_icon(SNAME("Close"));

ofs -= close->get_width() + get_theme_constant(SNAME("hseparator"), SNAME("Tree"));
ofs -= close->get_width() + get_theme_constant(SNAME("h_separation"), SNAME("Tree"));

Color color2(1, 1, 1);
if (delete_hover) {
Expand Down
2 changes: 1 addition & 1 deletion editor/icons/Key.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading