Skip to content

Commit 94cbf20

Browse files
Abbondanzofacebook-github-bot
authored andcommitted
Remove focus change listener and restore original when dropping view instance (facebook#52093)
Summary: Pull Request resolved: facebook#52093 This change updates the `BaseViewManager` implementation to drop and restore the original focus listener when a view instance has its `onDropViewInstance` method called. This is necessary to support view recycling, since the `addEventEmitters` method is called each time a recycled view is popped out of the stack. This would result in N+1 `onFocus`/`onBlur` calls for each time the view is recycled. Changelog: [Android][Fixed] - Remove focus change listener when dropping/recycling view instances Reviewed By: NickGerleman Differential Revision: D76852137 fbshipit-source-id: 9e980e7a1850a952baf04724bc251ff32186c6fa
1 parent 6bfc118 commit 94cbf20

3 files changed

Lines changed: 76 additions & 24 deletions

File tree

packages/react-native/ReactAndroid/api/ReactAndroid.api

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3315,6 +3315,7 @@ public abstract class com/facebook/react/uimanager/BaseViewManager : com/faceboo
33153315
public fun getExportedCustomBubblingEventTypeConstants ()Ljava/util/Map;
33163316
public fun getExportedCustomDirectEventTypeConstants ()Ljava/util/Map;
33173317
protected fun onAfterUpdateTransaction (Landroid/view/View;)V
3318+
public fun onDropViewInstance (Landroid/view/View;)V
33183319
public fun onLayoutChange (Landroid/view/View;IIIIIIII)V
33193320
protected fun prepareToRecycleView (Lcom/facebook/react/uimanager/ThemedReactContext;Landroid/view/View;)Landroid/view/View;
33203321
public fun setAccessibilityActions (Landroid/view/View;Lcom/facebook/react/bridge/ReadableArray;)V

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/BaseViewManager.java

Lines changed: 58 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -173,30 +173,19 @@ public BaseViewManager(@Nullable ReactApplicationContext reactContext) {
173173
protected void addEventEmitters(@NonNull ThemedReactContext reactContext, @NonNull T view) {
174174
super.addEventEmitters(reactContext, view);
175175

176-
@Nullable OnFocusChangeListener originalFocusChangeListener = view.getOnFocusChangeListener();
177-
view.setOnFocusChangeListener(
178-
(v, hasFocus) -> {
179-
if (originalFocusChangeListener != null) {
180-
originalFocusChangeListener.onFocusChange(v, hasFocus);
181-
}
182-
int surfaceId = UIManagerHelper.getSurfaceId(v.getContext());
183-
if (surfaceId == View.NO_ID) {
184-
return;
185-
}
186-
if (view.getContext() instanceof ThemedReactContext) {
187-
ThemedReactContext themedReactContext = (ThemedReactContext) v.getContext();
188-
@Nullable
189-
EventDispatcher eventDispatcher =
190-
UIManagerHelper.getEventDispatcherForReactTag(themedReactContext, view.getId());
191-
if (eventDispatcher != null) {
192-
if (hasFocus) {
193-
eventDispatcher.dispatchEvent(new FocusEvent(surfaceId, view.getId()));
194-
} else {
195-
eventDispatcher.dispatchEvent(new BlurEvent(surfaceId, view.getId()));
196-
}
197-
}
198-
}
199-
});
176+
BaseVMFocusChangeListener focusChangeListener =
177+
new BaseVMFocusChangeListener(view.getOnFocusChangeListener());
178+
focusChangeListener.attach(view);
179+
}
180+
181+
@Override
182+
public void onDropViewInstance(@NonNull T view) {
183+
super.onDropViewInstance(view);
184+
185+
@Nullable OnFocusChangeListener focusChangeListener = view.getOnFocusChangeListener();
186+
if (focusChangeListener instanceof BaseVMFocusChangeListener) {
187+
((BaseVMFocusChangeListener) focusChangeListener).detach(view);
188+
}
200189
}
201190

202191
// Currently, layout listener is only attached when transform or transformOrigin is set.
@@ -1052,4 +1041,49 @@ public void setTouchCancel(@NonNull T view, boolean value) {
10521041
}
10531042

10541043
// Please add new props to BaseViewManagerDelegate as well!
1044+
1045+
/**
1046+
* A helper class to keep track of the original focus change listener if one is set. This is
1047+
* especially helpful for views that are recycled so we can retain and restore the original
1048+
* listener upon recycling (onDropViewInstance).
1049+
*/
1050+
private class BaseVMFocusChangeListener<V extends View> implements OnFocusChangeListener {
1051+
private @Nullable OnFocusChangeListener mOriginalFocusChangeListener;
1052+
1053+
public BaseVMFocusChangeListener(@Nullable OnFocusChangeListener originalFocusChangeListener) {
1054+
mOriginalFocusChangeListener = originalFocusChangeListener;
1055+
}
1056+
1057+
public void attach(T view) {
1058+
view.setOnFocusChangeListener(this);
1059+
}
1060+
1061+
public void detach(T view) {
1062+
view.setOnFocusChangeListener(mOriginalFocusChangeListener);
1063+
}
1064+
1065+
@Override
1066+
public void onFocusChange(View view, boolean hasFocus) {
1067+
if (mOriginalFocusChangeListener != null) {
1068+
mOriginalFocusChangeListener.onFocusChange(view, hasFocus);
1069+
}
1070+
int surfaceId = UIManagerHelper.getSurfaceId(view.getContext());
1071+
if (surfaceId == View.NO_ID) {
1072+
return;
1073+
}
1074+
if (view.getContext() instanceof ThemedReactContext) {
1075+
ThemedReactContext themedReactContext = (ThemedReactContext) view.getContext();
1076+
@Nullable
1077+
EventDispatcher eventDispatcher =
1078+
UIManagerHelper.getEventDispatcherForReactTag(themedReactContext, view.getId());
1079+
if (eventDispatcher != null) {
1080+
if (hasFocus) {
1081+
eventDispatcher.dispatchEvent(new FocusEvent(surfaceId, view.getId()));
1082+
} else {
1083+
eventDispatcher.dispatchEvent(new BlurEvent(surfaceId, view.getId()));
1084+
}
1085+
}
1086+
}
1087+
}
1088+
}
10551089
}

packages/react-native/ReactAndroid/src/test/java/com/facebook/react/uimanager/BaseViewManagerTest.kt

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,4 +93,21 @@ class BaseViewManagerTest {
9393
view.onFocusChangeListener.onFocusChange(view, true)
9494
verify(originalFocusListener, times(1)).onFocusChange(view, true)
9595
}
96+
97+
@Test
98+
fun testDroppingViewInstanceRestoresFocusChangeListener() {
99+
val originalFocusListener = mock<OnFocusChangeListener>()
100+
view.onFocusChangeListener = originalFocusListener
101+
viewManager.addEventEmitters(themedReactContext, view)
102+
Assertions.assertThat(view.onFocusChangeListener).isNotEqualTo(originalFocusListener)
103+
104+
view.onFocusChangeListener.onFocusChange(view, true)
105+
verify(originalFocusListener, times(1)).onFocusChange(view, true)
106+
Assertions.assertThat(originalFocusListener).isNotEqualTo(view.onFocusChangeListener)
107+
108+
viewManager.onDropViewInstance(view)
109+
view.onFocusChangeListener.onFocusChange(view, true)
110+
verify(originalFocusListener, times(2)).onFocusChange(view, true)
111+
Assertions.assertThat(originalFocusListener).isEqualTo(view.onFocusChangeListener)
112+
}
96113
}

0 commit comments

Comments
 (0)