-
-
Notifications
You must be signed in to change notification settings - Fork 1k
Migrate next
to main
#3303
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
Migrate next
to main
#3303
Conversation
## Description This PR deprecates `Touchable` components in our repo ## Test plan Try importing any `Touchable` (or its `props`). After importing look at the console. --------- Co-authored-by: Jakub Piasecki <[email protected]>
## Description #3260 introduced a regression to the `Gesture Handlers`'s `TouchableNativeFeedback` component. The component used to be a direct reference to the `TouchableNativeFeedback`, which has a [couple of static methods](https://reactnative.dev/docs/touchablenativefeedback#methods). When it was replaced with a function component, the static methods got removed, leaving only the render function of the `TouchableNativeFeedback`. ## Test plan See how the type errors in the `example/src/release_tests/touchables/index.tsx` are resolved.
## Description This PR deprecates old Gesture Handler API. ## Test plan Try importing any handler from old API.
Do you think it'd make sense to rebase these changes onto main instead of squashing them? |
In our discussion with @j-piasecki we decided that it would be better to |
Should we close this PR? |
Why is that? |
Just asking, since I understood you plan to either cherrypick those changes directly to main without a need for PR, or open a separate PR for all of these changes separately:
|
What I meant was that we decided to cherry-pick those commits into one PR (this one) and then merge it. At least this is what we planned to do back then 😅 |
That makes sense, should we merge it then? Also, what is this commit doing in this PR? It doesn't seem relevant to the rest of the cherry-picked PRs. Besides that change, I think all of these PRs have been already extensively tested individually. |
How does it not seem relevant? It is strictly connected to
I think so, maybe before next release (cc @j-piasecki) |
src/web/tools/VelocityTracker.ts
Outdated
@@ -84,7 +84,7 @@ export default class VelocityTracker { | |||
return null; | |||
} | |||
|
|||
public getVelocity(): [number, number] { | |||
public get velocity() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't this change the return type?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does to number[]
. Changed in b7b4eaa
We are before the next release 😄 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only one nit-pick
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🙈
Description
This PR migrates changed from next branch into
main
.Test plan
Most of those commits are deprecations, to check
get
/set
run example on web.