Merge pull request #65446 from YuriSizov/dialogs-make-styleboxes-matter

Make `AcceptDialog` and derivatives utilize `StyleBox` fully

5 files changed, 125 insertions, 93 deletions

diff --git a/doc/classes/AcceptDialog.xml b/doc/classes/AcceptDialog.xml
index ee49523c90..b5a1461876 100644
--- a/doc/classes/AcceptDialog.xml
+++ b/doc/classes/AcceptDialog.xml
@@ -99,14 +99,11 @@
 		</signal>
 	</signals>
 	<theme_items>
-		<theme_item name="button_margin" data_type="constant" type="int" default="32">
-			Offset that is applied to the content of the window on the bottom, effectively moving the button row.
-		</theme_item>
-		<theme_item name="margin" data_type="constant" type="int" default="8">
-			Offset that is applied to the content of the window on top, left, and right.
+		<theme_item name="buttons_separation" data_type="constant" type="int" default="10">
+			The size of the vertical space between the dialog's content and the button row.
 		</theme_item>
 		<theme_item name="panel" data_type="style" type="StyleBox">
-			Panel that fills up the background of the window.
+			The panel that fills the background of the window.
 		</theme_item>
 	</theme_items>
 </class>
diff --git a/editor/editor_themes.cpp b/editor/editor_themes.cpp
index 09e02dd6c0..5bf79635b8 100644
--- a/editor/editor_themes.cpp
+++ b/editor/editor_themes.cpp
@@ -1420,6 +1420,7 @@ Ref<Theme> create_editor_theme(const Ref<Theme> p_theme) {
 
 	// AcceptDialog
 	theme->set_stylebox("panel", "AcceptDialog", style_window_title);
+	theme->set_constant("buttons_separation", "AcceptDialog", 8 * EDSCALE);
 
 	// HScrollBar
 	Ref<Texture2D> empty_icon = memnew(ImageTexture);
diff --git a/scene/gui/dialogs.cpp b/scene/gui/dialogs.cpp
index 0a1aeeda71..deac7c96bb 100644
--- a/scene/gui/dialogs.cpp
+++ b/scene/gui/dialogs.cpp
@@ -54,8 +54,7 @@ void AcceptDialog::_update_theme_item_cache() {
 	Window::_update_theme_item_cache();
 
 	theme_cache.panel_style = get_theme_stylebox(SNAME("panel"));
-	theme_cache.margin = get_theme_constant(SNAME("margin"));
-	theme_cache.button_margin = get_theme_constant(SNAME("button_margin"));
+	theme_cache.buttons_separation = get_theme_constant(SNAME("buttons_separation"));
 }
 
 void AcceptDialog::_notification(int p_what) {
@@ -64,6 +63,7 @@ void AcceptDialog::_notification(int p_what) {
 			if (is_visible()) {
 				get_ok_button()->grab_focus();
 				_update_child_rects();
+
 				parent_visible = get_parent_visible_window();
 				if (parent_visible) {
 					parent_visible->connect("focus_entered", callable_mp(this, &AcceptDialog::_parent_focused));
@@ -77,10 +77,12 @@ void AcceptDialog::_notification(int p_what) {
 		} break;
 
 		case NOTIFICATION_THEME_CHANGED: {
-			bg->add_theme_style_override("panel", theme_cache.panel_style);
+			bg_panel->add_theme_style_override("panel", theme_cache.panel_style);
 
-			label->set_begin(Point2(theme_cache.margin, theme_cache.margin));
-			label->set_end(Point2(-theme_cache.margin, -theme_cache.button_margin - 10));
+			child_controls_changed();
+			if (is_visible()) {
+				_update_child_rects();
+			}
 		} break;
 
 		case NOTIFICATION_EXIT_TREE: {
@@ -137,14 +139,16 @@ void AcceptDialog::_cancel_pressed() {
 }
 
 String AcceptDialog::get_text() const {
-	return label->get_text();
+	return message_label->get_text();
 }
 
 void AcceptDialog::set_text(String p_text) {
-	if (label->get_text() == p_text) {
+	if (message_label->get_text() == p_text) {
 		return;
 	}
-	label->set_text(p_text);
+
+	message_label->set_text(p_text);
+
 	child_controls_changed();
 	if (is_visible()) {
 		_update_child_rects();
@@ -168,19 +172,24 @@ bool AcceptDialog::get_close_on_escape() const {
 }
 
 void AcceptDialog::set_autowrap(bool p_autowrap) {
-	label->set_autowrap_mode(p_autowrap ? TextServer::AUTOWRAP_WORD : TextServer::AUTOWRAP_OFF);
+	message_label->set_autowrap_mode(p_autowrap ? TextServer::AUTOWRAP_WORD : TextServer::AUTOWRAP_OFF);
 }
 
 bool AcceptDialog::has_autowrap() {
-	return label->get_autowrap_mode() != TextServer::AUTOWRAP_OFF;
+	return message_label->get_autowrap_mode() != TextServer::AUTOWRAP_OFF;
 }
 
 void AcceptDialog::set_ok_button_text(String p_ok_button_text) {
-	ok->set_text(p_ok_button_text);
+	ok_button->set_text(p_ok_button_text);
+
+	child_controls_changed();
+	if (is_visible()) {
+		_update_child_rects();
+	}
 }
 
 String AcceptDialog::get_ok_button_text() const {
-	return ok->get_text();
+	return ok_button->get_text();
 }
 
 void AcceptDialog::register_text_enter(Control *p_line_edit) {
@@ -192,68 +201,79 @@ void AcceptDialog::register_text_enter(Control *p_line_edit) {
 }
 
 void AcceptDialog::_update_child_rects() {
-	Size2 label_size = label->get_minimum_size();
-	if (label->get_text().is_empty()) {
-		label_size.height = 0;
-	}
-
 	Size2 size = get_size();
-	Size2 hminsize = hbc->get_combined_minimum_size();
+	float h_margins = theme_cache.panel_style->get_margin(SIDE_LEFT) + theme_cache.panel_style->get_margin(SIDE_RIGHT);
+	float v_margins = theme_cache.panel_style->get_margin(SIDE_TOP) + theme_cache.panel_style->get_margin(SIDE_BOTTOM);
+
+	// Fill the entire size of the window with the background.
+	bg_panel->set_position(Point2());
+	bg_panel->set_size(size);
 
-	Vector2 cpos(theme_cache.margin, theme_cache.margin + label_size.height);
-	Vector2 csize(size.x - theme_cache.margin * 2, size.y - theme_cache.margin * 3 - hminsize.y - label_size.height);
+	// Place the buttons from the bottom edge to their minimum required size.
+	Size2 buttons_minsize = buttons_hbox->get_combined_minimum_size();
+	Size2 buttons_size = Size2(size.x - h_margins, buttons_minsize.y);
+	Point2 buttons_position = Point2(theme_cache.panel_style->get_margin(SIDE_LEFT), size.y - theme_cache.panel_style->get_margin(SIDE_BOTTOM) - buttons_size.y);
+	buttons_hbox->set_position(buttons_position);
+	buttons_hbox->set_size(buttons_size);
+
+	// Place the content from the top to fill the rest of the space (minus the separation).
+	Point2 content_position = Point2(theme_cache.panel_style->get_margin(SIDE_LEFT), theme_cache.panel_style->get_margin(SIDE_TOP));
+	Size2 content_size = Size2(size.x - h_margins, size.y - v_margins - buttons_size.y - theme_cache.buttons_separation);
 
 	for (int i = 0; i < get_child_count(); i++) {
 		Control *c = Object::cast_to<Control>(get_child(i));
 		if (!c) {
 			continue;
 		}
-
-		if (c == hbc || c == label || c == bg || c->is_set_as_top_level()) {
+		if (c == buttons_hbox || c == bg_panel || c->is_set_as_top_level()) {
 			continue;
 		}
 
-		c->set_position(cpos);
-		c->set_size(csize);
+		c->set_position(content_position);
+		c->set_size(content_size);
 	}
-
-	cpos.y += csize.y + theme_cache.margin;
-	csize.y = hminsize.y;
-
-	hbc->set_position(cpos);
-	hbc->set_size(csize);
-
-	bg->set_position(Point2());
-	bg->set_size(size);
 }
 
 Size2 AcceptDialog::_get_contents_minimum_size() const {
-	Size2 minsize = label->get_combined_minimum_size();
-
+	// First, we then iterate over the label and any other custom controls
+	// to try and find the size that encompasses all content.
+	Size2 content_minsize;
 	for (int i = 0; i < get_child_count(); i++) {
 		Control *c = Object::cast_to<Control>(get_child(i));
 		if (!c) {
 			continue;
 		}
 
-		if (c == hbc || c == label || c->is_set_as_top_level()) {
+		// Buttons will be included afterwards.
+		// The panel only displays the stylebox and doesn't contribute to the size.
+		if (c == buttons_hbox || c == bg_panel || c->is_set_as_top_level()) {
 			continue;
 		}
 
-		Size2 cminsize = c->get_combined_minimum_size();
-		minsize.x = MAX(cminsize.x, minsize.x);
-		minsize.y = MAX(cminsize.y, minsize.y);
+		Size2 child_minsize = c->get_combined_minimum_size();
+		content_minsize.x = MAX(child_minsize.x, content_minsize.x);
+		content_minsize.y = MAX(child_minsize.y, content_minsize.y);
+	}
+
+	// Then we take the background panel as it provides the offsets,
+	// which are always added to the minimum size.
+	if (theme_cache.panel_style.is_valid()) {
+		content_minsize += theme_cache.panel_style->get_minimum_size();
 	}
 
-	Size2 hminsize = hbc->get_combined_minimum_size();
-	minsize.x = MAX(hminsize.x, minsize.x);
-	minsize.y += hminsize.y;
-	minsize.x += theme_cache.margin * 2;
-	minsize.y += theme_cache.margin * 3; //one as separation between hbc and child
+	// Then we add buttons. Horizontally we're interested in whichever
+	// value is the biggest. Vertically buttons add to the overall size.
+	Size2 buttons_minsize = buttons_hbox->get_combined_minimum_size();
+	content_minsize.x = MAX(buttons_minsize.x, content_minsize.x);
+	content_minsize.y += buttons_minsize.y;
+	// Plus there is a separation size added on top.
+	content_minsize.y += theme_cache.buttons_separation;
 
-	Size2 wmsize = get_min_size();
-	minsize.x = MAX(wmsize.x, minsize.x);
-	return minsize;
+	// Last, we make sure that we aren't below the minimum window size.
+	Size2 window_minsize = get_min_size();
+	content_minsize.x = MAX(window_minsize.x, content_minsize.x);
+	content_minsize.y = MAX(window_minsize.y, content_minsize.y);
+	return content_minsize;
 }
 
 void AcceptDialog::_custom_action(const String &p_action) {
@@ -264,13 +284,19 @@ void AcceptDialog::_custom_action(const String &p_action) {
 Button *AcceptDialog::add_button(const String &p_text, bool p_right, const String &p_action) {
 	Button *button = memnew(Button);
 	button->set_text(p_text);
+
 	if (p_right) {
-		hbc->add_child(button);
-		hbc->add_spacer();
+		buttons_hbox->add_child(button);
+		buttons_hbox->add_spacer();
 	} else {
-		hbc->add_child(button);
-		hbc->move_child(button, 0);
-		hbc->add_spacer(true);
+		buttons_hbox->add_child(button);
+		buttons_hbox->move_child(button, 0);
+		buttons_hbox->add_spacer(true);
+	}
+
+	child_controls_changed();
+	if (is_visible()) {
+		_update_child_rects();
 	}
 
 	if (!p_action.is_empty()) {
@@ -285,24 +311,19 @@ Button *AcceptDialog::add_cancel_button(const String &p_cancel) {
 	if (p_cancel.is_empty()) {
 		c = "Cancel";
 	}
+
 	Button *b = swap_cancel_ok ? add_button(c, true) : add_button(c);
+
 	b->connect("pressed", callable_mp(this, &AcceptDialog::_cancel_pressed));
+
 	return b;
 }
 
 void AcceptDialog::remove_button(Control *p_button) {
 	Button *button = Object::cast_to<Button>(p_button);
 	ERR_FAIL_NULL(button);
-	ERR_FAIL_COND_MSG(button->get_parent() != hbc, vformat("Cannot remove button %s as it does not belong to this dialog.", button->get_name()));
-	ERR_FAIL_COND_MSG(button == ok, "Cannot remove dialog's OK button.");
-
-	Node *right_spacer = hbc->get_child(button->get_index() + 1);
-	// Should always be valid but let's avoid crashing
-	if (right_spacer) {
-		hbc->remove_child(right_spacer);
-		memdelete(right_spacer);
-	}
-	hbc->remove_child(button);
+	ERR_FAIL_COND_MSG(button->get_parent() != buttons_hbox, vformat("Cannot remove button %s as it does not belong to this dialog.", button->get_name()));
+	ERR_FAIL_COND_MSG(button == ok_button, "Cannot remove dialog's OK button.");
 
 	if (button->is_connected("pressed", callable_mp(this, &AcceptDialog::_custom_action))) {
 		button->disconnect("pressed", callable_mp(this, &AcceptDialog::_custom_action));
@@ -310,6 +331,19 @@ void AcceptDialog::remove_button(Control *p_button) {
 	if (button->is_connected("pressed", callable_mp(this, &AcceptDialog::_cancel_pressed))) {
 		button->disconnect("pressed", callable_mp(this, &AcceptDialog::_cancel_pressed));
 	}
+
+	Node *right_spacer = buttons_hbox->get_child(button->get_index() + 1);
+	// Should always be valid but let's avoid crashing.
+	if (right_spacer) {
+		buttons_hbox->remove_child(right_spacer);
+		memdelete(right_spacer);
+	}
+	buttons_hbox->remove_child(button);
+
+	child_controls_changed();
+	if (is_visible()) {
+		_update_child_rects();
+	}
 }
 
 void AcceptDialog::_bind_methods() {
@@ -355,25 +389,25 @@ AcceptDialog::AcceptDialog() {
 	set_exclusive(true);
 	set_clamp_to_embedder(true);
 
-	bg = memnew(Panel);
-	add_child(bg, false, INTERNAL_MODE_FRONT);
+	bg_panel = memnew(Panel);
+	add_child(bg_panel, false, INTERNAL_MODE_FRONT);
 
-	hbc = memnew(HBoxContainer);
+	buttons_hbox = memnew(HBoxContainer);
 
-	label = memnew(Label);
-	label->set_anchor(SIDE_RIGHT, Control::ANCHOR_END);
-	label->set_anchor(SIDE_BOTTOM, Control::ANCHOR_END);
-	add_child(label, false, INTERNAL_MODE_FRONT);
+	message_label = memnew(Label);
+	message_label->set_anchor(SIDE_RIGHT, Control::ANCHOR_END);
+	message_label->set_anchor(SIDE_BOTTOM, Control::ANCHOR_END);
+	add_child(message_label, false, INTERNAL_MODE_FRONT);
 
-	add_child(hbc, false, INTERNAL_MODE_FRONT);
+	add_child(buttons_hbox, false, INTERNAL_MODE_FRONT);
 
-	hbc->add_spacer();
-	ok = memnew(Button);
-	ok->set_text("OK");
-	hbc->add_child(ok);
-	hbc->add_spacer();
+	buttons_hbox->add_spacer();
+	ok_button = memnew(Button);
+	ok_button->set_text("OK");
+	buttons_hbox->add_child(ok_button);
+	buttons_hbox->add_spacer();
 
-	ok->connect("pressed", callable_mp(this, &AcceptDialog::_ok_pressed));
+	ok_button->connect("pressed", callable_mp(this, &AcceptDialog::_ok_pressed));
 
 	set_title(TTRC("Alert!"));
 
diff --git a/scene/gui/dialogs.h b/scene/gui/dialogs.h
index 8ba9c93861..81e82d851e 100644
--- a/scene/gui/dialogs.h
+++ b/scene/gui/dialogs.h
@@ -45,17 +45,18 @@ class AcceptDialog : public Window {
 	GDCLASS(AcceptDialog, Window);
 
 	Window *parent_visible = nullptr;
-	Panel *bg = nullptr;
-	HBoxContainer *hbc = nullptr;
-	Label *label = nullptr;
-	Button *ok = nullptr;
+
+	Panel *bg_panel = nullptr;
+	Label *message_label = nullptr;
+	HBoxContainer *buttons_hbox = nullptr;
+	Button *ok_button = nullptr;
+
 	bool hide_on_ok = true;
 	bool close_on_escape = true;
 
 	struct ThemeCache {
 		Ref<StyleBox> panel_style;
-		int margin = 0;
-		int button_margin = 0;
+		int buttons_separation = 0;
 	} theme_cache;
 
 	void _custom_action(const String &p_action);
@@ -82,12 +83,12 @@ protected:
 	void _cancel_pressed();
 
 public:
-	Label *get_label() { return label; }
+	Label *get_label() { return message_label; }
 	static void set_swap_cancel_ok(bool p_swap);
 
 	void register_text_enter(Control *p_line_edit);
 
-	Button *get_ok_button() { return ok; }
+	Button *get_ok_button() { return ok_button; }
 	Button *add_button(const String &p_text, bool p_right = false, const String &p_action = "");
 	Button *add_cancel_button(const String &p_cancel = "");
 	void remove_button(Control *p_button);
diff --git a/scene/resources/default_theme/default_theme.cpp b/scene/resources/default_theme/default_theme.cpp
index ea77a86f2b..2de4bfa90b 100644
--- a/scene/resources/default_theme/default_theme.cpp
+++ b/scene/resources/default_theme/default_theme.cpp
@@ -609,9 +609,8 @@ void fill_default_theme(Ref<Theme> &theme, const Ref<Font> &default_font, const
 	// Dialogs
 
 	// AcceptDialog is currently the base dialog, so this defines styles for all extending nodes.
-	theme->set_constant("margin", "AcceptDialog", 8 * scale);
-	theme->set_constant("button_margin", "AcceptDialog", 32 * scale);
-	theme->set_stylebox("panel", "AcceptDialog", make_flat_stylebox(style_popup_color, 0, 0, 0, 0));
+	theme->set_stylebox("panel", "AcceptDialog", make_flat_stylebox(style_popup_color, 8 * scale, 8 * scale, 8 * scale, 8 * scale));
+	theme->set_constant("buttons_separation", "AcceptDialog", 10 * scale);
 
 	// File Dialog