Repository update causes select widget to stop showing subwindow

[giannissc]

Upon updating the directory I have noticed that when I click the select button the tooltip (subwindow) does no longer show. I have tried rolling back to the previous release and everything works fine @derekdreery

[arifd]

Pinging @Lejero also who made the change.

Btw, having dropdown open a subwindow doesn't work in my tiled window manager (as the windows tile, and don't float). Would it be inconceivably hard to change the approach to dropdowns? (could it paint_with_z_index instead?)

[Lejero]

One of my PRs updated to a newer version of druid being referenced because there was a problem with the BackgroundBrush type not existing I think, but that's the only thing I can think of that would affect it at all. Otherwise I didn't touch code outside of the ProgressBar, and ProgressBar example. So I guess there might be a drawing change somewhere since druid was last updated?

[r-ml]

Both the select and dropdown examples are currently broken.
This happens because the DROP notification never reaches its intended target.
I assume this PR is the culprit after taking a look at the changelog, although I haven't tested reverting it yet.

For now, swapping submit_notification(DROP) with submit_command(DROP) is the workaround I'm using:

From f274f07ad94c6787422cd879769737807782e7ef Mon Sep 17 00:00:00 2001
From: rml <[email protected]>
Date: Wed, 14 Apr 2021 10:42:34 +0100
Subject: [PATCH] dropdown: submit DROP as a command instead of a notification

Widgets no longer handle notifications from themselves as of
https://github.com/linebender/druid/commit/cbc9be65823d0aa11756ee17511d177d0b2f142d
which causes the `DROP` notification to not reach `Dropdown::event()`.

Switch to using a `Command`.

https://github.com/linebender/druid-widget-nursery/issues/31
---
 examples/dropdown.rs   | 6 +++---
 src/dropdown.rs        | 2 +-
 src/dropdown_select.rs | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/examples/dropdown.rs b/examples/dropdown.rs
index 3b7057e..5f053df 100644
--- a/examples/dropdown.rs
+++ b/examples/dropdown.rs
@@ -32,7 +32,7 @@ fn main_widget() -> impl Widget<DropDownState> {
                         .with_flex_spacer(1.)
                         .with_child(
                             Button::new("V")
-                                .on_click(|ctx: &mut EventCtx, _, _| ctx.submit_notification(DROP)),
+                                .on_click(|ctx: &mut EventCtx, _, _| ctx.submit_command(DROP.to(ctx.widget_id()))),
                         ),
                     |_, _| {
                         let places: Vec<(&'static str, String)> =
@@ -53,7 +53,7 @@ fn main_widget() -> impl Widget<DropDownState> {
                         .with_flex_spacer(1.)
                         .with_child(
                             Button::new("V")
-                                .on_click(|ctx: &mut EventCtx, _, _| ctx.submit_notification(DROP)),
+                                .on_click(|ctx: &mut EventCtx, _, _| ctx.submit_command(DROP.to(ctx.widget_id()))),
                         ),
                     |_, _| {
                         RadioGroup::new(vec![
@@ -70,7 +70,7 @@ fn main_widget() -> impl Widget<DropDownState> {
             .with_child(
                 Dropdown::new_sized(
                     Button::new(|f: &Fruit, _: &Env| format!("{:?}", f))
-                        .on_click(|ctx: &mut EventCtx, _, _| ctx.submit_notification(DROP)),
+                        .on_click(|ctx: &mut EventCtx, _, _| ctx.submit_command(DROP.to(ctx.widget_id()))),
                     |_, _| {
                         RadioGroup::new(vec![
                             ("Apple", Fruit::Apple),
diff --git a/src/dropdown.rs b/src/dropdown.rs
index 74d8435..95c7b8c 100644
--- a/src/dropdown.rs
+++ b/src/dropdown.rs
@@ -49,7 +49,7 @@ impl<T: Data> Dropdown<T> {
 impl<T: Data> Widget<T> for Dropdown<T> {
     fn event(&mut self, ctx: &mut EventCtx, event: &Event, data: &mut T, env: &Env) {
         match event {
-            Event::Notification(n) if n.is(DROP) => {
+            Event::Command(n) if n.is(DROP) => {
                 let widget = (self.drop)(data, env);
                 let origin = ctx.to_screen(Point::new(0., ctx.size().height));
                 self.window = Some(
diff --git a/src/dropdown_select.rs b/src/dropdown_select.rs
index d8b8e3c..1ff9320 100644
--- a/src/dropdown_select.rs
+++ b/src/dropdown_select.rs
@@ -69,7 +69,7 @@ impl<T: Data + PartialEq> DropdownSelect<T> {
         })
         .on_click(|ctx: &mut EventCtx, t: &mut DropdownState<T>, _| {
             t.expanded = true;
-            ctx.submit_notification(DROP)
+            ctx.submit_command(DROP.to(ctx.widget_id()))
         });
 
         let make_drop = move |_t: &DropdownState<T>, env: &Env| {
-- 
2.30.2

[richard-uk1]

Informational post: Under the optimistic merging strategy, commits that partially or fully revert other commits are allowed.

[jneem]

@arifd I recently pushed some changes to druid to fix floating windows on tiled window managers (assuming you're using the GTK backend)

[arifd]

Druid main, or here in the widget nursery? And excuse my ignorance but when you say "pushed" does that mean the changes are in effect already?

Also, what did the change involve exactly?

Thanks!

[jneem]

On the main druid repo, in #1714. It doesn't seem to be in effect here yet, because this crate is pinned on an older version of druid...

[djeedai]

Note: Just tried the dropdown example on Windows and this still doesn't work, even after merging #37.

INFO druid::window: 1 unhandled notifications:
INFO druid::window: 0: Notification: Selector druid-widget-nursery.dropdown.drop from WidgetId(10)

[r-ml]

Note: Just tried the dropdown example on Windows and this still doesn't work, even after merging #37.

Of course not, the PR does not touch that example. It fixes the select example only.
You'd need to fix that one as well, see above patch.

BTW, does directing the command directly to the widget any faster, performance wise? I'm not familiar with druid's internals.

[djeedai]

I think this is resolved now with the additional fix of #43. @giannissc can you please confirm?

[giannissc]

Yeah I can confirm that this has been resolved