Skip to content

Commit

Permalink
Change type of SwipeRefreshLayoutManager.size prop from Int to String
Browse files Browse the repository at this point in the history
Summary:
This diff changes the type of the SwipeRefreshLayoutManager.size prop from Int to String in Fabric.

The current implementation of this prop allows JS developers to use "int" type when fabric is enables and "int or string" types when using Fabric is disabled.
Since long term we want to only support "string" type for this prop, I'm changing the type of the prop to be String.

After my diff Fabric will start supporting only "string" types, non fabric screens will keep supporting "int or string" values.

**Will this break production?**
No, because there are no usages of RefreshControl.Size prop in fbsource

**What about if someone start using this prop next week?**
IMO It's very unlikely because of the nature of this prop, I will be monitoring next week and if there's an usage it will be detected by flow when trying to land D25933457.

Changelog: [Android][Changed] - RefreshControl.size prop changed its type to string, the valid values are: 'default' and 'large'

Reviewed By: JoshuaGross

Differential Revision: D25933458

fbshipit-source-id: 55067d7405b063f1e8d9bb7a5fd7731f5f168960
  • Loading branch information
mdvacca authored and facebook-github-bot committed Jan 17, 2021
1 parent 69b3016 commit 65975dd
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -42,16 +42,8 @@ type NativeProps = $ReadOnly<{|
progressBackgroundColor?: ?ColorValue,
/**
* Size of the refresh indicator, see RefreshControl.SIZE.
*
* This type isn't currently accurate. It really is specific numbers
* hard coded in the Android platform.
*
* Also, 1 isn't actually a safe default. We are able to set this here
* because native code isn't currently consuming the generated artifact.
* This will end up being
* size?: WithDefault<'default' | 'large', 'default'>,
*/
size?: WithDefault<Int32, 1>,
size?: WithDefault<'default' | 'large', 'default'>,
/**
* Progress view top offset
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,21 @@ public void setProgressBackgroundColor(ReactSwipeRefreshLayout view, Integer col
}

// TODO(T46143833): Remove this method once the 'size' prop has been migrated to String in JS.
@Override
public void setSize(ReactSwipeRefreshLayout view, int value) {
view.setSize(value);
}

@Override
public void setSize(ReactSwipeRefreshLayout view, String size) {
if (size == null || size.equals("default")) {
view.setSize(SwipeRefreshLayout.DEFAULT);
} else if (size.equals("large")) {
view.setSize(SwipeRefreshLayout.LARGE);
} else {
throw new IllegalArgumentException("Size must be 'default' or 'large', received: " + size);
}
}

// This prop temporarily takes both 0 and 1 as well as 'default' and 'large'.
// 0 and 1 are deprecated and will be removed in a future release.
// See T46143833
Expand All @@ -103,15 +113,7 @@ public void setSize(ReactSwipeRefreshLayout view, Dynamic size) {
} else if (size.getType() == ReadableType.Number) {
view.setSize(size.asInt());
} else if (size.getType() == ReadableType.String) {
final String sizeStr = size.asString();
if (sizeStr.equals("default")) {
view.setSize(SwipeRefreshLayout.DEFAULT);
} else if (sizeStr.equals("large")) {
view.setSize(SwipeRefreshLayout.LARGE);
} else {
throw new IllegalArgumentException(
"Size must be 'default' or 'large', received: " + sizeStr);
}
setSize(view, size.asString());
} else {
throw new IllegalArgumentException("Size must be 'default' or 'large'");
}
Expand Down

0 comments on commit 65975dd

Please sign in to comment.