Enable hooking state on the stock Pressable component #863
Conversation
JasonVMo
requested review from
FalseLobster,
PPatBoyd,
Saadnajmi and
rurikoaraki
|
Thanks for this change @JasonVMo ! I was about to ask what I should do here now that we're on 0.63 and people want to use Pressable =) I'll review this when I can later today |
| onPress?: PressableProps['onPress']; | ||
|
|
||
| /** | ||
| * Duration (in addition to `delayPressIn`) after which a press gesture is | ||
| * considered a long press gesture. Defaults to 500 (milliseconds). | ||
| * Called when the press is activated to provide visual feedback. | ||
| */ | ||
| delayLongPress?: number; | ||
| onPressIn?: PressableProps['onPressIn']; | ||
|
|
||
| /** | ||
| * Duration to wait after press down before calling `onPressIn`. | ||
| * Called when the press is deactivated to undo visual feedback. | ||
| */ | ||
| delayPressIn?: number; | ||
| onPressOut?: PressableProps['onPressOut']; | ||
|
|
||
| /** | ||
| * Duration to wait after letting up before calling `onPressOut`. | ||
| * Called when a long press gesture has been triggered. | ||
| */ | ||
| delayPressOut?: number; | ||
| onLongPress?: PressableProps['onLongPress']; | ||
| }; |
There was a problem hiding this comment.
Why do we still need these extra props? Are they not in core?
There was a problem hiding this comment.
Think of the stock Pressable component as a thin shim around the Pressability library in react-native. The code I brought in before in the /Pressability directory was a port of that code to our project (and to TypeScript). I'm mostly leaving this part unchanged for now. Also PressabilityConfig is not quite the same as PressableProps, they intersect but aren't equal.
The long term plan would be:
- Get this change in
- Switch all components that use useAsPressable() on View components to usePressableState on Pressable components.
- Remove these old files along with the useAsPressable hook.
There was a problem hiding this comment.
The long term plan helps me understand the whole port of the Pressability library is going away, thank you. That explains to me why we still have these types.
| /** | ||
| * Called after the element is focused. | ||
| */ | ||
| onFocus?: (event: FocusEvent) => any; |
There was a problem hiding this comment.
Note: I think it's an open action item that we need to add onFocus/onBlur to RN-macOS pressable. I'm not sure how that affects FURN downstream.
There was a problem hiding this comment.
Likely the events never trigger but we should make sure that is on the 0.64 list.
| export type PressableHoverEventProps = { | ||
| /** | ||
| * While the user API is onHoverIn the View event is onMouseEnter | ||
| */ | ||
| onMouseEnter?: (event: MouseEvent) => any; | ||
|
|
||
| /** | ||
| * While the user API is onHoverOut the View event is onMouseLeave | ||
| */ | ||
| onMouseLeave?: (event: MouseEvent) => any; | ||
| }; |
There was a problem hiding this comment.
Where is this ViewEvent coming from? From RNW/win32/macOS?
There was a problem hiding this comment.
That's correct, though on 0.64 it may get an implementation on core at some point. The Pressability library has routing code for it, so the events are handled and work, this is simply the name of the events that get view implements.
| export type PressabilityConfig = Readonly< | ||
| PressablePressProps & | ||
| PressableFocusProps & | ||
| PressableHoverProps & { |
There was a problem hiding this comment.
Is this not a flow type that will not get read by typescript? That's where I thought ReadOnly<> decorators came from.
There was a problem hiding this comment.
Is there any chance of this config getting out of sync with RN-Core in a future version? I.E is this something we could just import from RN-Core?
There was a problem hiding this comment.
TypeScript has a Readonly concept as well. I put it in to PressabilityConfig when I did the port for consistency and am just staying consistent with what I had before here. In terms of getting out of sync, most of the types in here are not exposed and are on the inner Pressability library, not Pressable. As we get rid of the old code the interfaces should shift to be based off of what is exposed on PressableProps as much as possible.
| * Similarly the focus methods exist at the View level but are not exposed on pressable props in 0.63. As a result they | ||
| * need a similar treatment |
There was a problem hiding this comment.
Is this true for windows as well? I know it's not for macOS but I thought it was for windows?
3 participants
Platforms Impacted
Description of changes
When we were on a version <0.63 I ported a bunch of the Pressability and Pressable code from core react-native into our repo. This was done because the libraries were not yet available but actually worked fine against the earlier versions. Now that we have a baseline of >0.63 the Pressable component is now available.
The Pressable component will internally create a PressabilityConfig and do all the gesture management. It also has the android_ripple capabilities which we do not have. As such we should sunset much of the code. This change has a few parts:
Use Base Types (where possible)
This changes a bunch of the types to match what is exposed via
PressableProps. This is done to allow the helper hook functionsuseHoverHelper,useFocusHelper, andusePressHelperto work with either the existing code or the new hook function.**
usePressableStatehookThis is a new function that does the appropriate prop decoration to get the current state (and prompt state changes at the parent level) that
useAsPressabledoes. This one simply works against the stockPressablecomponent instead of aView. This leverages the fact that the stockPressableinternally will manage thePressabilityConfigobject.Verification
This was done as part of me reworking experimental button. (Hopefully a PR for that will be out later today). I split this out into a separate PR. I did this first though and switched experimental button to use a Pressable internally then verified it worked in the tester.
Pull request checklist
This PR has considered (when applicable):