added splitter #1028

LordMZTE · 2022-01-01T16:46:39Z

Closes #944.

The splitter currently still has some issues, which I'm not quite sure how to fix:

Elements can overlap when the splitte 8000 r is made too small
The splitter only allocates the current available size, and not the size the containing widgets actually need.

Guidance would be much appreciated!

parasyte · 2022-01-03T12:48:03Z

This is awesome. Just tried the demo and I have a few suggestions:

Pass two closures to the show method, instead of one closure that takes two Uis. Subjective, but this just feels more natural. This is also what was requested in Horizontal and vertical splitter #944.

Splitter::with_orientation(self.splitter_orientation)
    .ratio(self.splitter_ratio)
    .show(
        ui,
        |ui| {
            ui.label("Left/Top");
            ui.label("Left/Top");
            ui.label("Left/Top");
        },
        |ui| {
            ui.label("Right/Bottom");
            ui.label("Right/Bottom");
            ui.label("Right/Bottom");
        },
    );

Make the splitter line optionally draggable so it can be moved with the cursor. Like the panel containers (resizable method).
I think it makes sense to use the available width for vertical layouts, and the available height for horizontal layouts; let the caller define the remaining dimension.
- A configurable size would solve the second issue you mention.
It might be worth wrapping both child UIs in ScrollAreas. I tried this, and I really prefer the Splitter to do it instead of writing two nested closures for each side of the split. (Or two scroll areas in a single closure with two Uis.)
- It's hard to consider this and also not want to make these scroll areas configurable. Defaulting to both() with auto-shrink disabled is quite nice, though.
- When I prototyped this, I also had to make the child labels non-wrapping so the scrollbars would show when needed.

LordMZTE · 2022-01-03T13:25:42Z

Thanks! I'm glad you like it!

Pass two closures to the show method, instead of one closure that takes two Uis. Subjective, but this just feels more natural. This is also what was requested in Horizontal and vertical splitter #944.

Well, the reason I originally went with 2 closures is that emilk suggested it as an API in the original issue where this was discussed. I also personally prefer it, as there's no question about which order the closures will be called in and so on.

Make the splitter line optionally draggable so it can be moved with the cursor. Like the panel containers (resizable method).

Sounds like a useful feature, but to be honest I'm kind of scared at the math needed for this. Also I'm not sure how I'd fit it in alongside the current ratio implementation.

I think it makes sense to use the available width for vertical layouts, and the available height for horizontal layouts; let the caller define the remaining dimension.

A configurable size would solve the second issue you mention.

That sounds like a great idea! Although I'm not quite sure if the intended way of doing this kinda thing Isn't just to place the widget inside one that restricts its available space. Also this would make the API a bit less flexible for users who "just want a splitter" and don't really care all that much about precise layout and figuring out dimensions.

It might be worth wrapping both child UIs in ScrollAreas. I tried this, and I really prefer the Splitter to do it instead of writing two nested closures for each side of the split. (Or two scroll areas in a single closure with two Uis.)

It's hard to consider this and also not want to make these scroll areas configurable. Defaulting to both() with auto-shrink disabled is quite nice, though.

When I prototyped this, I also had to make the child labels non-wrapping so the scrollbars would show when needed.

That sounds like it would be a good way to go about fixing the clipping issue. I think that's worth doing, although I should probably add a way for the caller to disable this so they can figure out their own layouts and have more freedom.

BTW if you feel like working on this, you'd be more than welcome! Feel free to just open a PR on my fork on the feature/splitter branch.

emilk · 2022-01-03T20:30:59Z

Pass two closures to the show method, instead of one closure that takes two Uis

This will create borrowing issues, so I don't think it is a good idea.

parasyte · 2022-01-04T01:20:32Z

This will create borrowing issues, so I don't think it is a good idea.

That is a great point! It can be resolved by providing two show methods to split the lifetimes:

Splitter::with_orientation(self.splitter_orientation)
    .ratio(self.splitter_ratio)
    .show_left(ui, |ui| {
        ui.label("Left/Top");
        ui.label("Left/Top");
        ui.label("Left/Top");
    })
    .show_right(ui, |ui| {
        ui.label("Right/Bottom");
        ui.label("Right/Bottom");
        ui.label("Right/Bottom");
    });

It also resolves the other issue about ordering.

In this case, show_left consumes self and returns a type that implements show_right, which then finally returns a response with results from both closures. (Bikeshedding welcome on the names, but this should be acceptable. And IMHO it looks more reasonable than mixing two Ui refs in one closure. The clear delineation is nice.)

parasyte · 2022-01-04T01:38:04Z

Although I'm not quite sure if the intended way of doing this kinda thing Isn't just to place the widget inside one that restricts its available space. Also this would make the API a bit less flexible for users who "just want a splitter" and don't really care all that much about precise layout and figuring out dimensions.

This sounds completely reasonable.

I would want the demo to include such a wrapper (and of course, documentation) so that it is obvious that the expectation is for the user to allocate space in some way. Adding a spitter to some container is a fine way to allocate space without really caring to define dimensions. (This is what the demo currently does in this PR; Window is the container. Except it doesn't gracefully handle the situation where all of the collapsing headers are expanded in a small window.)

LordMZTE · 2022-01-04T11:41:13Z

Well, a big part of the reason I'm using only 1 closure is the response type that Splitter::show returns:

/// The response of showing a Splitter
pub struct SplitterResponse<R> {
    /// The return value of the closure passed into show.
    pub body_returned: R,
    /// The response of the top or left UI depending on the splitter's orientation.
    pub top_left_response: Response,
    /// The response of the bottom or right UI depending on the splitter's orientation.
    pub bottom_right_response: Response,
    /// The response of the whole splitter widget.
    pub splitter_response: Response,
}

With a show method that takes 2 closures, we'd need 2 fields (and 2 generic parameters) for the return type of both the closures. If we had 2 show functions, I'm not sure how we could make them chainable like you suggested, since that would make this response handling pretty much impossible.

parasyte · 2022-01-04T12:57:35Z

Here's a prototype: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=7b292915dc28bd21554bbc7457317545

It only adds minor complexity. But I think the improvement to ergonomics are worth it.

LordMZTE · 2022-01-04T14:27:12Z

I don't really like this. I think this is just more complex than it needs to be. Also this requires calling show_left before calling show_right, and I don't really understand what the point is in not just using one closure. That would allow for more flexibility too (like declaring a variable when creating one side and using it on the other, and letting the user choose which side to paint first).

parasyte · 2022-01-04T15:24:05Z

I don't think any of those are 8000 showstoppers. You can reverse the call order with another intermediate struct. If you want to set a variable in one side and use it in the other, you can just capture it in both closures (like num in the prototype).

Mingun · 2022-01-04T17:43:31Z

That is a great point! It can be resolved by providing two show methods to split the lifetimes:

I like that suggestion! Also, that way you can handle scrollables in a graceful way:

impl Splitter {
  // Only content, without wrapping into a scrollable area
  fn show_first<R>(...) -> R { ... }
  fn show_second<R>(...) -> R { ... }

  // Wraps content into a scrollable area
  fn show_scrollable_first<R>(...) -> R { ... }
  fn show_scrollable_second<R>(...) -> R { ... }
}

Also, I think it is better to use neutral words first/second instead of left/right because they cover also top/bottom variant.

LordMZTE added 2 commits January 1, 2022 17:43

added splitter

ef5f73b

fix splitter doc test

0f62f94

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

added splitter #1028

added splitter #1028

added splitter #1028

Are you sure you want to change the base?

added splitter #1028

Conversation