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

Introduce viewport widget #114

Closed
wants to merge 1 commit into from
Closed

Conversation

Ralith
Copy link
Collaborator

@Ralith Ralith commented Nov 14, 2023

I opted for concision rather than maintaining the same large family of aliases/etc that seem to be common in other widgets. Unsure if layout is the right way to do this; it feels like it's baking in some assumptions about how the list widget works.

@LPGhatguy
Copy link
Member

I've been mulling this over. I think there are a couple different ways to tackle this widget! This is the equivalent widget taken from our editor's codebase:

use yakui::util::widget_children;
use yakui::widget::Widget;
use yakui::{FlexFit, Response};

#[derive(Debug)]
pub struct Viewport;

impl Viewport {
    pub fn show(self, children: impl FnOnce()) -> Response<ViewportWidget> {
        widget_children(children, self)
    }
}

#[derive(Debug)]
pub struct ViewportWidget;

impl Widget for ViewportWidget {
    type Props = Viewport;
    type Response = ();

    fn new() -> Self {
        ViewportWidget
    }

    fn update(&mut self, _props: Self::Props) -> Self::Response {}

    fn flex(&self) -> (u32, FlexFit) {
        (1, FlexFit::Tight)
    }
}

Our version of the widget accepts children since we have some stuff that floats over the viewport. We use the default layout implementation and instead implement the flex function to grow on the primary axis if we're put into a List widget.

The code that uses the widget and lays out our entire editor looks like this:

let mut outer = List::column();
outer.cross_axis_alignment = CrossAxisAlignment::Stretch;
outer.show(|| {
    top_bar(game);

    let mut inner = List::row();
    inner.cross_axis_alignment = CrossAxisAlignment::Stretch;
    inner.show(|| {
        left_bar(game, systems);

        let res = Viewport.show(|| {
            viewport_bits(game);
        });

        viewport_id = Some(res.id);
        right_bar(game);
    });

    bottom_bar(game);
});

All that said, I wonder if this should be a widget that we ship with yakui. I see now that my viewport widget could be replaced with just the expanded alias of the Flexible widget.

Some of these problems would likely be solved with a more useful and comprehensive example library.

What are your thoughts?

@Ralith
Copy link
Collaborator Author

Ralith commented Nov 19, 2023

This is the equivalent widget taken from our editor's codebase:

When placed in a row(|| { ... }), both your widget and yakui::expanded produce a widget that fills the width (main axis size), but has zero height (cross axis size). That's why I had to add the layout impl. What container are you using yours inside?

Our version of the widget accepts children since we have some stuff that floats over the viewport.

That makes sense and seems valuable.

I wonder if this should be a widget that we ship with yakui. I see now that my viewport widget could be replaced with just the expanded alias of the Flexible widget.

Documenting how to solve this problem with a more generic widget SGTM, if it can actually be done.

@Ralith
Copy link
Collaborator Author

Ralith commented Nov 19, 2023

Oh, I missed CrossAxisAlignment::Stretch. That seems to work nicely with expanded, and on that basis I think this is reduced to a docs problem.

@Ralith Ralith closed this Nov 19, 2023
@Ralith Ralith deleted the viewport branch November 19, 2023 22:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants