Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extend the set of layout animation mocks #3974

Merged
merged 1 commit into from Jan 19, 2023

Conversation

jalooc
Copy link
Contributor

@jalooc jalooc commented Jan 19, 2023

Summary

This PR extends #3801 by introducing layout animation types and modifiers that those animation types require to be fully functional.

Currently, the tests fail with something like:

TypeError: Cannot read properties of undefined (reading 'delay')

 > 107 |   const transition = CurvedTransition.delay(10).duration(10),
       |                                       ^

Test plan

Creating a test (e.g. using React Native Testing Library) of a component containing any of the freshly added layout animation types and / or chained modifiers they support. E.g.:

const MyComponent = () => (
  <Animated.View layout={CurvedTransition.delay(10).duration(10)} />
)

test('test the test', () => {
  const screen = render(<MyComponent />)
})

The above example fails before the patch from this PR is introduced and succeeds afterward.

Question

One question to resolve is how closely the mocks should match the real api in the context of which modifiers apply to which animation types (e.g. not all animation types support .stiffness()). Doing so would require more complex mocks that are also harder to maintain for the need to keeping them in sync. An alternative, simpler approach is to add all the modifiers to all the animations - I figure it's okay since these kinds of mocks are not directly consumed in the tests - they just serve enabling test-rendering components utilising layout animations without errors. The latter is the approach I went with, but lmk if you think the other one is a better fit.

* added layout animation types
* added BaseAnimation methods that those layout animations support
@piaskowyk
Copy link
Member

@jalooc thanks for your PR! 🙌 When it comes to mocks, I think that they should be as simple as possible because mocks don't contain any logic. So adding modifiers to all the animations is fine for me.

@piaskowyk piaskowyk merged commit 883ec28 into software-mansion:main Jan 19, 2023
@tomekzaw tomekzaw added the Needs backport This PR needs to be backported to Reanimated2 branch label Jan 20, 2023
fluiddot pushed a commit to wordpress-mobile/react-native-reanimated that referenced this pull request Jun 5, 2023
<!-- Thanks for submitting a pull request! We appreciate you spending
the time to work on these changes. Please follow the template so that
the reviewers can easily understand what the code changes affect. -->

## Summary

This PR extends
software-mansion#3801 by
introducing layout animation types and modifiers that those animation
types require to be fully functional.

Currently, the tests fail with something like:
```
TypeError: Cannot read properties of undefined (reading 'delay')

 > 107 |   const transition = CurvedTransition.delay(10).duration(10),
       |                                       ^
```

## Test plan

Creating a test (e.g. using React Native Testing Library) of a component
containing any of the freshly added layout animation types and / or
chained modifiers they support. E.g.:
```tsx
const MyComponent = () => (
  <Animated.View layout={CurvedTransition.delay(10).duration(10)} />
)

test('test the test', () => {
  const screen = render(<MyComponent />)
})
```
The above example fails before the patch from this PR is introduced and
succeeds afterward.


## Question

One question to resolve is how closely the mocks should match the real
api in the context of which modifiers apply to witch animation types
(e.g. not all animation types support `.stiffness()`). Doing so would
require more complex mocks that are also harder to maintain for the need
to keeping them in sync. An alternative, simpler approach is to add all
the modifiers to all the animations - I figure it's okay since these
kinds of mocks are not directly consumed in the tests - they just serve
enabling test-rendering components utilising layout animations without
errors. The latter is the approach I went with, but lmk if you think the
other one is a better fit.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs backport This PR needs to be backported to Reanimated2 branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants