Skip to content
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

chore: minor refactor #473

Merged
merged 3 commits into from
Jun 2, 2021
Merged

chore: minor refactor #473

merged 3 commits into from
Jun 2, 2021

Conversation

vonovak
Copy link
Contributor

@vonovak vonovak commented Jun 1, 2021

Please provide enough information so that others can review your pull request:

Motivation

  1. eslintrc : there is no need to pass rules to prettier, it will take them from prettierrc
  2. BottomSheetHandleContainer.tsx - useMemo is not needed for a boolean
  3. onScrollBeginDrag -> onScroll - the events listener should be passed under onScroll as documented in https://docs.swmansion.com/react-native-reanimated/docs/2.0.0-rc.2/api/useAnimatedScrollHandler/#returns
  4. useScrollableInternal.ts - removed a line that likely has no effect

Math.round - on android device I was getting some wild snap points like 123.999428435834 and figured it might be nicer to round them to integer numbers so that when there is some arithmetics done (eg adding some numbers and then comparing numbers via === can fail) it might be safer to do work with whole numbers.

tested locally with example app

@gorhom
Copy link
Owner

gorhom commented Jun 2, 2021

@vonovak could you run yarn lint --fix on the root project, and add the changes on this pr, thanks <3

@vonovak vonovak marked this pull request as draft June 2, 2021 21:48
@vonovak vonovak marked this pull request as ready for review June 2, 2021 21:54
@gorhom
Copy link
Owner

gorhom commented Jun 2, 2021

thanks @vonovak!

@gorhom gorhom merged commit e209ebe into gorhom:v4 Jun 2, 2021
@vonovak vonovak deleted the chore/minor-refactor branch June 2, 2021 22:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants