Created attachment 154369 [details] [review]
[MessageTray] Support reactive notification actors

Created attachment 154695 [details] [review]
[MessageTray] Support reactive notification actors

Use captured-event and check for the event type rather than using
enter-event and leave-event to allow actors added by Notification.addActor to be reactive.

Review of attachment 154695 [details] [review]:

You should do something about your line wrapping. Seriously, try to review:

if (e
    t
else 
    t

Splinter is no fun anymore if you have to switch constantly between code and review ...

The patch obviously works, but then so did the previous version - I don't see how this is an improvement (if I'm not completely wrong you are doing just the same but with more code).

Note that I didn't try any of the proposals underneath, so they might be completely and utterly wrong. Nevertheless I think that restricting the event handling to events caused by the actor being entered from/left to the outside does make sense.

::: js/ui/messageTray.js
@@ +388,3 @@
+                                                                    do {
+                                                                    let source = event.get_source();
+        this.actor.connect('captured-event', Lang.bind(this, function(actor, event) {

Hit me if I'm wrong - but will source just never become null? You only capture events for this.actor, so I don't see how it could be triggered by any actor which is not a child of this.actor.

@@ +395,3 @@
+                                                                    do {
+                                                                    let source = event.get_source();
+        this.actor.connect('captured-event', Lang.bind(this, function(actor, event) {

If you move the pointer from the actor onto the child (or from the child onto the actor) you still call both this._onTrayLeft() and this._onTrayEntered().

If you want to avoid that you can check that event.get_source() == this.actor and event.get_related() is not a child of this.actor. Note that event.get_related() is only valid for crossing events (enter + leave), so you should check the event type before calling that (just check for other event types and return false).

When doing so it might be better to bite the bullet and use a separate function.

the code has changed in master to use StWidget:track-hover. does that fix the problem this bug was fixing?

(In reply to comment #4)
> the code has changed in master to use StWidget:track-hover. does that fix the
> problem this bug was fixing?

Yes.