Skip to content
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.

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

fix: convertPointToView() to use "ti.ui.defaultunit" #12320

Merged
merged 4 commits into from
Dec 11, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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
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
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
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