Skip to content

Commit 351de1b

Browse files
twasilczykfacebook-github-bot
authored andcommitted
RN: Overflow.fromString accepts null and returns non-null
Summary: Simplifies current and future uses of overflow fields. ReactViewGroup _overflow field is no longer nullable - it's default is now VISIBLE instead of null. The never-set or unrecognized case is no longer detectable, but it's fine - it will just default to VISIBLE, as all uses were already doing so. Differential Revision: D113186028
1 parent 0fc76bb commit 351de1b

10 files changed

Lines changed: 88 additions & 65 deletions

File tree

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/style/Overflow.kt

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -25,17 +25,18 @@ internal enum class Overflow {
2525
/**
2626
* Parses a string into an Overflow value.
2727
*
28-
* @param overflow The string value (case-insensitive)
29-
* @return The corresponding Overflow, or null if not recognized
28+
* @param overflow The string value (case-insensitive), or null
29+
* @param default The value to return when [overflow] is null or unrecognized
30+
* @return The corresponding Overflow, or [default]
3031
*/
3132
@JvmStatic
32-
fun fromString(overflow: String): Overflow? {
33-
return when (overflow.lowercase()) {
34-
"visible" -> VISIBLE
35-
"hidden" -> HIDDEN
36-
"scroll" -> SCROLL
37-
else -> null
38-
}
39-
}
33+
@JvmOverloads
34+
fun fromString(overflow: String?, default: Overflow = VISIBLE): Overflow =
35+
when (overflow?.lowercase()) {
36+
"visible" -> VISIBLE
37+
"hidden" -> HIDDEN
38+
"scroll" -> SCROLL
39+
else -> default
40+
}
4041
}
4142
}

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactHorizontalScrollView.kt

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -419,10 +419,11 @@ constructor(context: Context, private val fpsListener: FpsListener? = null) :
419419
if (overflow == null) {
420420
Overflow.SCROLL
421421
} else {
422-
Overflow.fromString(overflow)
423-
?: if (ReactNativeFeatureFlags.enablePropsUpdateReconciliationAndroid())
424-
Overflow.VISIBLE
425-
else Overflow.SCROLL
422+
Overflow.fromString(
423+
overflow,
424+
if (ReactNativeFeatureFlags.enablePropsUpdateReconciliationAndroid()) Overflow.VISIBLE
425+
else Overflow.SCROLL,
426+
)
426427
}
427428
invalidate()
428429
}

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactNestedScrollView.kt

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* This source code is licensed under the MIT license found in the
55
* LICENSE file in the root directory of this source tree.
66
*
7-
* @generated SignedSource<<cf60b52e15df339a179f392723007fab>>
7+
* @generated SignedSource<<1ad8f84ac759d8d225ce0fd57dccea7b>>
88
*/
99

1010
/**
@@ -387,10 +387,11 @@ constructor(context: Context, private val fpsListener: FpsListener? = null) :
387387
if (overflow == null) {
388388
Overflow.SCROLL
389389
} else {
390-
Overflow.fromString(overflow)
391-
?: if (ReactNativeFeatureFlags.enablePropsUpdateReconciliationAndroid())
392-
Overflow.VISIBLE
393-
else Overflow.SCROLL
390+
Overflow.fromString(
391+
overflow,
392+
if (ReactNativeFeatureFlags.enablePropsUpdateReconciliationAndroid()) Overflow.VISIBLE
393+
else Overflow.SCROLL,
394+
)
394395
}
395396
invalidate()
396397
}

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactNestedScrollViewManager.kt

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* This source code is licensed under the MIT license found in the
55
* LICENSE file in the root directory of this source tree.
66
*
7-
* @generated SignedSource<<d321b1f5f7a34b5717cb7ef5300dda96>>
7+
* @generated SignedSource<<543b6175d2ce5e4ec68f1625f0c04095>>
88
*/
99

1010
/**
@@ -450,17 +450,15 @@ constructor(private val fpsListener: FpsListener? = null) :
450450
public companion object {
451451
public const val REACT_CLASS: String = "RCTScrollView"
452452

453-
public fun createExportedCustomDirectEventTypeConstants(): Map<String, Any> =
454-
mapOf(
455-
getJSEventName(ScrollEventType.SCROLL) to mapOf("registrationName" to "onScroll"),
456-
getJSEventName(ScrollEventType.BEGIN_DRAG) to
457-
mapOf("registrationName" to "onScrollBeginDrag"),
458-
getJSEventName(ScrollEventType.END_DRAG) to
459-
mapOf("registrationName" to "onScrollEndDrag"),
460-
getJSEventName(ScrollEventType.MOMENTUM_BEGIN) to
461-
mapOf("registrationName" to "onMomentumScrollBegin"),
462-
getJSEventName(ScrollEventType.MOMENTUM_END) to
463-
mapOf("registrationName" to "onMomentumScrollEnd"),
464-
)
453+
public fun createExportedCustomDirectEventTypeConstants(): Map<String, Any> = mapOf(
454+
getJSEventName(ScrollEventType.SCROLL) to mapOf("registrationName" to "onScroll"),
455+
getJSEventName(ScrollEventType.BEGIN_DRAG) to
456+
mapOf("registrationName" to "onScrollBeginDrag"),
457+
getJSEventName(ScrollEventType.END_DRAG) to mapOf("registrationName" to "onScrollEndDrag"),
458+
getJSEventName(ScrollEventType.MOMENTUM_BEGIN) to
459+
mapOf("registrationName" to "onMomentumScrollBegin"),
460+
getJSEventName(ScrollEventType.MOMENTUM_END) to
461+
mapOf("registrationName" to "onMomentumScrollEnd"),
462+
)
465463
}
466464
}

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollView.kt

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -379,10 +379,11 @@ constructor(context: Context, private val fpsListener: FpsListener? = null) :
379379
if (overflow == null) {
380380
Overflow.SCROLL
381381
} else {
382-
Overflow.fromString(overflow)
383-
?: if (ReactNativeFeatureFlags.enablePropsUpdateReconciliationAndroid())
384-
Overflow.VISIBLE
385-
else Overflow.SCROLL
382+
Overflow.fromString(
383+
overflow,
384+
if (ReactNativeFeatureFlags.enablePropsUpdateReconciliationAndroid()) Overflow.VISIBLE
385+
else Overflow.SCROLL,
386+
)
386387
}
387388
invalidate()
388389
}

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/PreparedLayoutTextViewManager.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ internal class PreparedLayoutTextViewManager :
108108

109109
@ReactProp(name = "overflow")
110110
fun setOverflow(view: PreparedLayoutTextView, overflow: String?): Unit {
111-
view.overflow = overflow?.let { Overflow.fromString(it) } ?: Overflow.VISIBLE
111+
view.overflow = Overflow.fromString(overflow)
112112
}
113113

114114
@ReactProp(name = "accessible")

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextView.java

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -673,13 +673,7 @@ private void applyTextAttributes() {
673673
}
674674

675675
public void setOverflow(@Nullable String overflow) {
676-
if (overflow == null) {
677-
mOverflow = Overflow.VISIBLE;
678-
} else {
679-
@Nullable Overflow parsedOverflow = Overflow.fromString(overflow);
680-
mOverflow = parsedOverflow == null ? Overflow.VISIBLE : parsedOverflow;
681-
}
682-
676+
mOverflow = Overflow.fromString(overflow);
683677
invalidate();
684678
}
685679
}

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.kt

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1185,13 +1185,7 @@ public open class ReactEditText public constructor(context: Context) : AppCompat
11851185
}
11861186

11871187
public fun setOverflow(overflow: String?) {
1188-
if (overflow == null) {
1189-
this.overflow = Overflow.VISIBLE
1190-
} else {
1191-
val parsedOverflow = Overflow.fromString(overflow)
1192-
this.overflow = parsedOverflow ?: Overflow.VISIBLE
1193-
}
1194-
1188+
this.overflow = Overflow.fromString(overflow)
11951189
invalidate()
11961190
}
11971191

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewGroup.kt

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -818,22 +818,16 @@ public open class ReactViewGroup public constructor(context: Context?) :
818818
}
819819
}
820820

821-
private var _overflow: Overflow? = null
821+
private var _overflow: Overflow = Overflow.VISIBLE
822822
override var overflow: String?
823823
get() =
824824
when (_overflow) {
825825
Overflow.HIDDEN -> "hidden"
826826
Overflow.SCROLL -> "scroll"
827827
Overflow.VISIBLE -> "visible"
828-
else -> null
829828
}
830829
set(overflow) {
831-
_overflow =
832-
if (overflow == null) {
833-
Overflow.VISIBLE
834-
} else {
835-
Overflow.fromString(overflow)
836-
}
830+
_overflow = Overflow.fromString(overflow)
837831
invalidate()
838832
}
839833

@@ -846,9 +840,7 @@ public open class ReactViewGroup public constructor(context: Context?) :
846840
*/
847841
override fun getClipBounds(): Rect? {
848842
if (
849-
ReactNativeFeatureFlags.syncAndroidClipBoundsWithOverflow() &&
850-
_overflow != null &&
851-
_overflow != Overflow.VISIBLE
843+
ReactNativeFeatureFlags.syncAndroidClipBoundsWithOverflow() && _overflow != Overflow.VISIBLE
852844
) {
853845
val rect = Rect()
854846
getPaddingBoxRect(this, rect)
@@ -860,9 +852,7 @@ public open class ReactViewGroup public constructor(context: Context?) :
860852
/** See [getClipBounds]. */
861853
override fun getClipBounds(outRect: Rect): Boolean {
862854
if (
863-
ReactNativeFeatureFlags.syncAndroidClipBoundsWithOverflow() &&
864-
_overflow != null &&
865-
_overflow != Overflow.VISIBLE
855+
ReactNativeFeatureFlags.syncAndroidClipBoundsWithOverflow() && _overflow != Overflow.VISIBLE
866856
) {
867857
getPaddingBoxRect(this, outRect)
868858
return true
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
/*
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*/
7+
8+
package com.facebook.react.uimanager.style
9+
10+
import org.assertj.core.api.Assertions.assertThat
11+
import org.junit.Test
12+
13+
/** Tests for [Overflow.fromString] */
14+
class OverflowTest {
15+
@Test
16+
fun recognizedValuesAreParsedCaseInsensitively() {
17+
assertThat(Overflow.fromString("visible")).isEqualTo(Overflow.VISIBLE)
18+
assertThat(Overflow.fromString("hidden")).isEqualTo(Overflow.HIDDEN)
19+
assertThat(Overflow.fromString("scroll")).isEqualTo(Overflow.SCROLL)
20+
assertThat(Overflow.fromString("HiDdEn")).isEqualTo(Overflow.HIDDEN)
21+
}
22+
23+
@Test
24+
fun nullDefaultsToVisible() {
25+
assertThat(Overflow.fromString(null)).isEqualTo(Overflow.VISIBLE)
26+
}
27+
28+
@Test
29+
fun unrecognizedDefaultsToVisible() {
30+
assertThat(Overflow.fromString("bogus")).isEqualTo(Overflow.VISIBLE)
31+
}
32+
33+
@Test
34+
fun customDefaultHonoredForNullAndUnrecognized() {
35+
assertThat(Overflow.fromString(null, Overflow.SCROLL)).isEqualTo(Overflow.SCROLL)
36+
assertThat(Overflow.fromString("bogus", Overflow.SCROLL)).isEqualTo(Overflow.SCROLL)
37+
}
38+
39+
@Test
40+
fun customDefaultDoesNotOverrideRecognizedValues() {
41+
assertThat(Overflow.fromString("hidden", Overflow.SCROLL)).isEqualTo(Overflow.HIDDEN)
42+
}
43+
}

0 commit comments

Comments
 (0)