Skip to content

Conversation

@j-piasecki
Copy link
Member

Description

All handler factories were stored in the RNGestureHandlerModule which always seemed like an odd place to keep them in to me. This PR moves each factory to be a nested class of the relevant gesture handler. This gives us private access to the handler inside the factory, which in turn allows to get rid of the setter methods and assign directly instead. Those were with use since RNGH was written in java, but we have Kotlin now. It's time to let go and use proper syntax.

It also adds default constants for each config property that didn't have it before.

Test plan

Example app


fun setMinNumberOfPointers(minNumberOfPointers: Int) = apply {
this.minNumberOfPointers = minNumberOfPointers
shouldCancelWhenOutside = DEFAULT_SHOULD_CANCEL_WHEN_OUTSIDE
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like shouldCancelWhenOutside was reset to false on every config update so the default in the initializer wasn't doing anything 🙈

this.disallowInterruption = disallowInterruption
shouldActivateOnStart = DEFAULT_SHOULD_ACTIVATE_ON_START
disallowInterruption = DEFAULT_DISALLOW_INTERRUPTION
shouldCancelWhenOutside = DEFAULT_SHOULD_CANCEL_WHEN_OUTSIDE
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same

numberOfPointersRequired = numberOfPointers
return this
maxDist = defaultMaxDist
shouldCancelWhenOutside = DEFAULT_SHOULD_CANCEL_WHEN_OUTSIDE
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same

@j-piasecki j-piasecki requested a review from m-bert April 17, 2025 11:58
@j-piasecki j-piasecki merged commit 8b717ae into main Apr 18, 2025
3 checks passed
@j-piasecki j-piasecki deleted the @jpiasecki/move-factories branch April 18, 2025 08:07
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.

3 participants