Common panels #5659
Common panels #5659
Conversation
Merged the sides. Merged the panel options. Started panel shows
…move TopBottomSide NOTE: Non-working commit
…ethods NOTE: Non-working commit
NOTE: Non-working commit
following those steps:
1. Rename `egui::SidePanel` with `egui::Panel`.
2. Rename `width` with `size` in all chained methods.
- `default_width()` to `default_size()`
- `min_width()` to `min_size()`
- `max_width()` to `max_size()`
- `width_range()` to `size_range()`
- `exact_width()` to `exact_size()`
if using `Panel::new()`, replace
- `Side::Left` with `Side::Vertical(VerticalSide::Left)`
- `Side::Right` with `Side::Vertical(VerticalSide::Right)`
NOTE: Non-working commit
…pBottomPanel when both SidePanel and TopBottomPanel were mentioned.
following those steps:
1. Rename `egui::TopBottomPanel` with `egui::Panel`.
2. Rename `height` with `size` in all chained methods.
- `default_height()` to `default_size()`
- `min_height()` to `min_size()`
- `max_height()` to `max_size()`
- `height_range()` to `size_range()`
- `exact_height()` to `exact_size()`
if using `Panel::new()`, replace
- `TopBottomSide::Top` with `Side::Horizontal(HorizontalSide::Top)`
- `TopBottomSide::Bottom` with `Side::Horizontal(HorizontalSide::Bottom)`
NOTE: Non-working commit
- replace switch fn with closures in some places. - integrate the switch fn inside the main switch. - make sure lifetime are defined for PanelSizer. - remove some `mut`, `&`, and `*` where they superfluous. - wrongly called `ctx()` when it was a variable 🤦. - cannot use `impl Trait` in a variable declaration type.
|
Locally, all the tests passed (except those regarding |
| @@ -166,45 +361,47 @@ impl SidePanel { | |||
| self | |||
| } | |||
|
|
|||
| /// The initial wrapping width of the [`SidePanel`], including margins. | |||
| /// The initial wrapping width of the [`Panel`], including margins. | |||
There was a problem hiding this comment.
Width is wrong now, and it'd be nice if the comment could explain in which cases this is width and in which this is height
There was a problem hiding this comment.
I did miss that one!
I did explained the cases at the attribute default_size at line 275. It might be a bit too deep and hidden though in the docs. I'll check to add some more info in the overal containers doc.
| let get_ui_kind = || match side { | ||
| PanelSide::Vertical(v_side) => match v_side { | ||
| VerticalSide::Left => UiKind::LeftPanel, | ||
| VerticalSide::Right => UiKind::RightPanel, | ||
| }, | ||
| PanelSide::Horizontal(h_side) => match h_side { | ||
| HorizontalSide::Top => UiKind::TopPanel, | ||
| HorizontalSide::Bottom => UiKind::BottomPanel, | ||
| }, | ||
| }; |
There was a problem hiding this comment.
Is there a reason this is a closure?
| let get_ui_kind = || match side { | |
| PanelSide::Vertical(v_side) => match v_side { | |
| VerticalSide::Left => UiKind::LeftPanel, | |
| VerticalSide::Right => UiKind::RightPanel, | |
| }, | |
| PanelSide::Horizontal(h_side) => match h_side { | |
| HorizontalSide::Top => UiKind::TopPanel, | |
| HorizontalSide::Bottom => UiKind::BottomPanel, | |
| }, | |
| }; | |
| let ui_kind = match side { | |
| PanelSide::Vertical(v_side) => match v_side { | |
| VerticalSide::Left => UiKind::LeftPanel, | |
| VerticalSide::Right => UiKind::RightPanel, | |
| }, | |
| PanelSide::Horizontal(h_side) => match h_side { | |
| HorizontalSide::Top => UiKind::TopPanel, | |
| HorizontalSide::Bottom => UiKind::BottomPanel, | |
| }, | |
| }; |
There was a problem hiding this comment.
Not that I can think of. Like above, being totaly new to Rust, I may have overthinked some stuff here and there (although I see that I did put the two indent of match together and not as an external closure 😅, I guess I was being more confortable once I got here!).
Also, I did use RustRover refactoring to extract as a variable in many places and sometime it used a closure, sometime not. But I am not knowledgeable enough in Rust to provide concrete and technical reasoning being those choices.
I'll look into it with all the others.
| let opposite_vertical = |side: VerticalSide| -> PanelSide { | ||
| match side { | ||
| VerticalSide::Left => PanelSide::Vertical(VerticalSide::Right), | ||
| VerticalSide::Right => PanelSide::Vertical(VerticalSide::Left), | ||
| } | ||
| }; | ||
|
|
||
| let opposite_horizontal = |side: HorizontalSide| -> PanelSide { | ||
| match side { | ||
| HorizontalSide::Top => PanelSide::Horizontal(HorizontalSide::Bottom), | ||
| HorizontalSide::Bottom => PanelSide::Horizontal(HorizontalSide::Top), | ||
| } | ||
| }; | ||
|
|
There was a problem hiding this comment.
Why the closures? For readability? I'd just match both enums in the match, e.g. PanelSide::Horizontal(HorizontalSide::Top) => PanelSide::Horizontal(HorizontalSide::Bottom)
There was a problem hiding this comment.
At first I did as such (see below), but I had some issues (which were not related to have everything together. Being my first time touching Rust (in fact, first time on a low level language altogether), I segregated stuff to the smallest unit for my own thinking and readability.
And it is also my personal preference to (try to) keep only 1 or 2 indent level.
Same logic is true for all methods and function throughout my PR.
Let me know if you'd prefer I bring back the closure into a "single" operation.
match self {
PanelSide::Vertical(side) => match side {
VerticalSide::Left => PanelSide::Vertical(VerticalSide::Right),
VerticalSide::Right => PanelSide::Vertical(VerticalSide::Left),
},
PanelSide::Horizontal(side) => match side {
HorizontalSide::Top => PanelSide::Horizontal(HorizontalSide::Bottom),
HorizontalSide::Bottom => PanelSide::Horizontal(HorizontalSide::Top),
},
}
|
Preview available at https://egui-pr-preview.github.io/pr/5659-common-panels |
Paneltrait forSidePanelandTopBottomPanel#5643