-
Notifications
You must be signed in to change notification settings - Fork 62
Strip complicated but low priority features ahead of reanimated conversion
#149
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
base: master
Are you sure you want to change the base?
Conversation
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.
Comment @cursor review or bugbot run to trigger another review on this PR
| minZoom?: number; | ||
| doubleTapDelay?: number; | ||
| doubleTapZoomToCenter?: boolean; | ||
| bindToBorders?: boolean; |
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.
TypeScript types include removed prop causing silent failure
The panBoundaryPadding prop remains in the TypeScript interface definition, but the implementation has been completely removed (the applyPanBoundariesToOffset helper file was deleted and all related logic stripped out). TypeScript users can pass this prop without compilation errors, but it will be silently ignored at runtime. This causes unexpected behavior for users who set panBoundaryPadding expecting it to control pan boundaries.
Additional Locations (1)
elliottkember
left a comment
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.
I think this should be fine, but it's a breaking change so we'll need a minor version, and a CHANGELOG.md or some kind of breaking change notice in the release. I'll be interested to see whether anybody is using these features and has trouble upgrading
|
@elliottkember that's a good point. I've created this PR to create a better setup for changelog generation. We can edit the changelog (once it's generated) to explain further which features have been dropped once we publish. Additionally, we can also publish this as part of an RC version together with the reanimated stuff and ask the community for feedback before turning it into a official version |
reanimated conversion
|
@thomasttvo Sounds like a plan. It may actually be worth making a new major version RC. Given the scope of the reanimated change I think it's justified, and will avoid anybody accidentally updating their if they use |
Since we'll convert to
reanimatedin a later PR, it's important to strip some features to avoid a complicated migration. We can always reintroduce them in thereanimatedworld if we need to.Note
Remove pan-boundary/momentum and pin animation features, simplify offsets/gestures, and update API, docs, and builds accordingly.
applyPanBoundariesToOffsetand related animation helpers.bindToBorders,panBoundaryPadding,disableMomentum, andanimatePin; removepinAnimfromStaticPin; methods no longer acceptbindToBordersparams.useLatestCallbackhook.getZoomToAnimationin animations; add package type fields in build outputs.Written by Cursor Bugbot for commit c674720. Configure here.