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

Allow SplitContainer to have a grab area larger than its visual grabber #65355

Merged

Conversation

groud
Copy link
Member

@groud groud commented Sep 5, 2022

Sometimes in a SplitContainer, you would want a thin visible grabber, or possibly no visible grabber image at all (for a borderless split). But with the current implementation, the grabbing area is dependent on the texture width and the "sep" theme variable, so with no texture and a "sep" of zero, the grabbing area becomes too thin and thus hard/impossible to grab.

This PR allows you, via a theme constant, to have a grabbing area width of at least X pixels. It is implemented by adding an internal Control node on top of the two children nodes.

Here is an example with a borderless editor test:

Peek.05-09-2022.11-47.mp4

Closes godotengine/godot-proposals#3635

@groud groud added this to the 4.0 milestone Sep 5, 2022
@groud groud requested a review from KoBeWi September 5, 2022 10:00
@groud groud requested a review from a team as a code owner September 5, 2022 10:00
@groud groud force-pushed the split_container_min_grab_thickness branch from 2562618 to c779db1 Compare September 5, 2022 10:06
@rburing
Copy link
Member

rburing commented Sep 5, 2022

Does this implement godotengine/godot-proposals#3635? :)

@groud
Copy link
Member Author

groud commented Sep 5, 2022

Does this implement godotengine/godot-proposals#3635? :)

Yeah, I guess it does to some extend. You cannot have the same blue line on the whole width, but the borderless aspect is implemented by this PR, yes.

Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Approved in PR review meeting (pending edit suggested by Yuri).

@groud groud force-pushed the split_container_min_grab_thickness branch from c779db1 to 33e0e1a Compare September 7, 2022 08:15
@akien-mga
Copy link
Member

You need to update the class reference to include the new theme properties (and ideally give them a description).

@groud groud force-pushed the split_container_min_grab_thickness branch from 33e0e1a to 3a5ebb6 Compare September 7, 2022 10:25
@groud groud requested a review from a team as a code owner September 7, 2022 10:25
@groud groud force-pushed the split_container_min_grab_thickness branch from 3a5ebb6 to 4cbcb5a Compare September 7, 2022 10:33
@akien-mga akien-mga merged commit 48705b1 into godotengine:master Sep 7, 2022
@akien-mga
Copy link
Member

Thanks!

@Koyper
Copy link
Contributor

Koyper commented Sep 16, 2022

This is a great improvement. I have a scripted version of this with this same approach, but additionally does everything in https://github.com/godotengine/godot-proposals/issues/3635.

Here is what remains to be added to make this fully implement the issue:

  1. most importantly, the dragger area requires an additional offset so that if the first child control has a scroll bar, the area doesn't cover part or all of the scroll bar - a new int property like grabber_offset is added to the Rect2 position here:
		if (vertical) {
			dragging_area_control->set_rect(Rect2(Point2(0, middle_sep - (dragger_ctrl_size - sep) / 2), Size2(get_size().width, dragger_ctrl_size)));
		} else {
			dragging_area_control->set_rect(Rect2(Point2(middle_sep - (dragger_ctrl_size - sep) / 2, 0), Size2(dragger_ctrl_size, get_size().height)));
		}
  1. emit a new signal dragger_released so that the offset can be saved to a resource and later restored, without having to do it every time dragged emits. This also is very usefull for other post-drag functions.
	if (mb.is_valid()) {
		if (mb->get_button_index() == MouseButton::LEFT) {
			if (mb->is_pressed()) {
				sc->_compute_middle_sep(true);
				dragging = true;
				drag_ofs = sc->split_offset;
				if (sc->vertical) {
					drag_from = get_transform().xform(mb->get_position()).y;
				} else {
					drag_from = get_transform().xform(mb->get_position()).x;
				}
			} else {
				dragging = false;
				queue_redraw();
				sc->emit_signal(SNAME("dragger_released")); // New signal here
			}
		}
	}
  1. Make the SplitContainerDragger an exposed class accessible via SplitContainer.get_dragger_control(). Once you have the Control, you can add a draw_line or Panel with StyleBox to set its appearance, and change it's size if required.

It is essential to be able to set a custom size of the dragger Control in the axis of the split line, i.e. top/bottom "margins", as you can do some nifty GUI tricks with that.

@Koyper
Copy link
Contributor

Koyper commented Sep 16, 2022

Another nice trick you can do if you have the SplitContainerDragger, is add a button that rides on the split line. For example, an arrow icon that moves items between sides of the split.

@Koyper
Copy link
Contributor

Koyper commented Sep 16, 2022

In number 2) above, I am referring to saving/restoring the split_offset...

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.

Modernize the V/H SplitContainer to allow for zero-border resizing
5 participants