fix: press event order

[mdjastrzebski]

Summary

Resolves: #1687

Reverses pressOut and press event order so that it matches experimental data for regular presses:

1. pressIn,
2. (press duration)
3. pressOut
4. (no waiting) press.

The previous setup matched edge case for very short (< 130ms) presses, when order was:

1. pressIn
2. (short press duration < 130 ms)
3. press
4. (wait till 130 ms - short press duration)
5. pressOut

Scope

• Fix event order
• Prevent triggering events on unmounted components

See experiments: https://github.com/callstack/react-native-testing-library/wiki/Press-Events

Test plan

All tests pass

[mdjastrzebski]

@pierrezimmermannbam I'm messing with User Event press / longPress implementation. You might want to take a look,

[winghouchan]

@mdjastrzebski: interesting observation in regards to 'short' presses. I was able to reproduce this in my own experimentation too. Do you know if this phenomenon is documented somewhere (whether that be actual documentation or a reference to source code)?

[mdjastrzebski]

@winghouchan The best source is probably Pressability code: https://github.com/facebook/react-native/blob/33e1ae13f88979d8346e4f5ddd4cbce500a16999/packages/react-native/Libraries/Pressability/Pressability.js#L342

There is the code path when pressOut can be either executed immeditaly or delayed: https://github.com/facebook/react-native/blob/33e1ae13f88979d8346e4f5ddd4cbce500a16999/packages/react-native/Libraries/Pressability/Pressability.js#L788

Then there is the code path for press when there is no path for delay: https://github.com/facebook/react-native/blob/33e1ae13f88979d8346e4f5ddd4cbce500a16999/packages/react-native/Libraries/Pressability/Pressability.js#L757

[pierrezimmermannbam]

Nice work investigating the issue! The fix is correct but I suggested a slightly different approach

[mdjastrzebski]

This PR has been released in v12.8.1 🎉

[cartok]

Since these changes, I have issues on some tests when using userEvent() i'd have to setup userEvent() with the delay option, like { delay: 100 }. Was it expected from these changes? The fireEvent() still works as before.

[mdjastrzebski]

@cartok What kind of issues are you having with user event?

[cartok]

@mdjastrzebski Thanks for reply. I can no longer reproduce it. Must've been something with my test environment. Maybe related to the jest vscode extension.

[friederbluemle]

I also have a test case which is failing with 12.8.1 and passing with 12.8.0 - A bit unexpected that a patch increment can break something on the consumer side.

Simplified test looks like this - The screen makes an API call on first render, and then again after pull to refresh (using standard ScrollView/RefreshControl from RN. While the API call is in progress (async), a loading skeleton is shown on screen. As mentioned, up until 12.8.0 everything succeeds, but breaks with 12.8.1 - The API is never called, and loading-skeleton never shows up again (i.e. calling "onRefresh" with fireEvent no longer works as before).

  it('clears loading state after loading is done and pull to refresh', async () => {
    render(<TestScreen />);

    await waitForElementToBeRemoved(() => screen.getByTestId('loading-skeleton'));

    const scrollView = screen.getByTestId('scroll-view');
    fireEvent(scrollView.props.refreshControl, 'onRefresh');

    await waitForElementToBeRemoved(() => screen.getByTestId('loading-skeleton'));

    expect(Api.getFooBar).toHaveBeenCalledTimes(2);
  });

Is there a better way to interact with RefreshControl/onRefresh in a pull to refresh scenario? Any idea why 12.8.1 breaks this? Thank you!

[friederbluemle]

Ah interesting, after doing a bit more investigation, I found the root cause is related to those 3 added lines that check if the element is mounted:

function fireEvent(element: ReactTestInstance, eventName: EventName, ...data: unknown[]) {
  if (!isElementMounted(element)) {
    return;
  }
// ...

Maybe there is something wrong with the way I query the element? Why would scrollView.props.refreshControl not be mounted when I call fireEvent?

[mdjastrzebski]

Looks like you are trying to fire event on a prop (refresh control), which is a React.Element type (description of React component to be rendered),

Fire Event, and most other RNTL APIs expect to be called on ReactTestInstance, rendered React component.

As a workaround, you can try to either:

• assign your Refresh Control
  a test id, and use it to find the element
• manually call: refreshControl.onRefresh without using fire event

[friederbluemle]

Thanks for the quick reply @mdjastrzebski!

Fire Event, and most other RNTL APIs expect to be called on ReactTestInstance, rendered React component.

Yes, that makes sense.

Regarding the workarounds, assigning a testID also does not work, it is not able to find the element (I assume it is never "really" rendered).
But - workaround #2 does work! Calling onRefresh() directly via scrollView.props.refreshControl.props.onRefresh() makes the test pass like before.

Using the chained props access really doesn't look good which is why I would prefer fireEvent syntax. I understand it doesn't make sense to use it on an "unmounted" element. The next and real question is why is RefreshControl never rendered/mounted.

[friederbluemle]

So, is there more "RNTL way" to interact with RefreshControl and avoid accessing props directly?

[mdjastrzebski]

WIP #1822