Skip to content

Commit 7dc8dec

Browse files
authored
[Android] Apply the button's managed handler config once per prop transaction (#4357)
## Description Each prop setter on `RNGestureHandlerButtonViewManager` created or reconfigured the button's managed `NativeViewGestureHandler` on its own. That meant, per prop: a module lookup, a fresh config map, and a registry lookup — and since React Native does not guarantee prop order, `handlerTag` could be applied *after* the props meant to configure the handler, so those updates were silently dropped. Prop setters now only cache their value on the view and flag it dirty; a single `updateManagedHandler` pass runs from `onAfterUpdateTransaction` and creates, reconfigures or drops the handler from the cached state. Other changes this required: - The module is resolved by `moduleId` through a new `RNGestureHandlerModule.getModule` (a `WeakReference` map cleaned up on `invalidate`, and on a dangling read) rather than from the view's context, so `moduleId` participates in the same batched update. - A `handlerTag` that resets to its default (removed prop) now drops the handler instead of leaving it attached. - Cached props are cleared in `onDropViewInstance`: views are recycled and props matching their default are not re-applied on the next mount, so stale values would otherwise configure — or create — a handler for the next button from the previous one's props. - The config is now written in full every time, including `null`s, so a removed `testID` or `hitSlop` actually clears on the handler. ## Test plan - Buttons in `expo-example` still press, cancel on leave and respect `hitSlop`/`testID`, including when those props are toggled or removed after mount. - Verified recycled buttons in a long list do not inherit the previous item's handler config. - Checked `enabled` changes still reach the handler.
1 parent f580090 commit 7dc8dec

2 files changed

Lines changed: 129 additions & 68 deletions

File tree

packages/react-native-gesture-handler/android/src/main/java/com/swmansion/gesturehandler/react/RNGestureHandlerButtonViewManager.kt

Lines changed: 110 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -69,91 +69,53 @@ class RNGestureHandlerButtonViewManager :
6969
public override fun createViewInstance(context: ThemedReactContext) = ButtonViewGroup(context)
7070

7171
override fun onDropViewInstance(view: ButtonViewGroup) {
72-
view.managedHandlerTag?.let { tag ->
73-
getGestureHandlerModule(view).dropGestureHandler(tag)
74-
}
72+
dropManagedHandler(view)
73+
74+
// The view may end up recycled, and props matching their default aren't re-applied on the next
75+
// mount — leaving the cached values behind would configure (or even create) a handler for the
76+
// next button from the previous one's props.
77+
view.pendingHandlerTag = null
78+
view.managedHandlerCancelOnLeave = null
79+
view.managedHandlerTestID = null
80+
view.managedHandlerHitSlop = null
81+
view.moduleId = null
82+
// Has to come last — every setter above flags the view as needing a managed handler update.
83+
view.managedHandlerNeedsUpdate = false
7584

7685
super.onDropViewInstance(view)
7786
}
7887

79-
private fun getGestureHandlerModule(view: ButtonViewGroup) =
80-
(view.context as ReactContext).getNativeModule(RNGestureHandlerModule::class.java)!!
81-
82-
private fun updateManagedHandlerConfig(view: ButtonViewGroup, config: ReadableMap) {
83-
view.managedHandlerTag?.let { getGestureHandlerModule(view).updateGestureHandlerConfig(it, config) }
84-
}
85-
8688
@ReactProp(name = "handlerTag")
8789
override fun setHandlerTag(view: ButtonViewGroup, handlerTag: Double) {
88-
if (view.managedHandlerTag == handlerTag) {
89-
return
90-
}
91-
92-
view.managedHandlerTag?.let { oldTag ->
93-
getGestureHandlerModule(view).dropGestureHandler(oldTag)
94-
}
95-
96-
view.managedHandlerTag = handlerTag
97-
val module = getGestureHandlerModule(view)
98-
99-
module.createGestureHandler(
100-
"NativeViewGestureHandler",
101-
handlerTag,
102-
Arguments.createMap().apply {
103-
putBoolean("shouldActivateOnStart", false)
104-
putBoolean("disallowInterruption", true)
105-
putBoolean("yieldsToContinuousGestures", true)
106-
putBoolean("enabled", view.isEnabled)
107-
view.managedHandlerCancelOnLeave?.let { putBoolean("shouldCancelWhenOutside", it) }
108-
view.managedHandlerTestID?.let { putString("testID", it) }
109-
view.managedHandlerHitSlop?.let { putMap("hitSlop", it) }
110-
},
111-
)
112-
module.attachGestureHandler(handlerTag, view.id.toDouble(), GestureHandler.ACTION_TYPE_NONE.toDouble())
90+
view.pendingHandlerTag = handlerTag
11391
}
11492

11593
@ReactProp(name = "cancelOnLeave")
11694
override fun setCancelOnLeave(view: ButtonViewGroup, cancelOnLeave: Boolean) {
11795
view.managedHandlerCancelOnLeave = cancelOnLeave
118-
updateManagedHandlerConfig(
119-
view,
120-
Arguments.createMap().apply {
121-
putBoolean("shouldCancelWhenOutside", cancelOnLeave)
122-
},
123-
)
12496
}
12597

12698
@ReactProp(name = "gestureTestID")
12799
override fun setGestureTestID(view: ButtonViewGroup, gestureTestID: String?) {
128100
view.managedHandlerTestID = gestureTestID
129-
updateManagedHandlerConfig(
130-
view,
131-
Arguments.createMap().apply {
132-
putString("testID", gestureTestID)
133-
},
134-
)
135101
}
136102

137103
@ReactProp(name = "gestureHitSlop")
138104
override fun setGestureHitSlop(view: ButtonViewGroup, gestureHitSlop: ReadableMap?) {
139105
view.managedHandlerHitSlop = gestureHitSlop
140-
updateManagedHandlerConfig(
141-
view,
142-
Arguments.createMap().apply {
143-
putMap("hitSlop", gestureHitSlop)
144-
},
145-
)
146106
}
147107

148108
@ReactProp(name = "hasLongPressHandler")
149109
override fun setHasLongPressHandler(view: ButtonViewGroup, hasLongPressHandler: Boolean) {
150110
view.hasLongPressHandler = hasLongPressHandler
151111
}
152112

153-
// No-op — only iOS needs the module id to resolve the handler manager, Android
154-
// gets the module from the view's context.
113+
// Cached like the other managed-handler props — prop order isn't guaranteed, so the module is
114+
// resolved from the id only once the transaction ends, in `updateManagedHandler`.
155115
@ReactProp(name = "moduleId")
156-
override fun setModuleId(view: ButtonViewGroup, moduleId: Int) = Unit
116+
override fun setModuleId(view: ButtonViewGroup, moduleId: Int) {
117+
view.moduleId = moduleId
118+
}
157119

158120
@ReactProp(name = "foreground")
159121
override fun setForeground(view: ButtonViewGroup, useDrawableOnForeground: Boolean) {
@@ -173,13 +135,6 @@ class RNGestureHandlerButtonViewManager :
173135
@ReactProp(name = "enabled")
174136
override fun setEnabled(view: ButtonViewGroup, enabled: Boolean) {
175137
view.isEnabled = enabled
176-
177-
updateManagedHandlerConfig(
178-
view,
179-
Arguments.createMap().apply {
180-
putBoolean("enabled", enabled)
181-
},
182-
)
183138
}
184139

185140
@ReactProp(name = "borderWidth")
@@ -467,9 +422,70 @@ class RNGestureHandlerButtonViewManager :
467422
}
468423
}
469424

425+
/**
426+
* Creates, reconfigures or drops the [NativeViewGestureHandler] the button manages, based on the
427+
* props cached on the view. Runs once per prop transaction rather than from the individual prop
428+
* setters: each of those would otherwise repeat the module lookup, build a config map and look
429+
* the handler up in the registry, and since prop order isn't guaranteed, `handlerTag` may be
430+
* applied after the props that configure the handler.
431+
*/
432+
private fun updateManagedHandler(view: ButtonViewGroup) {
433+
val moduleId = view.moduleId ?: return
434+
if (!view.managedHandlerNeedsUpdate) {
435+
return
436+
}
437+
438+
view.managedHandlerNeedsUpdate = false
439+
440+
// Handler tags start at 1, so anything below that means the prop was either never set or reset
441+
// to its default because it got removed.
442+
val handlerTag = view.pendingHandlerTag?.takeIf { it > 0 }
443+
444+
if (handlerTag == null) {
445+
dropManagedHandler(view)
446+
return
447+
}
448+
449+
val module = RNGestureHandlerModule.getModule(moduleId) ?: return
450+
451+
if (handlerTag != view.managedHandlerTag) {
452+
dropManagedHandler(view)
453+
454+
module.createGestureHandler("NativeViewGestureHandler", handlerTag, buildManagedHandlerConfig(view))
455+
module.attachGestureHandler(handlerTag, view.id.toDouble(), GestureHandler.ACTION_TYPE_NONE.toDouble())
456+
457+
view.managedHandlerTag = handlerTag
458+
return
459+
}
460+
461+
module.updateGestureHandlerConfig(handlerTag, buildManagedHandlerConfig(view))
462+
}
463+
464+
private fun dropManagedHandler(view: ButtonViewGroup) {
465+
val tag = view.managedHandlerTag ?: return
466+
// Cleared even if the drop below doesn't go through — the view must not keep a dangling tag.
467+
view.managedHandlerTag = null
468+
469+
val moduleId = view.moduleId ?: return
470+
RNGestureHandlerModule.getModule(moduleId)?.dropGestureHandler(tag)
471+
}
472+
473+
private fun buildManagedHandlerConfig(view: ButtonViewGroup): ReadableMap = Arguments.createMap().apply {
474+
putBoolean("shouldActivateOnStart", false)
475+
putBoolean("disallowInterruption", true)
476+
putBoolean("yieldsToContinuousGestures", true)
477+
putBoolean("enabled", view.isEnabled)
478+
// Written even when null so that a removed prop clears the one on the handler.
479+
putString("testID", view.managedHandlerTestID)
480+
putMap("hitSlop", view.managedHandlerHitSlop)
481+
482+
putBoolean("shouldCancelWhenOutside", view.managedHandlerCancelOnLeave ?: true)
483+
}
484+
470485
override fun onAfterUpdateTransaction(view: ButtonViewGroup) {
471486
super.onAfterUpdateTransaction(view)
472487

488+
updateManagedHandler(view)
473489
view.updateBackground()
474490
view.updateLongPressAccessibility()
475491
}
@@ -498,13 +514,27 @@ class RNGestureHandlerButtonViewManager :
498514

499515
var managedHandlerTag: Double? = null
500516

501-
// Config props may be applied before `handlerTag` (prop order is not guaranteed) and
502-
// `updateManagedHandlerConfig` no-ops until the handler exists, so the values are cached
503-
// here and seeded into the config when `setHandlerTag` creates the handler. This also
504-
// re-applies them when the handler is recreated for a new tag.
517+
var pendingHandlerTag: Double? = null
518+
set(tag) = withManagedHandlerUpdate {
519+
field = tag
520+
}
505521
var managedHandlerCancelOnLeave: Boolean? = null
522+
set(cancelOnLeave) = withManagedHandlerUpdate {
523+
field = cancelOnLeave
524+
}
506525
var managedHandlerTestID: String? = null
526+
set(testID) = withManagedHandlerUpdate {
527+
field = testID
528+
}
507529
var managedHandlerHitSlop: ReadableMap? = null
530+
set(hitSlop) = withManagedHandlerUpdate {
531+
field = hitSlop
532+
}
533+
var moduleId: Int? = null
534+
set(moduleId) = withManagedHandlerUpdate {
535+
field = moduleId
536+
}
537+
var managedHandlerNeedsUpdate = false
508538

509539
var exclusive = true
510540
var hasLongPressHandler = false
@@ -591,6 +621,11 @@ class RNGestureHandlerButtonViewManager :
591621
needBackgroundUpdate = true
592622
}
593623

624+
private inline fun withManagedHandlerUpdate(block: () -> Unit) {
625+
block()
626+
managedHandlerNeedsUpdate = true
627+
}
628+
594629
fun setOverflow(overflow: String?) {
595630
clipChildrenToShape = overflow == "hidden"
596631
invalidate()
@@ -1249,7 +1284,14 @@ class RNGestureHandlerButtonViewManager :
12491284
val changed = enabled != isEnabled
12501285
super.setEnabled(enabled)
12511286

1252-
if (changed && isHovered) {
1287+
if (!changed) {
1288+
return
1289+
}
1290+
1291+
// The managed handler mirrors the button's enabled state.
1292+
managedHandlerNeedsUpdate = true
1293+
1294+
if (isHovered) {
12531295
animateHoverState()
12541296
}
12551297
}

packages/react-native-gesture-handler/android/src/main/java/com/swmansion/gesturehandler/react/RNGestureHandlerModule.kt

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ import com.facebook.soloader.SoLoader
1414
import com.swmansion.gesturehandler.NativeRNGestureHandlerModuleSpec
1515
import com.swmansion.gesturehandler.core.GestureHandler
1616
import com.swmansion.gesturehandler.react.events.RNGestureHandlerEventDispatcher
17+
import java.lang.ref.WeakReference
18+
import java.util.concurrent.ConcurrentHashMap
1719

1820
@ReactModule(name = RNGestureHandlerModule.NAME)
1921
class RNGestureHandlerModule(reactContext: ReactApplicationContext?) :
@@ -34,6 +36,7 @@ class RNGestureHandlerModule(reactContext: ReactApplicationContext?) :
3436

3537
init {
3638
registries[moduleId] = RNGestureHandlerRegistry()
39+
modules[moduleId] = WeakReference(this)
3740
}
3841

3942
override fun getName() = NAME
@@ -182,6 +185,7 @@ class RNGestureHandlerModule(reactContext: ReactApplicationContext?) :
182185
}
183186
}
184187
registries.remove(moduleId)
188+
modules.remove(moduleId)
185189
invalidateNative()
186190
super.invalidate()
187191
}
@@ -203,6 +207,21 @@ class RNGestureHandlerModule(reactContext: ReactApplicationContext?) :
203207
private var nextModuleId = 0
204208
val registries: MutableMap<Int, RNGestureHandlerRegistry> = mutableMapOf()
205209

210+
private val modules: MutableMap<Int, WeakReference<RNGestureHandlerModule>> = ConcurrentHashMap()
211+
212+
fun getModule(moduleId: Int): RNGestureHandlerModule? {
213+
val module = modules[moduleId]?.get()
214+
215+
if (module == null) {
216+
// Either the module was never registered, or it was collected without `invalidate`
217+
// running — drop the dangling reference so the maps don't grow across reloads.
218+
modules.remove(moduleId)
219+
registries.remove(moduleId)
220+
}
221+
222+
return module
223+
}
224+
206225
init {
207226
SoLoader.loadLibrary("gesturehandler")
208227
}

0 commit comments

Comments
 (0)