Skip to content

Commit

Permalink
fix: convertPointToView() to use "ti.ui.defaultunit" (#12320)
Browse files Browse the repository at this point in the history 
* fix: convertPointToView() to use "ti.ui.defaultunit"

Fixes TIMOB-27807

* fix: convertPointToView() to support string values with units

* fix(android): typo in convertToPoint() error message

Co-authored-by: Gary Mathews <contact@garymathews.com>
  • Loading branch information
@jquick-axway @garymathews
jquick-axway and garymathews committed Dec 11, 2020
1 parent 4dd2a82 commit b4f6c3e
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 48 deletions.
45 changes: 20 additions & 25 deletions android/titanium/src/java/org/appcelerator/titanium/proxy/TiViewProxy.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1238,73 +1238,68 @@ public void setKeepScreenOn(boolean keepScreenOn)
@Kroll.method
public KrollDict convertPointToView(KrollDict point, TiViewProxy dest)
{
// Validate arguments.
if (point == null) {
throw new IllegalArgumentException("convertPointToView: point must not be null");
}

if (dest == null) {
throw new IllegalArgumentException("convertPointToView: destinationView must not be null");
}

if (!point.containsKey(TiC.PROPERTY_X)) {
throw new IllegalArgumentException("convertPointToView: required property \"x\" not found in point");
}

if (!point.containsKey(TiC.PROPERTY_Y)) {
throw new IllegalArgumentException("convertPointToView: required property \"y\" not found in point");
}

// The spec says to throw an exception if x or y cannot be converted to numbers.
// TiConvert does that automatically for us.
int x = TiConvert.toInt(point, TiC.PROPERTY_X);
int y = TiConvert.toInt(point, TiC.PROPERTY_Y);
// Fetch the given coordinate.
TiDimension dimensionX = TiConvert.toTiDimension(point, TiC.PROPERTY_X, TiDimension.TYPE_LEFT);
TiDimension dimensionY = TiConvert.toTiDimension(point, TiC.PROPERTY_Y, TiDimension.TYPE_TOP);
if (dimensionX == null) {
throw new IllegalArgumentException("convertPointToView: property \"x\" must be of type number or string");
}
if (dimensionY == null) {
throw new IllegalArgumentException("convertPointToView: property \"y\" must be of type number or string");
}

// Fetch the views to convert between.
TiUIView view = peekView();
TiUIView destView = dest.peekView();
if (view == null) {
Log.w(TAG, "convertPointToView: View has not been attached, cannot convert point");
return null;
}

if (destView == null) {
Log.w(TAG, "convertPointToView: DestinationView has not been attached, cannot convert point");
return null;
}

View nativeView = view.getNativeView();
View destNativeView = destView.getNativeView();
if (nativeView == null || nativeView.getParent() == null) {
Log.w(TAG, "convertPointToView: View has not been attached, cannot convert point");
return null;
}

if (destNativeView == null || destNativeView.getParent() == null) {
Log.w(TAG, "convertPointToView: DestinationView has not been attached, cannot convert point");
return null;
}

// Convert this view's point to the given view's coordinate space.
int[] viewLocation = new int[2];
int[] destLocation = new int[2];
nativeView.getLocationInWindow(viewLocation);
destNativeView.getLocationInWindow(destLocation);

if (Log.isDebugModeEnabled()) {
Log.d(TAG, "nativeView location in window, x: " + viewLocation[0] + ", y: " + viewLocation[1],
Log.DEBUG_MODE);
Log.d(TAG, "destNativeView location in window, x: " + destLocation[0] + ", y: " + destLocation[1],
Log.DEBUG_MODE);
}

int pointWindowX = viewLocation[0] + x;
int pointWindowY = viewLocation[1] + y;

// Apply reverse transformation to get the original location
float[] points = new float[] { pointWindowX - destLocation[0], pointWindowY - destLocation[1] };
float[] points = new float[] { viewLocation[0] - destLocation[0], viewLocation[1] - destLocation[1] };
points = destView.getPreTranslationValue(points);
points[0] += (float) dimensionX.getPixels(nativeView);
points[1] += (float) dimensionY.getPixels(nativeView);
dimensionX = new TiDimension((double) points[0], TiDimension.TYPE_LEFT);
dimensionY = new TiDimension((double) points[1], TiDimension.TYPE_TOP);

// Return converted point using the app's default units.
KrollDict destPoint = new KrollDict();
destPoint.put(TiC.PROPERTY_X, (int) points[0]);
destPoint.put(TiC.PROPERTY_Y, (int) points[1]);
destPoint.put(TiC.PROPERTY_X, dimensionX.getAsDefault(destNativeView));
destPoint.put(TiC.PROPERTY_Y, dimensionY.getAsDefault(destNativeView));
return destPoint;
}

Expand Down
17 changes: 5 additions & 12 deletions iphone/TitaniumKit/TitaniumKit/Sources/API/TiUtils.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -430,20 +430,13 @@ + (CGPoint)pointValue:(id)value valid:(BOOL *)isValid
}
return [value point];
} else if ([value isKindOfClass:[NSDictionary class]]) {
id xVal = [value objectForKey:@"x"];
id yVal = [value objectForKey:@"y"];
if (xVal && yVal) {
if (![xVal respondsToSelector:@selector(floatValue)] || ![yVal respondsToSelector:@selector(floatValue)]) {
if (isValid) {
*isValid = NO;
}
return CGPointMake(0.0, 0.0);
}

TiDimension xDimension = [self dimensionValue:@"x" properties:value];
TiDimension yDimension = [self dimensionValue:@"y" properties:value];
if (!TiDimensionIsUndefined(xDimension) && !TiDimensionIsUndefined(yDimension)) {
if (isValid) {
*isValid = YES;
}
return CGPointMake([xVal floatValue], [yVal floatValue]);
return CGPointMake(xDimension.value, yDimension.value);
}
}
if (isValid) {
Expand All @@ -463,7 +456,7 @@ + (CGPoint)pointValue:(id)value bounds:(CGRect)bounds defaultOffset:(CGPoint)def
yDimension = [value yDimension];
} else if ([value isKindOfClass:[NSDictionary class]]) {
xDimension = [self dimensionValue:@"x" properties:value];
yDimension = [self dimensionValue:@"x" properties:value];
yDimension = [self dimensionValue:@"y" properties:value];
} else {
xDimension = TiDimensionUndefined;
yDimension = TiDimensionUndefined;
Expand Down
11 changes: 7 additions & 4 deletions iphone/TitaniumKit/TitaniumKit/Sources/API/TiViewProxy.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -744,25 +744,28 @@ - (TiPoint *)convertPointToView:(id)args
ENSURE_ARG_AT_INDEX(arg1, args, 0, NSObject);
ENSURE_ARG_AT_INDEX(arg2, args, 1, TiViewProxy);
BOOL validPoint;
CGPoint oldPoint = [TiUtils pointValue:arg1 valid:&validPoint];
CGPoint givenPoint = [TiUtils pointValue:arg1 valid:&validPoint];
if (!validPoint) {
[self throwException:TiExceptionInvalidType subreason:@"Parameter is not convertable to a TiPoint" location:CODELOCATION];
}

__block BOOL validView = NO;
__block CGPoint p;
__block CGPoint pointOffsetDips;
TiThreadPerformOnMainThread(
^{
if ([self viewAttached] && self.view.window && [arg2 viewAttached] && arg2.view.window) {
validView = YES;
p = [self.view convertPoint:oldPoint toView:arg2.view];
pointOffsetDips = [self.view convertPoint:CGPointZero toView:arg2.view];
}
},
YES);
if (!validView) {
return (TiPoint *)[NSNull null];
}
return [[[TiPoint alloc] initWithPoint:p] autorelease];
TiPoint *tiPoint = [[TiPoint alloc] autorelease];
[tiPoint setX:NUMFLOAT(convertDipToDefaultUnit(pointOffsetDips.x + givenPoint.x))];
[tiPoint setY:NUMFLOAT(convertDipToDefaultUnit(pointOffsetDips.y + givenPoint.y))];
return tiPoint;
}

#pragma mark nonpublic accessors not related to Housecleaning
Expand Down
20 changes: 13 additions & 7 deletions tests/Resources/ti.ui.view.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -705,12 +705,7 @@ describe('Titanium.UI.View', function () {
win.open();
});

// FIXME: I think there's a parity issue here!
// Android returns x/y values as pixels *always*. while the input '100' uses the default unit (dip)
// which may vary based on screen density (Ti.Platform.DisplayCaps.ydpi) - so may be 100 or 200 pixels!
// But iOS *always* returns 123 for our value, so it must report the convertPointToView results in the default units too!
// So I think iOS always reports back dip values here and Android always reports back pixels
it.androidAndWindowsBroken('convertPointToView', function (finish) {
it.windowsBroken('convertPointToView', function (finish) {
win = Ti.UI.createWindow();
const a = Ti.UI.createView({ backgroundColor: 'red' });
const b = Ti.UI.createView({ top: '100', backgroundColor: 'blue' });
Expand All @@ -728,7 +723,18 @@ describe('Titanium.UI.View', function () {
should(result.x).be.a.Number(); // Windows: expected '123.000000' to be a number
should(result.y).be.a.Number();
should(result.x).eql(123);
should(result.y).eql(123); // Android sometimes gives 223? I assume this is a screen density thing?
should(result.y).eql(123);

result = b.convertPointToView({ x: '123', y: '23' }, a);
should(result.x).eql(123);
should(result.y).eql(123);

result = b.convertPointToView({
x: Ti.UI.convertUnits('123dp', 'px') + 'px',
y: Ti.UI.convertUnits('23dp', 'px') + 'px',
}, a);
should(result.x).eql(123);
should(result.y).eql(123);
} catch (err) {
return finish(err);
}
Expand Down

0 comments on commit b4f6c3e

Please sign in to comment.