fix: onActivation hooks not called in v2

theKashey · Jul 2, 2019 · ee1cb14 · ee1cb14
1 parent b0cb28a
commit ee1cb14
Show file tree

Hide file tree

Showing 2 changed files with 51 additions and 24 deletions.
diff --git a/_tests/FocusLock.spec.js b/_tests/FocusLock.spec.js
@@ -1069,5 +1069,27 @@ describe('react-focus-lock', () => {
         }, 1);
       });
     });
+
+    describe('Hooks',  () => {
+      it('onActivation/Deactivation', () => {
+        const onActivation = sinon.spy();
+        const onDeactivation = sinon.spy();
+        const wrapper = mount(
+          <div>
+            <FocusLock onActivation={onActivation} onDeactivation={onDeactivation}>
+              <button id="b1">button1</button>
+              <button id="b2">button2</button>
+            </FocusLock>
+          </div>
+        );
+        sinon.assert.calledOnce(onActivation);
+        sinon.assert.notCalled(onDeactivation);
+
+        wrapper.unmount();
+
+        sinon.assert.calledOnce(onActivation);
+        sinon.assert.calledOnce(onDeactivation);
+      })
+    });
   });
 });
diff --git a/src/Lock.js b/src/Lock.js
@@ -14,28 +14,49 @@ function FocusLock(props) {
   const isActive = useRef(false);
   const originalFocusedElement = useRef(null);
 
+  const {
+    children,
+    disabled,
+    noFocusGuards,
+    persistentFocus,
+    autoFocus,
+    allowTextSelection,
+    group,
+    className,
+    whiteList,
+    shards = emptyArray,
+    as: Container = 'div',
+    lockProps: containerProps = {},
+    sideCar: SideCar,
+
+    returnFocus: shouldReturnFocus,
+
+    onActivation: onActivationCallback,
+    onDeactivation: onDeactivationCallback,
+  } = props;
+
   // SIDE EFFECT CALLBACKS
 
   const onActivation = useCallback(() => {
     originalFocusedElement.current = (
       originalFocusedElement.current || (document && document.activeElement)
     );
-    if (observed.current && props.onActivation) {
-      props.onActivation(observed.current);
+    if (observed.current && onActivationCallback) {
+      onActivationCallback(observed.current);
     }
     isActive.current = true;
-  }, []);
+  }, [onActivationCallback]);
 
   const onDeactivation = useCallback(() => {
     isActive.current = false;
-    if (props.onDeactivation) {
-      props.onDeactivation(observed.current);
+    if (onDeactivationCallback) {
+      onDeactivationCallback(observed.current);
     }
-  }, []);
+  }, [onDeactivationCallback]);
 
   const returnFocus = useCallback(() => {
     const { current } = originalFocusedElement;
-    if (props.returnFocus && current && current.focus) {
+    if (shouldReturnFocus && current && current.focus) {
       current.focus();
       originalFocusedElement.current = null;
     }
@@ -56,27 +77,11 @@ function FocusLock(props) {
 
   const setObserveNode = useCallback((newObserved) => {
     if (observed.current !== newObserved) {
-      observed.current = realObserved;
+      observed.current = newObserved;
       setObserved(newObserved);
     }
   }, []);
 
-  const {
-    children,
-    disabled,
-    noFocusGuards,
-    persistentFocus,
-    autoFocus,
-    allowTextSelection,
-    group,
-    className,
-    whiteList,
-    shards = emptyArray,
-    as: Container = 'div',
-    lockProps: containerProps = {},
-    sideCar: SideCar,
-  } = props;
-
   if (process.env.NODE_ENV !== 'production') {
     if (typeof allowTextSelection !== 'undefined') {
       // eslint-disable-next-line no-console