idanarye / woab

Widgets on Actors Bridge - a GUI microframework for combining GTK with Actix
https://crates.io/crates/woab
MIT License
14 stars 1 forks source link

Add a `connect_nonbuilder_signals` test. #11

Closed idanarye closed 3 years ago

idanarye commented 3 years ago

@piegamesde I'm making this a PR because woab::connect_signal_handler is your addition and I'm not sure how exactly it's supposed to be used. I also want to replace it with a BuilderSingalConnector based syntax to support tag and inhibit and whatever else we'll add to it in the future.

So - is this the correct way to use it? To split the same signal to multiple actors? Or is its purpose to connect to the same actor signals that are not originated in the builder?

piegamesde commented 3 years ago

The only difference is that I make the call within started and that I'm not restricted to widgets. For widgets, I of course prefer the usual API.

let new = gio::SimpleAction::new("new", None);
woab::connect_signal_handler::<Self, _, _>(&new, "activate", "NewDocument", ctx);
application.add_action(&new);
application.set_accels_for_action("app.new", &["<Primary>N"]);

The make_signal_handler method has to be connected manually to the object. This can be used in the following two edge cases:

  1. One wants to recover from a potential failure
  2. One needs access to the SignalHandlerId. This is required to inhibit or de-register signal handlers later on.

Btw I was planning to make a PR with documentation+tests+examples anyways, the next time I find some time.

idanarye commented 3 years ago

Ideally I would like to make this work with BuilderSingalConnector:

let new = gio::SimpleAction::new("new", None);
let save = gio::SimpleAction::new("save", None);
SignalType::connector().route_to(ctx)
    .connect(&new, "activate", "NewDocument").unwrap()
    .connect(&save, "activate", "SaveDocument").unwrap();

Because then we can get tagging (is tagging needed for actions? It's still useful for manually connected widgets...) and inhibit (do actions use inhibit?). Tagging is easy:

let new = gio::SimpleAction::new("new", None);
let save = gio::SimpleAction::new("save", None);
SignalType::connector().tag(my_tag).route_to(ctx)
    .connect(&new, "activate", "NewDocument").unwrap()
    .connect(&save, "activate", "SaveDocument").unwrap();

But inhibit can get weird - you have to specify the inhibit and match on the signals before the list of connect calls:

let new = gio::SimpleAction::new("new", None);
let save = gio::SimpleAction::new("save", None);
SignalType::connector()
    .inhibit(|signal| match signal {
        NewDocument => gtk::Inhibit(true),
        SaveDocument => gtk::Inhibit(false),
    })
    .route_to(ctx)
    .connect(&new, "activate", "NewDocument").unwrap()
    .connect(&save, "activate", "SaveDocument").unwrap();

Of course, since we are connecting each signal separately I could do this instead:

let new = gio::SimpleAction::new("new", None);
let save = gio::SimpleAction::new("save", None);
SignalType::connector().route_to(ctx)
    .connect(&new, "activate", "NewDocument", |_| gtk::Inhibit(true)).unwrap()
    .connect(&save, "activate", "SaveDocument", |_| gtk::Inhibit(false)).unwrap();

But then if you want to actually use the signal you'd have to match again:

let new = gio::SimpleAction::new("new", None);
let save = gio::SimpleAction::new("save", None);
SignalType::connector().route_to(ctx)
    .connect(&new, "activate", "NewDocument", |signal| match signal {
        SignalType::NewDocument(arg1, arg2) => ...,
        _ => panic!("wrong signal type"),

    }.unwrap())
    .connect(&save, "activate", "SaveDocument", |_| gtk::Inhibit(false)).unwrap();

Then again - considering how actions don't use inhibit (since there is no "default" behavior), we should be fine with the original connector().inhibit() syntax.

piegamesde commented 3 years ago

do actions use inhibit?

I've had a look at it, and as far as I can tell the callbacks don't return anything, so no. I generally like the new syntax, but what I'm worried about is how to access the SignalHandlerId from the connected signal.

idanarye commented 3 years ago

How about we let the signal router return the closure, and then you register it yourself in the action?

let new = gio::SimpleAction::new("new", None);
let save = gio::SimpleAction::new("save", None);
let signal_router = SignalType::connector().route_to(ctx);
let new_document_handler_id = new.connect_local("activate", signal_router.handler("NewDocument").unwrap());
let save_document_handler_id = save.connect_local("activate", signal_router.handler("SaveDocument").unwrap());
idanarye commented 3 years ago

@piegamesde I changed the test to use actions instead of widgets. Does it seem right now?

Initially I disconnected the signal for action2, but that broke the stream for action1 so I had to change it to block_signal. I think this is because we are using two streams (maybe underneath Actix unites them or something?), and maybe once I change it to the router syntax I suggested it would use a single stream we should be able to disconnect signals.