feat: `userEvent.press` #1386

pierrezimmermannbam · 2023-04-13T11:20:27Z

Summary

With @MattAgn we've been working on the userEvent feature and this is a first proposition for an implementation of userEvent.press. It's not perfect yet but I'm pretty excited about it and it can be a support for discussion.

Unlike fireEvent, this uses only the host components. This has several benefits:

Allows better and easier simulation of real touch events. We trigger events (onResponderGrant, onResponderRelease) rather than calling props. For instance we just have to change the press duration to trigger a longPress instead of a press.
It's easier to work only with host components versus host & composite. For instance with the TextInputs we had several issues where it worked on host and not on composite or the opposite.
We're working on a lower level so we don't have to code React Native again for good simulations so this makes the implementation way simpler and I feel more maintainable as well.

Overall I feel this works really well and provides tons of advantages. It has also some drawbacks though:

Since real touch events involve time, the implementation relies on the usage of fake timers but I think this should be the recommended approach. An alternative would be to make the API asynchronous and just wait but we don't want longPresses to take 500ms. But this means that it will be harder to use this feature for some users. Also I think we'd need to check if fake timers are enabled and throw if that's not the case, which I haven't done yet.
The implementation can be a bit complex since it requires to dig into the react native code to see what happens at host component level. It should be easier to maintain though and if it provides users with a better abstraction I feel it's a good compromise to make.

Regarding the option pressDuration, I was thinking initially of making a userEvent.longPress API but since the delayLongPress can be modified by the user, it would have required to look at a composite components props, so I figured it would make more sense to think in terms of press duration rather than press / longPress.

For now this doesn't handle the touch events (for instance onTouchStart prop would not be called), this handles only press events because pressability and gestures are handled by different classes in React Native. It can very well be added later though.

If you think it's promising then in a second time we'll need to start discussing the strategy for release and user adoption.

Test plan

I added a bunch of test cases showcasing what props are called and how the pressDuration option works.

Curious to have your opinion on this @mdjastrzebski @MattAgn @thymikee @AugustinLF

mdjastrzebski · 2023-04-14T22:00:08Z

@pierrezimmermannbam, @MattAgn I'm really happy that you are working on this.

From reading the tests it seems that the current version of userEvent.press works with Pressables. Would it also work with other components that have press events, like Touchable* family, Text, etc? I think it's important for this API to be universal, so the user can safely invoke userEvent.press on any element where they could use fireEvent.press but with more realistic event simulation.

src/userEvent/touchEvents.ts

mdjastrzebski · 2023-04-14T22:03:28Z

src/userEvent/touchEvents.ts

+};
+
+export type PressOptions = {
+  pressDuration: number;


Q: What are the use cases for pressDuration prop?

The main use case is to simulate a long press. If press duration is greater or equal than the long press duration offset then it will call onLongPress and not onPress. You can look at the test cases for pressDuration, it should make things clearer

Hmmm, exposing pressDuration is looking like quite a low-level implementation detail. To use it properly it would require user to know the default press duration of Pressable. I think a simpler API would be to expose two events:

press: () => void, longPress: ({ duration = 500 }) => void

As for regular presses, which are far more popular than long press, the duration does not matter that much, and for longPresses most of the users would probably stick with the default pressDuration value. If they tweaked it, then they would also have option to apply the theaked value in test.

I don't agree with press duration being an implementation detail, it's directly related to the user action. In my opinion longPress will be interpreted like it triggers my onLongPress prop so I think it's more linked to implementation details. So I think it's a good addition to be able to change the duration, having an additional longPress api with a different default may be a good idea I'm not too sure on this

src/__tests__/userEvent/userEvent.press.test.tsx

src/helpers/isHostElementPointerEventEnabled.ts

src/__tests__/userEvent/userEvent.press.test.tsx

src/userEvent/touchEvents.ts

pierrezimmermannbam · 2023-04-20T10:06:44Z

From reading the tests it seems that the current version of userEvent.press works with Pressables. Would it also work with other components that have press events, like Touchable* family, Text, etc? I think it's important for this API to be universal, so the user can safely invoke userEvent.press on any element where they could use fireEvent.press but with more realistic event simulation.

This is based on the Pressability implementation in React Native so it should work with every components that is pressable, I'll make sure and add test cases for those

mdjastrzebski · 2023-04-20T14:53:24Z

re Pressability: I am really curious if this will work with other components out of the box. That would be the lucky option, as we would handle all types of components with single flow. In the unlucky case that they need a separate implementation, I think we should still support them, e.g. by checking the element type === Pressable and then going with pressable or basic flow (perhaps just invoking fireEvent.press, etc)

pierrezimmermannbam · 2023-04-21T19:54:11Z

The initial implementation works with every touchable/pressable but it doesn't work this Text or TextInput. The reason for that is that the implementation of Text is mocked in the React Native preset so it's just a component with name Text that has the same props than the composite one. It's not the same for pressables because the View is mocked but not pressable directly. It would work if only the native part of the Text component was mocked and I don't know why it's done this way, I'm gonna ask on the React Native repo if they know and maybe open a PR but that would be a huge breaking change and I don't know yet what impacts it could have so I'd need to try it on my projects to see how tests behave. In the meantime we can have a special treatment for Text and TextInput as implemented in this pr but it's a shame

pierrezimmermannbam · 2023-05-04T10:58:19Z

For the real timers :

I have added support for real timers as well
api is now async
users get a warning when using press or longPress with real timers
I have duplicated tests to have tests with both fake and real timers

There are some issues right now with the tests:

we are testing long presses with real timers so it takes a lot of time (~5s). We can't shorten that time I think
we have to duplicate the tests to run with both fake and real timers

I think the solution would be to have a jest config with two projects, one with fake timers enabled and another with real timers. This way every test would be run with both setups without having to duplicate them. Also we could have a command that would run only tests with fake or real timers so that while developing we're still able to have fast tests and run tests with real timers less frequently. If you think

For longPress, I have reworked the API as @mdjastrzebski suggested:

press does not have a duration option anymore (I let the default at 0 for now)
I added longPress api that has a default duration of 500ms and a duration option

For Text and TextInput I have done a workaround for now, I still need to ask on the React Native repo if they want to change or not the way those are mocked but it will take time and we'll still need support for earlier version of RN either way so the workaround will stay for a while

@mdjastrzebski @AugustinLF @MattAgn wdyt?

mdjastrzebski

Awesome job @pierrezimmermannbam I really like the direction where this is going. Supporting both advanced Pressables/etc, as well as simpler Text/etc is very useful.

The current version of the code, seems to use older User Event API (userEvent.press()), the latest, v14, User Event API is slightly different:

const user = userEvent.setup();
user.press()

While there are some pros of using the older version of the API, e.g. it's simpler, I think the way forward is to use the latest version of the API, in order to follow RTL's best practices and provide high degree of compatibility. It also has some useful features as configuring the User Event default params, etc.

I've been recently working on implementing User Event type() API, and I've got the userEvent.setup(), etc already implemented in #1396. Perhaps, I should extract that + some reusable utils like wait to a separate PR that could get reviewed and merged quickly so that we would be working on the same base API.

I am really happy how fast is the progress for User Event moving. Having press() and type() will cover the majority of the basic use cases.

src/userEvent/utils/warnAboutRealTimers.ts

src/userEvent/touchEvents.ts

src/helpers/isHostElementPointerEventEnabled.ts

mdjastrzebski

I've extracted the common User Event code in #1405. @pierrezimmermannbam pls take a look so that we have a common base API to add proper functions.

mdjastrzebski · 2023-05-10T12:03:01Z

I've merged #1405 to main, @pierrezimmermannbam pls rebase 🚀

… and onPressOut

mdjastrzebski · 2023-05-31T13:55:29Z

src/user-event/press/press.ts

+    }
+    await wait(config, options.duration);
+    if (onPressOut) {
+      if (DEFAULT_MIN_PRESS_DURATION - options.duration > 0) {


Is that 130 ms happening experimentally in an actual RN app for Text/TextInput? We should consider adding it artificially if this is the case.

Yes it happens also for Text and TextInputbecause they use the same implementation of Pressability. Not sure I understand your comment though, we're already adding it artificially here

Co-authored-by: Maciej Jastrzebski <mdjastrzebski@gmail.com>

pierrezimmermannbam · 2023-06-29T10:34:48Z

@mdjastrzebski I think I addressed all your comments, could you please review when you find the time?

mdjastrzebski · 2023-07-14T15:31:41Z

src/user-event/press/press.ts

+        await wait(config, DEFAULT_MIN_PRESS_DURATION - options.duration);
+      }
+      onPressOut(EventBuilder.Common.touch());
+    }


[high] Looks like we need an early return here, otherwise it will continue propagation to the host parent.

Oh you're right, good catch!

mdjastrzebski · 2023-07-14T15:42:35Z

src/user-event/press/press.ts

+      dispatchConfig: { registrationName: 'onResponderRelease' },
+    });
+
+    if (DEFAULT_MIN_PRESS_DURATION - options.duration > 0) {


[mid] q: why do we need to do additional waiting to reach at least min press duration limit here and in the Text/TextInput block above?

I've found this line in RN source. This seems to be an un-documented internal implementation detail of RN. I wonder if:

If it's already being applied so that we would not need to add manual wait

If not is it worth relying on such implementation details.

@pierrezimmermannbam wdyt?

What happens in RN code when the press is finished is:

the delay for the pressOut is computed: it's equal to minimum press duration - press duration(see source code

if the delay is > 0 then pressOut is programmed through a setTimeout with the delay computed at the above step (see source code)

In the implementation for Pressable, we wait because if the press duration is shorter than the minimum press duration, then onPressOuthas not been called yet. This is problematic for two reasons:

users would expect onPressOut to have been called

it results in an act warning

This is why we wait in this case and in the second scenario, we could just call onPressOut without waiting, it would just be less realistic. The upside of not waiting is that tests that don't use fake timers would be faster

Could you confirm that I get that right: the Pressable is actually delaying the action, so that we wait at least 130 ms, event when running under tests. If we do not wait to that 130 ms, then the event handler will be called outside of async act causing async act warning? So we are actually waiting to prevent async act warning from happening.

Yes that's correct. We're waiting both to prevent act warning but also to have onPressOut called synchronously so user doesn't have to wait for it

src/user-event/press/press.ts

src/user-event/press/utils/warnAboutRealTimers.test.tsx

src/user-event/press/utils/warnAboutRealTimers.ts

src/user-event/press/__tests__/press.real-timers.test.tsx

mdjastrzebski · 2023-07-14T15:52:42Z

src/user-event/press/utils/warnAboutRealTimers.test.tsx

+
+const mockConsoleWarn = jest.spyOn(console, 'warn').mockImplementation();
+
+test('logs a warning about usign real timers with user event', () => {


Testing this method seems redundant. In order to achieve code coverage it would be better to check if by calling userEvent.press with real times. That way would would at least check that it's being invoked from press() function. Here we are just confirming the logged message.

Yes I agree it's far from ideal, the reason I tested this separately is because I'm mocking here console warn to prevent warnings to appear in console when running tests and I didn't want to mock console.warn for every test in the userEvent.press tests. I did it a bit differently by putting the other tests in separate describe to have different mock just for them within the test file

mdjastrzebski

Awesome work @pierrezimmermannbam! Also thank you for patience with that, as due to large amount of work and holiday time I was not very responsive lately.

I wonder if we should simulate the min 130 ms press delay by ourselves. First of all it looks like it might already be applied for us by Pressability code in RN (could you check that?). Second of all, it might create additional hurdle for adoption of userEvent.press as users would be nagged to enable fake timers with warnings even for default zero duration presses. Personally, I would rather have easier adoption for the less advanced users rather than increased reality due to 130 ms delay.

@thymikee @AugustinLF @MattAgn perhaps you can give your outsider perspective on how you see that trade off.

…ing function

codecov · 2023-07-16T14:50:27Z

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.13 🎉

Comparison is base (8c22290) 96.88% compared to head (17a2a43) 97.02%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1386      +/-   ##
==========================================
+ Coverage   96.88%   97.02%   +0.13%     
==========================================
  Files          65       68       +3     
  Lines        3693     3862     +169     
  Branches      538      568      +30     
==========================================
+ Hits         3578     3747     +169     
  Misses        115      115

Impacted Files	Coverage Δ
src/fireEvent.ts	`100.00% <100.00%> (ø)`
src/helpers/pointer-events.ts	`100.00% <100.00%> (ø)`
src/test-utils/events.ts	`100.00% <100.00%> (ø)`
src/user-event/event-builder/common.ts	`100.00% <100.00%> (ø)`
src/user-event/index.ts	`100.00% <100.00%> (ø)`
src/user-event/press/constants.ts	`100.00% <100.00%> (ø)`
src/user-event/press/index.ts	`100.00% <100.00%> (ø)`
src/user-event/press/press.ts	`100.00% <100.00%> (ø)`
src/user-event/press/utils/warnAboutRealTimers.ts	`100.00% <100.00%> (ø)`
src/user-event/setup/setup.ts	`100.00% <100.00%> (ø)`
... and 1 more

... and 3 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

pierrezimmermannbam · 2023-07-16T17:58:36Z

Regarding the 130ms delay, it's not quite the same when pressing Text/TextInput and other Pressable like:

For Pressables, we don't directly call the onPressOut prop, we only call onResponderRelease and based on that RN uses a setTimeout to program the call to onPressOut. Here we don't really have the choice to wait I think. If we didn't, onPressOut would not be called synchronously which would be confusing imo and also the user would see an act warning if he doesn't have waitFor later on in his test.

For Text/TextInput it's a bit different. We call directly onPressOut so here we could skip the delay and call it directly. It would mean faster tests for real timers users but would make the simulation a bit less close to reality so here I'm not sure what the best tradeoff is.

However I think that the main use cases for press and longPress will be to target Pressable. For those there will be a delay because it's the way RN works so I would be suprised to see users adopt userEvent.press while using real timers anyway, especially given that we log a warning. I agree that it's important to make it easy to adopt but this feature cannot work properly with real timers, I mean it can it will just be slow but that's maybe even worse so we should not encourage users to use it with real timers imo

mdjastrzebski

Time to merge it! 🚀

mdjastrzebski · 2023-07-21T11:24:04Z

This PR has been released in v12.1.3 🚢
It is not added to package-level exports yet, waiting for userEvent.type() PR.
However you can use it by doing import { userEvent } from '@testing-library/react-native/build/user-event';

pierrezimmermannbam requested review from mdjastrzebski, MattAgn, AugustinLF and thymikee April 13, 2023 11:20

pierrezimmermannbam mentioned this pull request Apr 13, 2023

fireEvent cannot be triggered on elements with pointer events box only #1387

Closed