Skip to content

Commit

Permalink
fix: migrate setNativeProps to state (#10968)
Browse files Browse the repository at this point in the history
### Motivation

This PR is a third attempt to migrate setNativeProps to state in @react-navigation/stack following the official Moving setNativeProps to state guide.

After issues with the implementation of #10767 and more problems with the fix in #10871 we had to take a step back and rethink where the problem lays with these implementations.

The solution was to move the state inside the CardSheet component and use useImperativeHandle to hoist a setter function up to Card to avoid triggering a rerender during Card animate function.
  • Loading branch information
kacperkapusciak committed Nov 4, 2022
1 parent a72b15b commit 37d5440
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 6 deletions.
8 changes: 4 additions & 4 deletions packages/stack/src/views/Stack/Card.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import {
PanGestureHandlerGestureEvent,
} from '../GestureHandler';
import ModalStatusBarManager from '../ModalStatusBarManager';
import CardSheet from './CardSheet';
import CardSheet, { CardSheetRef } from './CardSheet';

type Props = ViewProps & {
interpolationIndex: number;
Expand Down Expand Up @@ -242,7 +242,7 @@ export default class Card extends React.Component<Props> {
private setPointerEventsEnabled = (enabled: boolean) => {
const pointerEvents = enabled ? 'box-none' : 'none';

this.contentRef.current?.setNativeProps({ pointerEvents });
this.ref.current?.setPointerEvents(pointerEvents);
};

private handleStartInteraction = () => {
Expand Down Expand Up @@ -425,7 +425,7 @@ export default class Card extends React.Component<Props> {
}
}

private contentRef = React.createRef<View>();
private ref = React.createRef<CardSheetRef>();

render() {
const {
Expand Down Expand Up @@ -555,7 +555,7 @@ export default class Card extends React.Component<Props> {
/>
) : null}
<CardSheet
ref={this.contentRef}
ref={this.ref}
enabled={pageOverflowEnabled}
layout={layout}
style={contentStyle}
Expand Down
16 changes: 14 additions & 2 deletions packages/stack/src/views/Stack/CardSheet.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,27 @@ type Props = ViewProps & {
children: React.ReactNode;
};

export type CardSheetRef = {
setPointerEvents: React.Dispatch<ViewProps['pointerEvents']>;
};

// This component will render a page which overflows the screen
// if the container fills the body by comparing the size
// This lets the document.body handle scrolling of the content
// It's necessary for mobile browsers to be able to hide address bar on scroll
export default React.forwardRef<View, Props>(function CardSheet(
export default React.forwardRef<CardSheetRef, Props>(function CardSheet(
{ enabled, layout, style, ...rest },
ref
) {
const [fill, setFill] = React.useState(false);
// To avoid triggering a rerender in Card during animation we had to move
// the state to CardSheet. The `setPointerEvents` is then hoisted back to the Card.
const [pointerEvents, setPointerEvents] =
React.useState<ViewProps['pointerEvents']>('auto');

React.useImperativeHandle(ref, () => {
return { setPointerEvents };
});

React.useEffect(() => {
if (typeof document === 'undefined' || !document.body) {
Expand All @@ -32,7 +44,7 @@ export default React.forwardRef<View, Props>(function CardSheet(
return (
<View
{...rest}
ref={ref}
pointerEvents={pointerEvents}
style={[enabled && fill ? styles.page : styles.card, style]}
/>
);
Expand Down

0 comments on commit 37d5440

Please sign in to comment.