enesozturk / react-native-hold-menu

šŸ“± A performant, easy to use hold to open context menu for React Native powered by Reanimated šŸš€
https://enesozturk.github.io/react-native-hold-menu/
MIT License
1.4k stars 91 forks source link

menu height is dynamic and needs to be reactive #109

Closed NoodleOfDeath closed 12 months ago

NoodleOfDeath commented 1 year ago

Summary

First off, love this library! I've learned a great deal about reanimated, and continuing to learn more!

A lot of the variables defined as "constants" really are not constants, because they can change randomly at runtime. This is especially important if the screen is rotated at the same time as launch, data populating the menu is asynchronous, or the variable values are loaded on demand in a flatlist. This particular PR fixes an in production issue, where the first item is not calculating the font scale correctly because a reactive hook is not used to capture the fontScale memoized property of Dimensions. The below videos indicate what this is fixing, although a lot of other reactive issues may still need to be addressed as they reveal themselves.

Without Fix

https://github.com/enesozturk/react-native-hold-menu/assets/14790443/3eb4d630-080c-485c-864a-457e0888a921

With Fix

https://github.com/enesozturk/react-native-hold-menu/assets/14790443/ec3af9b6-1338-4fb9-8b16-0231ecd56356

NoodleOfDeath commented 1 year ago

@kesha-antonov @ansh would love any code review on this as well! i currently use this in production so I'm highly motivated to fix bugs haha

NoodleOfDeath commented 1 year ago

@ansh I don't know why I read that as "let's go to the moon" šŸ˜‚ when i see this regularly at work

ansh commented 1 year ago

@enesozturk Please check it out and merge, thanks!

kesha-antonov commented 1 year ago

Looks good šŸ‘

NoodleOfDeath commented 1 year ago

@ansh actually looks like the issue is still occurring. I will investigate further before asking for this to be merged. The other PR i have open is ready though