Fix nested clickables (#710)

* added repro example

* simplify and "fix" BubbleEvent

* encapsulate mouse capture to avoid unintentional side effects

* various cleanup

* add effect hook to relase capture on unmount

* reset selectedExample

* fix comment typo and use line comments instead of block comments

examples/Examples.re

@@ -150,6 +150,11 @@ let state: state = {
       render: _ => ZoomExample.render(),
       source: "ZoomExample.re",
     },
+    {
+      name: "Nested Clickables",
+      render: _ => NestedClickable.render(),
+      source: "NestedClickable.re",
+    },
   ],
   selectedExample: "Animation",
 };

examples/NestedClickable.re

@@ -0,0 +1,29 @@
+open Revery;
+open Revery.UI;
+open Revery.UI.Components;
+
+module Styles = {
+  open Style;
+
+  let outer = [
+    position(`Absolute),
+    top(0),
+    bottom(0),
+    left(0),
+    right(0),
+    justifyContent(`Center),
+    alignItems(`Center),
+    backgroundColor(Colors.yellow),
+  ];
+};
+
+let%component clickies = () => {
+  let%hook (text, setText) = Hooks.state("Click something");
+
+  <Clickable
+    style=Styles.outer onClick={() => setText(_ => "Clicked outside")}>
+    <Button title=text onClick={() => setText(_ => "Clicked inside")} />
+  </Clickable>;
+};
+
+let render = () => <clickies />;

src/UI/Mouse.re

@@ -428,80 +428,64 @@ let rec handleMouseEnterDiff = (deepestNode, evtParams, ~newNodes=[], ()) => {
 };
 
 let dispatch =
-    (cursor: Cursor.t, evt: Events.internalMouseEvents, node: Node.node) => {
-  node#hasRendered()
-    ? {
-      let pos = getPositionFromMouseEvent(cursor, evt);
-
-      let eventToSend = internalToExternalEvent(cursor, evt);
-
-      let mouseDown = isMouseDownEv(eventToSend);
-      if (mouseDown) {
-        switch (getFirstFocusable(node, pos)) {
-        | Some(node) => Focus.dispatch(node)
-        | None => Focus.loseFocus()
-        };
-      } else {
-        ();
+    (cursor: Cursor.t, evt: Events.internalMouseEvents, node: Node.node) =>
+  if (node#hasRendered()) {
+    let pos = getPositionFromMouseEvent(cursor, evt);
+    let eventToSend = internalToExternalEvent(cursor, evt);
+
+    if (isMouseDownEv(eventToSend)) {
+      switch (getFirstFocusable(node, pos)) {
+      | Some(node) => Focus.dispatch(node)
+      | None => Focus.loseFocus()
       };
+    };
 
-      handleListeners(eventToSend);
-
-      if (!handleCapture(eventToSend)) {
-        let deepestNode = getTopMostNode(node, pos);
-        let mouseMove = isMouseMoveEv(eventToSend);
-        if (mouseMove) {
-          let mouseMoveEventParams = getMouseMoveEventParams(cursor, evt);
-
-          switch (deepestNode) {
-          | None =>
-            /*
-             * if no node found, call bubbled MouseOut on deepestStoredNode if there's some stored nodes
-             * And recursively send mouseLeave events to storedNodes if they exist
-             */
-            switch (storedNodesUnderCursor^) {
-            | [] => ()
-            | [node, ..._] => bubble(node, MouseOut(mouseMoveEventParams))
-            };
-
-            sendMouseLeaveEvents(
-              storedNodesUnderCursor^,
-              mouseMoveEventParams,
-            );
-
-          | Some(deepNode) =>
-            switch (storedNodesUnderCursor^) {
-            | [] =>
-              /*
-               * If some deepNode is found and there aer no storedNodes
-               * Traverse the tree and call MouseEnter on each  node -  https://developer.mozilla.org/en-US/docs/Web/Events/mouseenter
-               * And call bubbled MouseOver on deepNode
-               */
-              sendMouseEnterEvents(deepNode, mouseMoveEventParams);
-              bubble(deepNode, MouseOver(mouseMoveEventParams));
-            | [node, ..._] =>
-              /*
-               * Only handle diff if the deepestStoredNode !==  the deepestFoundNode
-               */
-              if (node#getInternalId() != deepNode#getInternalId()) {
-                handleMouseEnterDiff(deepNode, mouseMoveEventParams, ());
-              }
-            }
-          };
-        };
+    handleListeners(eventToSend);
+
+    if (!handleCapture(eventToSend)) {
+      let deepestNode = getTopMostNode(node, pos);
+
+      if (isMouseMoveEv(eventToSend)) {
+        let mouseMoveEventParams = getMouseMoveEventParams(cursor, evt);
 
         switch (deepestNode) {
-        | None => ()
-        | Some(node) =>
-          let bbox = node#getBoundingBox();
-          DebugDraw.setActive(bbox);
-          bubble(node, eventToSend);
-          let cursor = node#getCursorStyle();
-          Event.dispatch(onCursorChanged, cursor);
+        | None =>
+          // if no node found, call bubbled MouseOut on deepestStoredNode if there's some stored nodes
+          // And recursively send mouseLeave events to storedNodes if they exist
+          switch (storedNodesUnderCursor^) {
+          | [] => ()
+          | [node, ..._] => bubble(node, MouseOut(mouseMoveEventParams))
+          };
+
+          sendMouseLeaveEvents(storedNodesUnderCursor^, mouseMoveEventParams);
+
+        | Some(deepNode) =>
+          switch (storedNodesUnderCursor^) {
+          | [] =>
+            // If some deepNode is found and there are no storedNodes
+            // Traverse the tree and call MouseEnter on each  node -  https://developer.mozilla.org/en-US/docs/Web/Events/mouseenter
+            // And call bubbled MouseOver on deepNode
+            sendMouseEnterEvents(deepNode, mouseMoveEventParams);
+            bubble(deepNode, MouseOver(mouseMoveEventParams));
+          | [node, ..._] =>
+            // Only handle diff if the deepestStoredNode !==  the deepestFoundNode
+            if (node#getInternalId() != deepNode#getInternalId()) {
+              handleMouseEnterDiff(deepNode, mouseMoveEventParams, ());
+            }
+          }
         };
       };
 
-      Cursor.set(cursor, pos);
-    }
-    : ();
-};
+      switch (deepestNode) {
+      | None => ()
+      | Some(node) =>
+        let bbox = node#getBoundingBox();
+        DebugDraw.setActive(bbox);
+        bubble(node, eventToSend);
+        let cursor = node#getCursorStyle();
+        Event.dispatch(onCursorChanged, cursor);
+      };
+    };
+
+    Cursor.set(cursor, pos);
+  };

src/UI/UiEvents.re

@@ -1,60 +1,29 @@
 open Node;
 open NodeEvents;
 
-module BubbledEvent = {
-  type bubbledEvent = {
-    id: int,
-    event,
-    shouldPropagate: bool,
-    defaultPrevented: bool,
-    stopPropagation: unit => unit,
-    preventDefault: unit => unit,
-  };
-
-  let generateId = () => {
-    let id = ref(1);
-    () => {
-      id := id^ + 1;
-      id^;
+module BubbleEvent: {
+  type t =
+    pri {
+      event,
+      mutable shouldPropagate: bool,
+      mutable defaultPrevented: bool,
     };
+
+  let stopPropagation: t => unit;
+  let preventDefault: t => unit;
+  let make: event => t;
+} = {
+  type t = {
+    event,
+    mutable shouldPropagate: bool,
+    mutable defaultPrevented: bool,
   };
 
-  let getId = generateId();
-  let activeEvent = ref(None);
+  let stopPropagation = event => event.shouldPropagate = false;
 
-  let stopPropagation = (id, ()) =>
-    switch (activeEvent^) {
-    | Some(evt) =>
-      if (id == evt.id) {
-        activeEvent := Some({...evt, shouldPropagate: false});
-      }
-    | None => ()
-    };
+  let preventDefault = event => event.defaultPrevented = true;
 
-  let preventDefault = (id, ()) =>
-    switch (activeEvent^) {
-    | Some(evt) =>
-      if (id == evt.id) {
-        activeEvent := Some({...evt, defaultPrevented: true});
-      }
-    | None => ()
-    };
-
-  let make = event => {
-    let id = getId();
-    let wrappedEvent =
-      Some({
-        id,
-        event,
-        shouldPropagate: true,
-        defaultPrevented: false,
-        stopPropagation: stopPropagation(id),
-        preventDefault: preventDefault(id),
-      });
-
-    activeEvent := wrappedEvent;
-    wrappedEvent;
-  };
+  let make = event => {event, shouldPropagate: true, defaultPrevented: false};
 };
 
 let isNodeImpacted = (n, pos) => n#hitTest(pos);
@@ -102,7 +71,7 @@ let getTopMostNode = (node: node, pos) => {
       let ignored = mode == Ignore;
 
       let revChildren = List.rev(node#getChildren());
-      let ret =
+      let maybeChildNode =
         switch (revChildren) {
         | [] => ignored ? None : Some(node)
         | children =>
@@ -117,19 +86,18 @@ let getTopMostNode = (node: node, pos) => {
           )
         };
 
-      switch (ret) {
+      switch (maybeChildNode) {
       | None => ignored ? None : Some(node)
-      | Some(v) => Some(v)
+      | Some(childNode) => Some(childNode)
       };
     };
   };
 
-  let ret: option(node) = f(node, Default);
-  ret;
+  f(node, Default);
 };
 
 let rec traverseHeirarchy = (node: node, bubbled) =>
-  BubbledEvent.(
+  BubbleEvent.(
     /*
      track if default prevent or propagation stopped per node
      stop traversing node hierarchy if stop propagation is called
@@ -146,9 +114,8 @@ let rec traverseHeirarchy = (node: node, bubbled) =>
 
 let bubble = (node, event: event) => {
   /* Wrap event with preventDefault and stopPropagation */
-  let evt = BubbledEvent.make(event);
-  switch (evt) {
-  | Some(e) => traverseHeirarchy(node, e)
-  | None => ()
-  };
+  traverseHeirarchy(
+    node,
+    BubbleEvent.make(event),
+  );
 };

src/UI_Components/Button.rei

@@ -12,7 +12,7 @@ Simple out-of-box button component
 let make:
   (
     ~title: string,
-    ~onClick: Clickable.clickFunction=?,
+    ~onClick: unit => unit=?,
     ~color: Revery_Core.Color.t=?,
     ~fontSize: int=?,
     ~width: int=?,