From b4f6c3ed76707ae96b244499acd0636559220412 Mon Sep 17 00:00:00 2001 From: Joshua Quick Date: Fri, 11 Dec 2020 05:04:19 -0800 Subject: [PATCH] fix: convertPointToView() to use "ti.ui.defaultunit" (#12320) * 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 --- .../titanium/proxy/TiViewProxy.java | 45 +++++++++---------- .../TitaniumKit/Sources/API/TiUtils.m | 17 +++---- .../TitaniumKit/Sources/API/TiViewProxy.m | 11 +++-- tests/Resources/ti.ui.view.test.js | 20 ++++++--- 4 files changed, 45 insertions(+), 48 deletions(-) diff --git a/android/titanium/src/java/org/appcelerator/titanium/proxy/TiViewProxy.java b/android/titanium/src/java/org/appcelerator/titanium/proxy/TiViewProxy.java index 38aa875c084..a84900e0d75 100644 --- a/android/titanium/src/java/org/appcelerator/titanium/proxy/TiViewProxy.java +++ b/android/titanium/src/java/org/appcelerator/titanium/proxy/TiViewProxy.java @@ -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; } diff --git a/iphone/TitaniumKit/TitaniumKit/Sources/API/TiUtils.m b/iphone/TitaniumKit/TitaniumKit/Sources/API/TiUtils.m index c5780d9b89f..e1487d36170 100644 --- a/iphone/TitaniumKit/TitaniumKit/Sources/API/TiUtils.m +++ b/iphone/TitaniumKit/TitaniumKit/Sources/API/TiUtils.m @@ -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) { @@ -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; diff --git a/iphone/TitaniumKit/TitaniumKit/Sources/API/TiViewProxy.m b/iphone/TitaniumKit/TitaniumKit/Sources/API/TiViewProxy.m index 73578e0268d..b1be91a0c77 100644 --- a/iphone/TitaniumKit/TitaniumKit/Sources/API/TiViewProxy.m +++ b/iphone/TitaniumKit/TitaniumKit/Sources/API/TiViewProxy.m @@ -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 diff --git a/tests/Resources/ti.ui.view.test.js b/tests/Resources/ti.ui.view.test.js index da756e6ea93..a6a0dcf1c50 100644 --- a/tests/Resources/ti.ui.view.test.js +++ b/tests/Resources/ti.ui.view.test.js @@ -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' }); @@ -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); }