Skip to content

Incubator.Dialog - use new Gesture API + refactor #2409

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jan 5, 2023

Conversation

M-i-k-e-l
Copy link
Collaborator

Description

Incubator.Dialog - use new Gesture.Pan API and refactor to a "single" SharedValue.
You should probably look at the file as a whole and not the changes.

As discussed there are a few improvements here:

  1. Using the new Gesture.Pan from react-native-gesture-handler.
  2. Using a "single" SharedValue for animations (still needed an extra one for the initialTranslation).
  3. Not all improvements have been done, prominently: HiddenLocation - consider changing from useState to useSharedValue

Notes:

  1. I've left it all in one file, please let me know if you want to fragment it.
  2. I had to add a modalVisibility state in order to handle the user changing visible to false (for example pressing a Button \ ListItem that closes the Dialog.
  3. All animations are withTiming and with the default duration, we can change it later.
  4. I've tested on in a few places, I did not see any bugs we've had before.
  5. It looks like that onLayout is called even when the view is hidden (not sure if this is the new API or if I was wrong before), but it should make the opacity flicker a non-issue.

Changelog

Incubator.Dialog - use new Gesture API + refactor

@M-i-k-e-l M-i-k-e-l added the Important for Next Release PR that must be included in the release version label Jan 5, 2023
@M-i-k-e-l M-i-k-e-l requested a review from ethanshar January 5, 2023 08:41
@M-i-k-e-l M-i-k-e-l merged commit 14a5e70 into master Jan 5, 2023
@M-i-k-e-l M-i-k-e-l deleted the infra/incubator-dialog-use-new-api-and-refactor branch January 5, 2023 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Important for Next Release PR that must be included in the release version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants