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

fix(animations): make inner views animated only when necessary #117

Merged
merged 1 commit into from
Dec 29, 2022

Conversation

MorCohenAres
Copy link
Contributor

@MorCohenAres MorCohenAres commented Dec 27, 2022

If we turn off the swipeToDismiss functionality, the inner views (and the whole bottom sheet) are always animated, no matter what values change.
This is because of self.configuration property (which is configured to trigger an animation) always returning false for the equality function ==.
It seems like a mistake in the equality function, where it accidentally checks for both lhs.isSwipeToDismissEnabled and rhs.isSwipeToDismissEnabled instead of just comparing them to each other (and they return false because we turned off the swipeToDismiss functionality).

@lucaszischka
Copy link
Owner

Ok yes seems to be a copy paste error, that I never would have noticed / found. Thanks for submitting the pull request, I will merge it after the checks complete.

@lucaszischka lucaszischka merged commit c481324 into lucaszischka:main Dec 29, 2022
@MorCohenAres
Copy link
Contributor Author

MorCohenAres commented Dec 29, 2022

@lucaszischka Great! thanks.
Does the new commit need to be tagged or something to get its own version in CocoaPods?

@MorCohenAres MorCohenAres deleted the fix/animations branch December 29, 2022 15:58
@MorCohenAres MorCohenAres restored the fix/animations branch December 29, 2022 15:58
@lucaszischka
Copy link
Owner

Yes I would need to publish a new release. However as this is a really small fix, I won’t publish it now and wait for more changes. However you can use the SPM to add your package with branch set to main. Then you will be able tu use this fix without a new release - and don5 worry I am not using the main branch for development or testing; only finished code gets pushed to it.

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