Skip to content

feat(android): percentage support in translate #43193

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
intergalacticspacehighway wants to merge 14 commits into from
+46 −15
Closed

feat(android): percentage support in translate #43193

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
145a69d
android translate percentage support
intergalacticspacehighway Feb 19, 2024
42ebf7c
process transform update
intergalacticspacehighway Feb 24, 2024
e1f6c08
Merge branch 'main' into android-translate-percentage
intergalacticspacehighway Feb 24, 2024
e1948d9
Merge branch 'main' into android-translate-percentage
intergalacticspacehighway Feb 25, 2024
f25cf04
translate regex to support %
intergalacticspacehighway Feb 25, 2024
4faf27e
translate percentage test
intergalacticspacehighway Feb 25, 2024
91812cd
Update packages/react-native/Libraries/StyleSheet/processTransform.js
intergalacticspacehighway Mar 30, 2024
274ba79
flow check fix
intergalacticspacehighway Mar 31, 2024
f6a528c
prettier fix
intergalacticspacehighway Mar 31, 2024
eaee88b
fix flow check
intergalacticspacehighway Mar 31, 2024
55e2453
Merge branch 'main' into android-translate-percentage
intergalacticspacehighway May 20, 2024
6b1a151
handle NumberFormatException exception
intergalacticspacehighway May 22, 2024
bf8e03b
rename transforms array
intergalacticspacehighway May 22, 2024
5e70511
move add layout listener to after update transaction
intergalacticspacehighway May 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 4 additions & 7 deletions ...react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/BaseViewManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ protected T prepareToRecycleView(@NonNull ThemedReactContext reactContext, T vie
return view;
}

// Currently. onLayout listener is only attached when transform origin prop is being used.
// Currently, layout listener is only attached when transform or transformOrigin is set.
@Override
public void onLayoutChange(
View v,
Expand All @@ -171,7 +171,7 @@ public void onLayoutChange(
if ((currentHeight != oldHeight || currentWidth != oldWidth)) {
ReadableArray transformOrigin = (ReadableArray) v.getTag(R.id.transform_origin);
ReadableArray transformMatrix = (ReadableArray) v.getTag(R.id.transform);
if (transformMatrix != null && transformOrigin != null) {
if (transformMatrix != null || transformOrigin != null) {
Copy link
Contributor

@NickGerleman NickGerleman May 22, 2024

Choose a reason for hiding this comment

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

This isn't a matrix most of the time, right? Can we rename?

Copy link
Contributor Author

@intergalacticspacehighway intergalacticspacehighway May 22, 2024

Choose a reason for hiding this comment

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

Agree, renamed to transforms since it's an array.

setTransformProperty((T) v, transformMatrix, transformOrigin);
}
}
Expand All @@ -197,18 +197,15 @@ public void setFilter(@NonNull T view, @Nullable ReadableArray filter) {
public void setTransform(@NonNull T view, @Nullable ReadableArray matrix) {
view.setTag(R.id.transform, matrix);
view.setTag(R.id.invalidate_transform, true);
view.addOnLayoutChangeListener(this);
Copy link
Member

@javache javache Mar 4, 2024

Choose a reason for hiding this comment

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

We probably discussed this in the original transformOrigin PR, but why can't we override layout (or as Android prefers onLayout) to update the transform?

Copy link
Contributor Author

@intergalacticspacehighway intergalacticspacehighway Mar 30, 2024

Choose a reason for hiding this comment

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

Yeah i remember discussing it here - #38558 (comment). To override layout i think we'll have to move transform logic to ReactViewGroup.

onLayout runs during layout pass and can be used to update child view positions, in our case we update transform of the view. Not very ideal, i guess in the long run moving it to RootViewGroup might be better.

Copy link
Contributor

@NickGerleman NickGerleman May 22, 2024

Choose a reason for hiding this comment

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

I checked code, and Android will no-op if adding existing listener, but it's a little bit smelly that we can add ourselves as a listener, after we have already added ourselves as listener. Not sure a better way though (probably don't want to waste cycles to always attach).

}

@Override
@ReactProp(name = ViewProps.TRANSFORM_ORIGIN)
public void setTransformOrigin(@NonNull T view, @Nullable ReadableArray transformOrigin) {
view.setTag(R.id.transform_origin, transformOrigin);
view.setTag(R.id.invalidate_transform, true);
if (transformOrigin != null) {
view.addOnLayoutChangeListener(this);
} else {
view.removeOnLayoutChangeListener(this);
}
view.addOnLayoutChangeListener(this);
}

@Override
Expand Down
38 changes: 34 additions & 4 deletions ...react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/TransformHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,14 +103,36 @@ public static void processTransform(
MatrixMathHelper.applyScaleY(helperMatrix, transform.getDouble(transformType));
} else if ("translate".equals(transformType)) {
ReadableArray value = transform.getArray(transformType);
double x = value.getDouble(0);
double y = value.getDouble(1);
double x = 0;
if (value.getType(0) == ReadableType.String) {
x = parsePercentageToValue(value.getString(0), viewWidth);
} else {
x = value.getDouble(0);
}
double y = 0;
if (value.getType(1) == ReadableType.String) {
y = parsePercentageToValue(value.getString(1), viewHeight);
} else {
y = value.getDouble(1);
}
double z = value.size() > 2 ? value.getDouble(2) : 0d;
MatrixMathHelper.applyTranslate3D(helperMatrix, x, y, z);
} else if ("translateX".equals(transformType)) {
MatrixMathHelper.applyTranslate2D(helperMatrix, transform.getDouble(transformType), 0d);
double translateValue = 0;
if (transform.getType(transformType) == ReadableType.String) {
translateValue = parsePercentageToValue(transform.getString(transformType), viewWidth);
} else {
translateValue = transform.getDouble(transformType);
}
MatrixMathHelper.applyTranslate2D(helperMatrix, translateValue, 0d);
} else if ("translateY".equals(transformType)) {
MatrixMathHelper.applyTranslate2D(helperMatrix, 0d, transform.getDouble(transformType));
double translateValue = 0;
if (transform.getType(transformType) == ReadableType.String) {
translateValue = parsePercentageToValue(transform.getString(transformType), viewHeight);
} else {
translateValue = transform.getDouble(transformType);
}
MatrixMathHelper.applyTranslate2D(helperMatrix, 0d, translateValue);
} else if ("skewX".equals(transformType)) {
MatrixMathHelper.applySkewX(helperMatrix, convertToRadians(transform, transformType));
} else if ("skewY".equals(transformType)) {
Expand All @@ -130,6 +152,14 @@ public static void processTransform(
}
}

private static double parsePercentageToValue(String stringValue, double dimension) {
if (stringValue.endsWith("%")) {
Copy link
Contributor

@NickGerleman NickGerleman May 22, 2024

Choose a reason for hiding this comment

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

I'm not sure the old code was reliable here, but we usually want to avoid native crashes on invalid style values. So we probably want to be handling NumberFormatException somewhere.

Copy link
Contributor Author

@intergalacticspacehighway intergalacticspacehighway May 22, 2024

Choose a reason for hiding this comment

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

makes sense, updated!

double percentage = Double.parseDouble(stringValue.substring(0, stringValue.length() - 1));
return percentage * dimension / 100.0;
} else {
return Double.parseDouble(stringValue);
}
}
private static float[] getTranslateForTransformOrigin(
float viewWidth, float viewHeight, ReadableArray transformOrigin) {
if (transformOrigin == null || (viewHeight == 0 && viewWidth == 0)) {
Expand Down