Connect Dialog popup always centers after toggling "Advanced" tab

[stephen-berry]

Summary of issue
The "Connect a Signal to a Method" GUI centers upon creation, but any time you move the GUI around and toggle the "Advanced" tab, it re-centers.

Why this is being discussed
This seems counter-intuitive, as an existing GUI should not move unless the user decides to move it. I have a video displaying this in action.

Recording of Issue

popup_center-2024-06-02_08.27.04.mp4

Proposal vs Ticket
I wouldn't classify this as a bug because it is coded to be this way, but I would think it should be an improvement. If people agree that it shouldn't re-center, then an improvement ticket could be made.

Solution/Implementation
In connections_dialog.cpp, we have a call to Window::popup_centered() in both ConnectDialog::popup_dialog() and ConnectDialog::advanced_pressed().

ConnectDialog::popup_dialog() should be the only place the centering should happen, as it is the startup of the GUI. The advanced press method that toggles the Advanced button (imo) has no reason to re-center. If this call was simply removed, it would no longer center, making it a trivial fix.

Note
I may have put this in the wrong spot (Proposal vs discussion) but either way I'm sure I could paste this elsewhere if needed.

[AThousandShips]

I wouldn't classify this as a bug because it is coded to be this way

I don't expect it was intended, can you point to the code where it's done? This sounds like a bug

[stephen-berry]

So this is done under connections_dialog.cpp

Here are the methods I mentioned, starting at line ~650

void ConnectDialog::popup_dialog(const String &p_for_signal) {
	from_signal->set_text(p_for_signal);
	warning_label->add_theme_color_override("font_color", warning_label->get_theme_color(SNAME("warning_color"), EditorStringName(Editor)));
	error_label->add_theme_color_override("font_color", error_label->get_theme_color(SNAME("error_color"), EditorStringName(Editor)));
	filter_nodes->clear();
	if (!advanced->is_pressed()) {
		error_label->set_visible(!_find_first_script(get_tree()->get_edited_scene_root(), get_tree()->get_edited_scene_root()));
	}
	if (first_popup) {
		first_popup = false;
		_advanced_pressed();
	}
	popup_centered();
}

void ConnectDialog::_advanced_pressed() {
	if (advanced->is_pressed()) {
		connect_to_label->set_text(TTR("Connect to Node:"));
		tree->set_connect_to_script_mode(false);
		vbc_right->show();
		error_label->hide();
	} else {
		reset_size();
		connect_to_label->set_text(TTR("Connect to Script:"));
		tree->set_connect_to_script_mode(true);

		vbc_right->hide();
		error_label->set_visible(!_find_first_script(get_tree()->get_edited_scene_root(), get_tree()->get_edited_scene_root()));
	}
	EditorSettings::get_singleton()->set_project_metadata("editor_metadata", "use_advanced_connections", advanced->is_pressed());
	popup_centered();
}

Most important is the last line of each method. _advanced_pressed() is called every time advanced is toggled. popup_dialog() from what I could tell through debugging is only called once the GUI gets opened.