Merge 1152812 into 935499a

jamesplease · Jun 15, 2021 · a8c3571 · a8c3571
2 parents 935499a + 1152812
commit a8c3571
Show file tree

Hide file tree

Showing 4 changed files with 75 additions and 21 deletions.
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@please/lrud",
-  "version": "0.0.13-beta.1",
+  "version": "0.0.13-beta.4",
   "description": "A React library for managing focus in TV apps.",
   "main": "es/index.js",
   "module": "es/index.js",

diff --git a/src/focus-node.tsx b/src/focus-node.tsx
@@ -273,11 +273,40 @@ export function FocusNode(
   const nodeRef = useRef(node);
   nodeRef.current = node;
 
+  let nodeExistsInTree = useRef(false);
+
+  useEffect(() => {
+    // This ensures that we don't check for updates on the first render.
+    if (!nodeExistsInTree.current) {
+      return;
+    }
+
+    store.updateNode(nodeId, {
+      disabled: Boolean(disabled),
+      isExiting: Boolean(isExiting),
+      defaultFocusColumn,
+      defaultFocusRow,
+      wrapping,
+      trap: isTrap,
+      restoreTrapFocusHierarchy,
+    });
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, [
+    disabled,
+    isExiting,
+    defaultFocusColumn,
+    defaultFocusRow,
+    wrapping,
+    isTrap,
+    restoreTrapFocusHierarchy,
+  ]);
+
   useEffect(() => {
     store.createNodes(
       staticDefinitions.providerValue.focusNodesHierarchy,
       staticDefinitions.providerValue.focusDefinitionHierarchy
     );
+    nodeExistsInTree.current = true;
 
     const unsubscribe = store.subscribe(() =>
       checkForUpdate({
@@ -299,20 +328,13 @@ export function FocusNode(
     });
 
     return () => {
+      nodeExistsInTree.current = false;
       store.deleteNode(nodeId);
       unsubscribe();
     };
     // eslint-disable-next-line react-hooks/exhaustive-deps
   }, []);
 
-  useEffect(() => {
-    store.updateNode(nodeId, {
-      disabled: Boolean(disabled),
-      isExiting: Boolean(isExiting),
-    });
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [disabled, isExiting]);
-
   const classNameString = `${className} ${node.isFocused ? focusedClass : ''} ${
     node.isFocusedLeaf ? focusedLeafClass : ''
   } ${node.disabled ? disabledClass : ''} ${
@@ -338,7 +360,8 @@ export function FocusNode(
             nodeRef.current.children.length === 0 &&
             !nodeRef.current.disabled &&
             focusState._hasPointerEventsEnabled &&
-            focusState.interactionMode === 'pointer'
+            focusState.interactionMode === 'pointer' &&
+            nodeExistsInTree.current
           ) {
             staticDefinitions.providerValue.store.setFocus(nodeId);
           }
@@ -361,7 +384,10 @@ export function FocusNode(
           }
 
           const focusState = staticDefinitions.providerValue.store.getState();
-          if (!focusState._hasPointerEventsEnabled) {
+          if (
+            !focusState._hasPointerEventsEnabled ||
+            !nodeExistsInTree.current
+          ) {
             return;
           }
 

diff --git a/src/focus-store.ts b/src/focus-store.ts
@@ -26,6 +26,18 @@ interface CreateFocusStoreOptions {
   pointerEvents?: boolean;
 }
 
+// When these props of a node change, then the store
+// will alert subscribers.
+const dynamicNodeProps = [
+  'disabled',
+  'isExiting',
+  'defaultFocusColumn',
+  'defaultFocusRow',
+  'wrapping',
+  'trap',
+  'restoreTrapFocusHierarchy',
+];
+
 export default function createFocusStore({
   orientation = 'horizontal',
   wrapping = false,
@@ -284,26 +296,37 @@ export default function createFocusStore({
       return;
     }
 
-    const newDisabledState = Boolean(update.disabled);
-    const newExitState = Boolean(update.isExiting);
+    update.disabled = Boolean(update.disabled);
+    update.isExiting = Boolean(update.isExiting);
 
-    const disableChanged = currentNode.disabled !== newDisabledState;
-    const exitChanged = currentNode.isExiting !== newExitState;
-    const nodeChanged = disableChanged || exitChanged;
+    const nodeChanged = dynamicNodeProps.some((prop) => {
+      // @ts-ignore
+      return currentNode[prop] !== update[prop];
+    });
 
     if (update && nodeChanged) {
       const newNode: Node = {
         ...currentNode,
-        disabled: newDisabledState,
-        isExiting: newExitState,
+        disabled: update.disabled,
+        isExiting: update.isExiting,
+        defaultFocusColumn:
+          update.defaultFocusColumn ?? currentNode.defaultFocusColumn,
+        defaultFocusRow: update.defaultFocusRow ?? currentNode.defaultFocusRow,
+        wrapping: update.wrapping ?? currentNode.wrapping,
+        trap: update.wrapping ?? currentNode.trap,
+        restoreTrapFocusHierarchy:
+          update.restoreTrapFocusHierarchy ??
+          currentNode.restoreTrapFocusHierarchy,
       };
 
       const updatedChildren = recursivelyUpdateChildren(
         currentState.nodes,
         newNode.children,
+        // Note: we don't pass the full update as the other attributes (trap, wrapping, etc)
+        // only affect the parent, whereas these specific values affect the children.
         {
-          disabled: newDisabledState,
-          isExiting: newExitState,
+          disabled: update.disabled,
+          isExiting: update.isExiting,
         }
       );
 
@@ -320,7 +343,7 @@ export default function createFocusStore({
         },
       };
 
-      if (nodeWasFocused && (newDisabledState || newExitState)) {
+      if (nodeWasFocused && (update.disabled || update.isExiting)) {
         const parentId = newNode.parentId as Id;
 
         updatedState = updateFocus({

diff --git a/src/types.ts b/src/types.ts
@@ -130,6 +130,11 @@ export type Listener = () => void;
 export interface NodeUpdate {
   disabled?: boolean;
   isExiting?: boolean;
+  wrapping?: boolean;
+  trap?: boolean;
+  restoreTrapFocusHierarchy?: boolean;
+  defaultFocusColumn?: number;
+  defaultFocusRow?: number;
 }
 
 export interface FocusNode extends BaseNode {