Documentation on when to send WidgetAdded events to children

[gmorenz]

When I replace the child of a Controller and call ctx.children_changed() the new child never receives a WidgetAdded event (first version of the code below). If I've understood the code correctly the fix is to listen for InternalLifeCycle::RouteWidgetAdded lifecycle events in the controller and route them to the child (i.e. the modifications in the final code block).

Assuming this is the intended behavior, it should probably be documented somewhere, perhaps in the documentation of Controller. It would also be nice if the error message included this as a potential cause (but I'm not sure if that's possible to fix generally for all widgets). The current error message is as follows:

thread 'main' panicked at 'TextLayout::layout_metrics called without rebuilding layout object. Text was ''', /home/greg/apps/druid/druid/src/text/layout.rs:214:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

If I can generally get a confirmation that the documentation I'm suggesting would be a good idea, and that the updated version of the code is legal and reasonable idiomatic, I'd be happy to submit a pr (or I'd be equally happy if someone else updated it).

A bit of an aside, but #1259 unfortunately does not fix this issue despite being closely related. Running on #1259 merged into master I get the same error message as above. Running on on #1259 directly instead of crashing entirely I get the following warning and no text appearing

WARN  [druid::widget::label] Label text changed without call to update. See LabelAdapter::set_text for information.

Test code:

use druid::*;
use druid::widget::*;

const CHANGE_VIEW: Selector<()> = Selector::new("change_view");

fn controlled_widget() -> ControllerHost<Label<u32>, ViewController> {
    ControllerHost::new(Label::new("Original"), ViewController)
}

struct Delegate;
impl AppDelegate<u32> for Delegate {
    fn event(&mut self, ctx: &mut DelegateCtx, window_id: WindowId, event: Event, _: &mut u32, _: &Env) -> Option<Event> {
        if let Event::KeyDown(_) = event {
            ctx.submit_command(Command::new(CHANGE_VIEW, (), window_id));
            None
        }
        else {
            Some(event)
        }
    }
}

struct ViewController;
impl Controller<u32, Label<u32>> for ViewController {
    fn event(&mut self, child: &mut Label<u32>, ctx: &mut EventCtx, event: &Event, data: &mut u32, env: &Env) {
        if let Event::Command(ref cmd) = *event  {
            if let Some(()) = cmd.get(CHANGE_VIEW) {
                *child =Label::new("New Label");
                ctx.children_changed();
                return;
            }
        }

        child.event(ctx, event, data, env)
    }
}

fn main() -> Result<(), PlatformError> {
    AppLauncher::with_window(WindowDesc::new(controlled_widget))
        .delegate(Delegate)
        .use_simple_logger()
        .launch(0)
}

Replacing ViewController with the following fixes the error

struct ViewController {
    child_replaced: bool
}
impl Controller<u32, Label<u32>> for ViewController {
    fn event(&mut self, child: &mut Label<u32>, ctx: &mut EventCtx, event: &Event, data: &mut u32, env: &Env) {
        if let Event::Command(ref cmd) = *event  {
            if let Some(()) = cmd.get(CHANGE_VIEW) {
                *child = Label::new("New Label");
                self.child_replaced = true;
                ctx.children_changed();
                return;
            }
        }

        child.event(ctx, event, data, env)
    }

    fn lifecycle(&mut self, child: &mut Label<u32>, ctx: &mut LifeCycleCtx, event: &LifeCycle, data: &u32, env: &Env) {
        if self.child_replaced {
            if let LifeCycle::Internal(InternalLifeCycle::RouteWidgetAdded) = event {
                self.child_replaced = false;
                child.lifecycle(ctx, &LifeCycle::WidgetAdded, data, env);
            }
        }

        child.lifecycle(ctx, event, data, env);
    }
}
...

[luleyleo]

If I understood this correctly, your issue is that Label<T> is basically only a "half widget". To make it complete, it needs a WidgetPod around it. If you do the following for example:

use druid::*;
use druid::widget::*;

const CHANGE_VIEW: Selector<()> = Selector::new("change_view");

fn controlled_widget() -> ControllerHost<Container<u32>, ViewController> {
    ControllerHost::new(Container::new(Label::new("Original")), ViewController)
}

struct Delegate;
impl AppDelegate<u32> for Delegate {
    fn event(&mut self, ctx: &mut DelegateCtx, window_id: WindowId, event: Event, _: &mut u32, _: &Env) -> Option<Event> {
        if let Event::KeyDown(_) = event {
            ctx.submit_command(Command::new(CHANGE_VIEW, (), window_id));
            None
        }
        else {
            Some(event)
        }
    }
}

struct ViewController;
impl Controller<u32, Container<u32>> for ViewController {
    fn event(&mut self, child: &mut Container<u32>, ctx: &mut EventCtx, event: &Event, data: &mut u32, env: &Env) {
        if let Event::Command(ref cmd) = *event  {
            if let Some(()) = cmd.get(CHANGE_VIEW) {
                *child = Container::new(Label::new("New Label"));
                ctx.children_changed();
                return;
            }
        }

        child.event(ctx, event, data, env)
    }
}

fn main() -> Result<(), PlatformError> {
    AppLauncher::with_window(WindowDesc::new(controlled_widget))
        .delegate(Delegate)
        .use_simple_logger()
        .launch(0)
}

If you have just Label, you will have to take care of events like RouteWidgetAdded yourself, but this will work, because Container puts the Label into a WidgetPod which is responsible for doing this. I think it might be useful to have a lightweight container widget Pod<T, W> that just puts the child into a WidgetPod.

Replacing children the way you did here will likely cause problems though. Technically it would be possible to do the same with WidgetPod, eg. *pod.widget_mut() = Label::new("") and it will crash. I'm not sure how we should deal with this though.

I suppose the morale of this is that if you add / replace widgets that are not inside a WidgetPod, you are on your own to make it work.

[gmorenz]

To rephrase what I think you said, a "complete widget" for a lack of a better term typically consists of a WidgetPod containing a Widget. In the special case of a Controller the complete widget instead consists of a WidgetPod containing a HostController widget in tern containing a child Widget and a Controller. This is because the HostController/Controller/child Widget object is in some sense considered to be a single bigger Widget not a container widget containing a child widget.

The smallest unit that Druid supports the replacement of is a complete widget. Your modification makes this happen by making the first complete widget be a set of WidgetPod, HostController, Controller, and Container, and the second child complete widget be a set of WidgetPod and Label.

I can definitely work with this and it seems like a fine model, but it feels like it needs documenting and maybe some thought put into the right terminology.