Skip to content

Commit

Permalink
perf: remove unnecessary state changes for isActive if not used (#161)
Browse files Browse the repository at this point in the history
  • Loading branch information
pierpo authored Nov 21, 2024
1 parent c031bd3 commit 3c53f44
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 52 deletions.
40 changes: 15 additions & 25 deletions packages/lib/src/spatial-navigation/components/FocusableView.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
import { SpatialNavigationNode, SpatialNavigationNodeDefaultProps } from './Node';
import {
FocusableNodeState,
SpatialNavigationNode,
SpatialNavigationNodeDefaultProps,
} from './Node';
import { Platform, View, ViewStyle, ViewProps } from 'react-native';
import { forwardRef, useImperativeHandle, useMemo, useRef } from 'react';
import { SpatialNavigationNodeRef } from '../types/SpatialNavigationNodeRef';
Expand All @@ -7,18 +11,7 @@ import { useSpatialNavigatorFocusableAccessibilityProps } from '../hooks/useSpat

type FocusableViewProps = {
style?: ViewStyle;
children:
| React.ReactElement
| ((props: {
/** Returns whether the root is focused or not. */
isFocused: boolean;
/** Returns whether the root is active or not. An active node is active if one of its children is focused. */
isActive: boolean;
/** Returns whether the root is active or not.
* This is very handy if you want to hide the focus on your page elements when
* the side-menu is focused (since it is a different root navigator) */
isRootActive: boolean;
}) => React.ReactElement);
children: React.ReactElement | ((props: FocusableNodeState) => React.ReactElement);
viewProps?: ViewProps & {
onMouseEnter?: () => void;
};
Expand Down Expand Up @@ -58,14 +51,12 @@ export const SpatialNavigationFocusableView = forwardRef<SpatialNavigationNodeRe

return (
<SpatialNavigationNode isFocusable {...props} ref={nodeRef}>
{({ isFocused, isActive, isRootActive }) => (
{(nodeState) => (
<InnerFocusableView
viewProps={viewProps}
webProps={webProps}
style={style}
isActive={isActive}
isFocused={isFocused}
isRootActive={isRootActive}
nodeState={nodeState}
>
{children}
</InnerFocusableView>
Expand All @@ -86,15 +77,16 @@ type InnerFocusableViewProps = FocusableViewProps & {
onMouseEnter?: undefined;
onClick?: undefined;
};
isActive: boolean;
isFocused: boolean;
isRootActive: boolean;
nodeState: FocusableNodeState;
};

const InnerFocusableView = forwardRef<View, InnerFocusableViewProps>(
({ viewProps, webProps, children, isActive, isFocused, isRootActive, style }, ref) => {
({ viewProps, webProps, children, nodeState, style }, ref) => {
const accessibilityProps = useSpatialNavigatorFocusableAccessibilityProps();
const accessibilityState = useMemo(() => ({ selected: isFocused }), [isFocused]);
const accessibilityState = useMemo(
() => ({ selected: nodeState.isFocused }),
[nodeState.isFocused],
);

return (
<View
Expand All @@ -105,9 +97,7 @@ const InnerFocusableView = forwardRef<View, InnerFocusableViewProps>(
{...viewProps}
{...webProps}
>
{typeof children === 'function'
? children({ isFocused, isActive, isRootActive })
: children}
{typeof children === 'function' ? children(nodeState) : children}
</View>
);
},
Expand Down
70 changes: 43 additions & 27 deletions packages/lib/src/spatial-navigation/components/Node.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,31 +10,27 @@ import { NodeIndexRange } from '@bam.tech/lrud';
import { SpatialNavigationNodeRef } from '../types/SpatialNavigationNodeRef';
import { useIsRootActive } from '../context/IsRootActiveContext';

type NonFocusableNodeState = {
/** Returns whether the root is active or not. An active node is active if one of its children is focused. */
isActive: boolean;
/** Returns whether the root is active or not.
* This is very handy if you want to hide the focus on your page elements when
* the side-menu is focused (since it is a different root navigator) */
isRootActive: boolean;
};

export type FocusableNodeState = NonFocusableNodeState & {
/** Returns whether the root is focused or not. */
isFocused: boolean;
};

type FocusableProps = {
isFocusable: true;
children: (props: {
/** Returns whether the root is focused or not. */
isFocused: boolean;
/** Returns whether the root is active or not. An active node is active if one of its children is focused. */
isActive: boolean;
/** Returns whether the root is active or not.
* This is very handy if you want to hide the focus on your page elements when
* the side-menu is focused (since it is a different root navigator) */
isRootActive: boolean;
}) => React.ReactElement;
children: (props: FocusableNodeState) => React.ReactElement;
};
type NonFocusableProps = {
isFocusable?: false;
children:
| React.ReactElement
| ((props: {
/** Returns whether the root is active or not. An active node is active if one of its children is focused. */
isActive: boolean;
/** Returns whether the root is active or not.
* This is very handy if you want to hide the focus on your page elements when
* the side-menu is focused (since it is a different root navigator) */
isRootActive: boolean;
}) => React.ReactElement);
children: React.ReactElement | ((props: NonFocusableNodeState) => React.ReactElement);
};
type DefaultProps = {
onFocus?: () => void;
Expand Down Expand Up @@ -166,17 +162,23 @@ export const SpatialNavigationNode = forwardRef<SpatialNavigationNodeRef, Props>

const shouldHaveDefaultFocus = useSpatialNavigatorDefaultFocus();

const accessedPropertiesRef = useRef<Set<keyof FocusableNodeState>>(new Set());

useEffect(() => {
spatialNavigator.registerNode(id, {
parent: parentId,
isFocusable,
onBlur: () => {
currentOnBlur.current?.();
setIsFocused(false);
if (accessedPropertiesRef.current.has('isFocused')) {
setIsFocused(false);
}
},
onFocus: () => {
currentOnFocus.current?.();
setIsFocused(true);
if (accessedPropertiesRef.current.has('isFocused')) {
setIsFocused(true);
}
},
onSelect: () => currentOnSelect.current?.(),
onLongSelect: () => currentOnLongSelect.current?.(),
Expand All @@ -185,11 +187,15 @@ export const SpatialNavigationNode = forwardRef<SpatialNavigationNodeRef, Props>
indexRange,
onActive: () => {
currentOnActive.current?.();
setIsActive(true);
if (accessedPropertiesRef.current.has('isActive')) {
setIsActive(true);
}
},
onInactive: () => {
currentOnInactive.current?.();
setIsActive(false);
if (accessedPropertiesRef.current.has('isActive')) {
setIsActive(false);
}
},
});

Expand All @@ -203,11 +209,21 @@ export const SpatialNavigationNode = forwardRef<SpatialNavigationNodeRef, Props>
}
}, [id, isFocusable, shouldHaveDefaultFocus, spatialNavigator]);

// This proxy allows to track whether a property is used or not
// hence allowing to ignore re-renders for unused properties
const proxyObject = new Proxy(
{ isFocused, isActive, isRootActive },
{
get(target, prop: keyof FocusableNodeState) {
accessedPropertiesRef.current.add(prop);
return target[prop];
},
},
);

return (
<ParentIdContext.Provider value={id}>
{typeof children === 'function'
? bindRefToChild(children({ isFocused, isActive, isRootActive }))
: children}
{typeof children === 'function' ? bindRefToChild(children(proxyObject)) : children}
</ParentIdContext.Provider>
);
},
Expand Down

0 comments on commit 3c53f44

Please sign in to comment.