react-native-gesture-handler icon indicating copy to clipboard operation
react-native-gesture-handler copied to clipboard

Create gesturized pressable component

Open latekvo opened this issue 1 year ago • 29 comments

This PR adds our own Pressable component

Closes https://github.com/software-mansion/react-native-gesture-handler/issues/1221

latekvo avatar Jun 12 '24 15:06 latekvo

Original onBlur and onFocus: Pressable defaults to blurred state. When in blurred state, the first click on the pressable calls onFocus. When in focused state, when the user clicks outside of Pressable, onBlur is called and blurred state is set. Original onBlur and onFocus is only implemented on Web, and doesn't work on Android

latekvo avatar Jun 14 '24 09:06 latekvo

touchWithinBounds is still up for further investigation and i think i'll remove it all together in favour of dynamically changing hitSlop, if that'll be possible. onFocus and onBlur still have to be implemented, it may be tricky as the pointer would have to be tracked outside of Pressable, it's likely I'll have to remove it all together, which may not be a big deal, as previously it was a web specific feature anyways.

Other Pressable features seem to all be implemented, but I'll be testing their alignment with the original some more.

What's up for discussion is compatibility with legacy events. None of them are possible to fully recreate within rngh, and they weren't too consistent across web / mobile, but they could be partially imitated to better reflect those returned by the original callbacks.

latekvo avatar Jun 14 '24 13:06 latekvo

Pressable's onPress supposedly returns GestureResponderEvent, which looks something like this: image

And yet, when you actually inspect that event, it looks like this: image

latekvo avatar Jun 19 '24 10:06 latekvo

Last blocker: setting hitSlop doesn't have any effect on the pressable area.

latekvo avatar Jun 19 '24 12:06 latekvo

~~Could someone please confirm there is a crash on android with the standalone new_api example? It appears fontWeight style is causing it, but I still haven't ruled out that my particular android installation is somehow corrupted, which happened to me before :P~~

Edit: the android installation was corrupted 👍 + other issues

latekvo avatar Jun 20 '24 10:06 latekvo

State of things

  • on android, pressable is missing the feature to activate onPressIn, when an already pressed mouse/finger slides over the component
  • on ios, same as android ~~both ManualGesture's and NativeGesture's hitSlop setting behaves differently than on android, only making it extend a touch or click that has already been placed, but doesn't extend the clickable area itself~~
  • on web, I won't be implementing either hitSlop or retentionOffset

latekvo avatar Jun 20 '24 13:06 latekvo

I see no further issues, and all bahaviours align with the original. I'll work on further testing, reduction of code, and polishing the adapted event, but as a whole, it's ready for review.

latekvo avatar Jun 21 '24 08:06 latekvo

for the web, we cannot use RNButton, and have to replace it with RectButton instead, but that doesn't infer on any functionality as far as I see.

latekvo avatar Jun 21 '24 08:06 latekvo

I am aware that the tests have strange styling on web, but I will take care of that after I finish writing them.

latekvo avatar Jun 21 '24 11:06 latekvo

Screenshot 2024-06-21 at 15 09 16

This is up for investigation, first of all, kotlin/native side interface doesn't align with the react typing, secondly the number I am providing doesn't seem to align with any kind of rgb standard, and is likely something else. Converting from hex 0xRRGGBB to int won't get the desired result, neither will 0xRGB. The random number that is present there is just a one that works at all, to showcase what is happening.

latekvo avatar Jun 21 '24 13:06 latekvo

I still need to fix styling for examples on web, and fix pressDelay on web, it's not being cancelled on touch up.

latekvo avatar Jun 24 '24 14:06 latekvo

For targetId, we could use a hidden variable _nativeTag located on View, which in our case is pressableRef. _nativeTag is not publicly exposed but could be used. It does seem to yield the correct target id, but it doesn't work on web and changes on every render. I'm not sure if it's an approach we want to be using.

latekvo avatar Jun 24 '24 14:06 latekvo

Regarding the border example, do we want to nest another View inside the RNButton to allow for some broken styles to be more aligned with the original? (c.c. @j-piasecki )

Screenshot 2024-06-26 at 09 31 41 Screenshot 2024-06-26 at 09 32 52

latekvo avatar Jun 26 '24 07:06 latekvo

Regarding the border example, do we want to nest another View inside the RNButton to allow for some broken styles to be more aligned with the original? (c.c. @j-piasecki )

Possibly, though It's something I'd like to avoid, given it's similar to Touchables from GH and there were many issues regarding that.

j-piasecki avatar Jun 26 '24 08:06 j-piasecki

I think we can try the approach with an additional view as a parent of the button. You would probably style the view and simply pass StyleSheet.absoluteFill as the style to the button. You may need to split the other props in some wacky way.

j-piasecki avatar Jun 26 '24 13:06 j-piasecki

I think we can try the approach with an additional view as a parent of the button. You would probably style the view and simply pass StyleSheet.absoluteFill as the style to the button. You may need to split the other props in some wacky way.

~~Hey just to clarify, in the attached screenshot, View was the inner most child, and it inherited all the styles, removing them from the button entirely. Everything else adjusted perfectly automatically.~~

~~Is there a reason why you'd prefer the View to be the topmost parent, or is it of no importance?~~

~~For now I'll commit the component with the additional View as it's child, but please let me know if this is what you wanted.~~

edit: Having the View as the inner child breaks ripple :P ~~Will try around a bit more, but I think it's likely~~ Your approach works better.

latekvo avatar Jun 26 '24 14:06 latekvo

Even if borderStyle causes the issue in Pressable (and I've mentioned it in bordersExample.tsx), I don't understand why do we need this comment here. In the separator styles work fine (though they look different when comparing platforms)

This comment refers to borderWidth, which is set to an arbitrary number. IOS does allow for dotted/slashed borders, just refuses to render the entire object, when any 1-side styles are set to any of the sides of the border, which is also dotted/slashed on any of the sides.

Because of this, we have to set the border width to a really small number, so that all the borders are squashed into a single line, while also making them thick enough, so that the slashed effect does not look like little dots, as it's look depends on border's width.

latekvo avatar Jun 28 '24 09:06 latekvo

Okay, after I've stumbled upon

The key here is borderStyle, it's the only thing causing issue on IOS.

I assumed that the problem was just borderStyle and borderWidth has nothing to do with it.

m-bert avatar Jun 28 '24 09:06 m-bert

When delay hover in/out is set, shouldn't it be canceled when moving outside of the view?

That's a good point. I have no idea how hover delay behaves on the legacy component, but I think that suggestion makes a bit more sense than the current implementation. I'll do that, shouldn't be a problem at all.

latekvo avatar Jul 02 '24 08:07 latekvo

When delay hover in/out is set, shouldn't it be canceled when moving outside of the view?

I think that if you hover on Pressable that has hoverDelayIn and then you move outside it before callback will be triggered, then it shouldn't be called at all (i.e. it should be cancelled).

m-bert avatar Jul 02 '24 08:07 m-bert

hover with delay tests:

https://github.com/software-mansion/react-native-gesture-handler/assets/74246391/da3c6f5a-9fdf-46d8-b79f-9f5b0dfbe5fa

latekvo avatar Jul 02 '24 10:07 latekvo

I've found 2 more problems:

Functional styling (all platforms)

If you drag out of Pressable and return to it, next press doesn't work

https://github.com/software-mansion/react-native-gesture-handler/assets/63123542/16fe1fb3-05fb-4902-84e3-a28d1aec11ae

Delay on press (web only)

Sometimes press stops working

https://github.com/software-mansion/react-native-gesture-handler/assets/63123542/b91046c1-bbef-4e2f-852c-7ec57466ad5f

Conclusion

I think we've done a lot in this PR and further issues may be addressed in following PRs. What's your stance on that, @j-piasecki?

m-bert avatar Jul 02 '24 13:07 m-bert

Also I've noticed that if you use stylus and you click on "delay hover" example, when you lift stylus you will get Hover out with delay registered, without hover in being logged.

And last thing for now, we still have problems with borders on iOS:

image

(...) I fixed this error since, so I think I can remove the borders example all together, as all the styles are now aligned with the legacy pressable.

Please, either remove the example or change borders to solid

m-bert avatar Jul 02 '24 13:07 m-bert

Functional styling (all platforms)

fixed, it was an issue with touch cancelling

Delay on press (web only)

couldn't replicate it, could you tell me which browser you're using?

Problems with borders on iOS

removed them

(c.c. @m-bert )

latekvo avatar Jul 03 '24 07:07 latekvo

Regarding the hover in/out bug on web browser with stylus, I think what you described is normal behaviour.

When just clicking (press in immediately followed by press out), well then before hoverIn's timer finishes (press in), hoverOut's is started (press out), thus only the latter will be displayed.

If you were to hold down the hover element, you'd also see Hovered in with delay.

This is in line with what you requested yesterday:

image

I hope this explains the issue you've found.

latekvo avatar Jul 03 '24 08:07 latekvo

couldn't replicate it, could you tell me which browser you're using?

I'm using Google Chrome (126.0.6478.63).

I hope this explains the issue you've found.

Makes sense 😅

m-bert avatar Jul 03 '24 09:07 m-bert

I think this is yet another bug 😅 But as I said earlier, I'm not sure if we want to fix it in this PR

https://github.com/software-mansion/react-native-gesture-handler/assets/63123542/e6bfbf42-6558-467b-a050-39e2627ef3cd

m-bert avatar Jul 03 '24 09:07 m-bert

I think we've done a lot in this PR and further issues may be addressed in the following PRs.

We can do that. Could you open issues for the problems you've found in that case?

j-piasecki avatar Jul 03 '24 10:07 j-piasecki

Could you open issues for the problems you've found in that case?

I will.

m-bert avatar Jul 03 '24 10:07 m-bert