Druid: Problems with TextBox::fix_width()

[MGlolenstine]

Whenever I change TextBoxs width, the textbox resizes, but debug_paint_layout shows it's CollisionBox still being original size.

I can click into CollisionBox, but I cannot click outside of it.
The layouts work correctly though.

EDIT: Flex still takes the original size and not the "fixed" one.

[Finnerale]

Would be helpful to have a minimal example to reproduce the issue.

[MGlolenstine]

Minimal runnable example

use druid::widget::{Flex, Label, TextBox, Scroll, CrossAxisAlignment, List, ListIter, Align};
use druid::{
    AppLauncher, Data, Lens, LocalizedString, Widget, WidgetExt, WindowDesc, UnitPoint
};
use druid::im::{vector, Vector};

const WINDOW_TITLE: LocalizedString<AppData> = LocalizedString::new("Minimal runnable example");

#[derive(Clone, Data, Lens)]
pub struct AppData {
    text: String,
    values: Vector<Text>,
}

#[derive(Clone, Data, Lens)]
pub struct Text{
    text: String
}

impl ListIter<Text> for AppData{
    fn for_each(&self, mut cb: impl FnMut(&Text, usize)) {
        let mut index = 0 as usize;
        for e in self.values.clone().into_iter(){
            cb(&e, index);
            index+=1;
        }
    }

    fn for_each_mut(&mut self, mut cb: impl FnMut(&mut Text, usize)) {
        let mut index = 0 as usize;
        for e in self.values.iter_mut(){
            cb(e, index);
            index+=1;
        }
    }

    fn data_len(&self) -> usize {
        self.values.len()
    }
}

pub fn main() {
    let main_window = WindowDesc::new(ui_builder)
        .window_size((1270.0, 720.0))
        .title(WINDOW_TITLE);
    let data = AppData {
        text: "".into(),
        values: vector![Text{text: "Hello World!".to_string()}],
    };

    AppLauncher::with_window(main_window)
        .use_simple_logger()
        .launch(data)
        .expect("launch failed");
}

fn ui_builder() -> impl Widget<AppData> {
    let mut root = Flex::column();

    let mut lists = Flex::row().cross_axis_alignment(CrossAxisAlignment::Start);

    lists.add_flex_child(
        Scroll::new(List::new(|| {
            Flex::row()
                .with_flex_spacer(1.0)
                .with_flex_child(Align::new(UnitPoint::CENTER, Label::new(|item: &Text, _env: &_| {format!(r#""{}""#, item.text)})), 1.0)
                .with_flex_spacer(1.0)
                .with_flex_child(Label::new(" "), 0.0)
                .with_flex_spacer(1.0)
                .with_flex_child(
                    Flex::row()
                        .with_child(TextBox::new().fix_width(250.0).lens(Text::text))
                     , 1.0)
                .with_flex_spacer(1.0)
                .padding(10.0)
                .fix_height(50.0)
                .expand_width()
        }))
        .vertical()
    , 1.0);

    root.add_flex_child(lists, 1.0);
    root.debug_paint_layout()
    // root
}

I tried making it as small as possible.
Running MRE in GTK/X11 results in this

[Finnerale]

I think I found the issue. Turns out that Flex does not constrain non-flex children along the main axis, the docs also mention this:

First we measure (calling their layout method) our non-flex children, providing them with unbounded space on the main axis.

But this feels kind of wrong to me, it probably should provide them with "all the remaining space"?

This is what's happening right now:

Flex::row()
    .with_flex_spacer(1.0)
    .with_flex_child(Align::new(UnitPoint::CENTER, Label::new(|item: &Text, _env: &_| {format!(r#""{}""#, item.text)})), 1.0)
    .with_flex_spacer(1.0)
    .with_flex_child( // this imposes a max width, say 100
        Flex::row().with_child( // this does not impose a max width / imposes infinity
            TextBox::new().fix_width(250.0).lens(Text::text) // this wants to be 250 wide and does get "permission"
         ),
         1.0,
    ) // now this gets a child with width 250 despite imposing a max width of 100

[MGlolenstine]

Thanks for investigating! This does indeed solve my problem!

My problem has been solved, but I'm wondering if we should close this issue or just leave it open as a reminder to look over it again and maybe change the way it renders behaves?

[Finnerale]

I believe we should change the behaviour of Flex, because this seems rather unintuitive to me.

[cmyr]

hmm.

I guess the main question for me is: what layout are you trying to achieve? I think that the sample provided is ill-formed; you are giving the child a fixed width, but then asking the flex widget to treat it as if it has a flexible width, but the child is ignoring whatever constraints it is receiving and is just returning its fixed size.

I'm also not sure what the rationale is for wrapping the text widget in an additional flex widget; this also makes things complicated, and is likely to cause layout problems.

It's possible that this is a bug, but I'm not convinced by the provided example; I don't think it describes a valid layout. It would be nice if we were warning about this at some point, though?

In any case, to offer any more insight I would need to know what you desire the layout to look like.

On how we distribute space during flex layout:

Also stepping back a bit: I also think that the current way we distribute space in flex is correct. The idea is that non-flex children have total control over their layout, and it is the responsibility of the user to ensure that the result makes sense.

[MGlolenstine]

Ok, so the layout I was trying to achieve was the whole window split in half and every half having one centred widget.
I wanted to expand the TextBox as much as I could, but the fill_width wasn't doing me any favours, as it was complaining about infinite width and app becoming empty because of it.

[cmyr]

that sounds like it was a non-flex child. This produces the layout you describe:

Flex::row()
    .with_flex_child(
        Label::new(|item: &Text, _env: &_| format!(r#""{}""#, item.text)).center(),
        1.0,
    )
    .with_flex_child(TextBox::new().expand_width().lens(Text::text), 1.0)

Some additional observations from the sample you provided:

• instead of all the flex_spacer, try something like .main_axis_alignment(MainAxisAlignment::SpaceEvenly) (this won't work if you're using flex children, in which case I would suggest just using fixed-width spacers; flex spacers are a weird fit here, since they will be allotted an equal layout size as your flex widgets)
• you shouldn't need .expand_width() if you have a flex child; the flex child will necessarily use all available width, filling the axis. If you don't have a flex child, you should prefer calling must_fill_main_axis on the flex container.

[MGlolenstine]

Oh, those are great ideas!
I'll check them out on Monday when I'm back at work ;)

Thanks for the help!

[cmyr]

This stuff is definitely nuanced and not always intuitive; the Flex widget docs are definitely worth going over, and it's very possible we should have a few more examples in there.

[smmalis37]

Just chiming in that I also hit similar confusion here from just reading the docs. I assumed that Flex::add_child would add a child and flexibly size it, and was very confused when it didn't. I skimmed the docs and saw add_flex_child, but after reading the sentence "This method is used when you need more control over the behaviour of the widget you are adding." stopped, because I don't need any additional control. I just wanted 4 things that were equally sized and took up all available room and assumed that was the default behavior (as it has been in some other ui toolkits i've used). Eventually I figured out that I needed to call add_flex_child(foo, 1.0) for everything and it all worked out.

I think the existence of the two separate methods is confusing here. add_child sounds like it's the default, and given that we are in a Flex I initially expected everything to have a flexible size. It seems both MGlolenstine and I fell into that trap. My suggestion would be to only have a single add_child method, with a second parameter that's an enum of FixedSize and Flex(f64) or something.

[ForLoveOfCats]

I agree that it seems reasonable to assume that add_child is the "default" method to use. I remembered there being an issue with some more context which you can read here.

[cmyr]

I believe that by default, in a row or column, you do not want items to be flex; this was a major cause of pain in earlier revisions. It may be that naming the container Flex is a source of confusion, and we could consider changing that, but I'm not sure how to make the docs much clearer, any suggestions welcome!

[smmalis37]

That's why my suggestion is for there to be no default. Make flex vs non-flex another parameter on a single add_child method. I think that would make it a lot clearer that a flex is just a line of children, but that scaling them is very adjustable.

If sticking with the current design is desired, maybe add more text to the add_child docs to make it clearer that 'this child's size will be constant, no flexing will occur. if you want the size to actually change use add_flex_child' or similar.

[cmyr]

This was true initially; there was a single method, and you passed a flex factor, with 0 for no flex. The result there was that people would consistently pass 1.0 for the flex factor, which also is not what you generally want.

The current API is based heavily on flutter, which defaults to non-flex children and has a separate API for flex children. This was all workshopped heavily in march; the most relevant discussion from then would be #691 #639 and #607.

I'm still not totally happy with this API! It is confusing, but there is also substantial underlying complexity, and I doubt there is a simple API that will work as expected in most cases. I do think that the flex docs are quite good, although I'm very happy to hear suggestions for how things could be clearer.

[smmalis37]

I'm generally against having sentinel values in apis like that for reasons like this. I personally think that if it was switched to a single parameter enum of FixedSize and Flex(f64) there would be a lot less confusion. It would mean users don't need to dig through docs to find an alternate function (there's only one) but it would also force them to state upfront whether something is Flex or not, rather than just using an implicit number that could be overlooked.

[cmyr]

But even in this world they would have to decide which variant of the enum to use in a given circumstance, which means they would still have to know how this works, which means they'd have to go read the docs. This would also require the caller to import another type into scope, which isn't the end of the world but is definitely something I would want to justify.

I think it is going to be difficult for someone to use the flex layout system competently without reading the docs and looking at the examples (the flex example is also something I put a lot of time into). It feels to me like we're trying to find some API that is simple and intuitive and also expresses all of the underlying complexity, and having spent a number of weeks thinking hard about this I'm just not sure there's a solution.

[smmalis37]

I guess the difference between the two approaches in my mind is what the user is forced to deal with, and what they're not. For example, with the current approach:

1. User skims the docs looking for a function named something like add_child.
2. User sees add_child and doesn't read the docs, just calls it.
3. User doesn't get the behavior they want (flexing) and is confused

vs.

1. User skims the docs looking for a function named something like add_child.
2. User sees add_child and doesn't read the docs, just calls it.
3. User is forced to figure out which enum variant gets passed to add_child, and so long as they're named well the user probably still doesn't need to read the docs.

As nice as it might be, there's no way to force developers to read documentation for everything they do (but man, what a better world we'd live in if we did lol). By changing where this decision is encountered from somewhere that can easily be overlooked to somewhere that can't, I'd hope it would make the developer more aware of the choice they have to make.

[cmyr]

It's tricky. Your position is basically "make the API hard to use, forcing the developer to confront the complexity." My position is "make the default choice the one that is most likely to be correct, and move the complexity to a separate method".

There was an intermediate design that was as you describe (with the exception that children can have more than just their flex factor customized; see FlexParams; it's now just flex and alignment, but it used to include a few other things like loose/tightness of flex, and it might grow in the future) (#639) and ultimately it just didn't feel good. This solution felt less bad, but I'm still not especially happy with it; but I'm yet to hear anything that feels clearly better.

In any case, we're of course free to disagree on this! It's a subtle matter of priorities and philosophy, and I can see the value of either position. This API ended up winning out at the moment, and I'm not especially interested in breaking the API again until a clearly better approach surfaces. 🤷