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

Conversation

jquick-axway
Copy link
Contributor

@jquick-axway jquick-axway commented Dec 5, 2020

JIRA:
https://jira.appcelerator.org/browse/TIMOB-27807

Summary:

  • Fixes bug where convertPointToView() always returns pixels on Android.
  • Fixes bug where convertPointToView() always returns dips on iOS.
  • This method now returns the units set by "tiapp.xml" file's ti.ui.defaultunit". (Dips by default.)
  • Fixed convertPointToView() to accept x/y point string values with post-fixed units.

Test:

  1. Build and run the below code on Android.
  2. Attempt to drag one of the squares on screen.
  3. Verify square follows your finger. (Square may wobble on Android due to a separate issue.)
  4. Build for iOS and repeat steps 2 - 3.
  5. In the "tiapp.xml", set the following property to pixels.
    <property name="ti.ui.defaultunit" type="string">px</property>
  6. Build for Android and repeat steps 2 -3.
  7. Build for iOS and repeat steps 2 -3.

app.js

function createDragSquare(top, left, color) {
	var view = Ti.UI.createView({
		touchEnabled: true,
		backgroundColor: color,
		center: { x: left + 50, y: top + 50 },
		width: 100,
		height: 100,
	});
	view.addEventListener("touchmove", function(event) {
		view.center = view.convertPointToView({ x: event.x, y: event.y }, window);
		event.cancelBubble = true;
	});
	return view;
}

var window = Ti.UI.createWindow({ backgroundColor: "white" });
window.add(createDragSquare(40, 40, "red"));
window.add(createDragSquare(120, 120, "green"));
window.add(createDragSquare(200, 200, "blue"));
window.open();

@build
Copy link
Contributor

build commented Dec 5, 2020

Messages
📖 👍 Hey!, You deleted more code than you added. That's awesome!
📖

💾 Here's the generated SDK zipfile.

📖 ✊ The commits in this PR match our conventions! Feel free to Rebase and Merge this PR when ready.
📖

✅ All tests are passing
Nice one! All 15820 tests are passing.
(There are 993 skipped tests not included in that total)

Generated by 🚫 dangerJS against 387904e

Copy link
Contributor

@garymathews garymathews left a comment

Choose a reason for hiding this comment

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

This PR brings current behaviour inline with iOS. However, I brought up that Point can accept String parameters allowing units to be specified. @jquick-axway found that iOS also did not support this. So either we drop String support, or allow units to be specified.

@jquick-axway
Copy link
Contributor Author

Updated PR. Now supports x/y string value with units postfixed to it.

Copy link
Contributor

@garymathews garymathews left a comment

Choose a reason for hiding this comment

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

CR: PASS

Copy link
Contributor

@ssjsamir ssjsamir left a comment

Choose a reason for hiding this comment

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

FR Passed: Tested using the test case mentioned above. Note* As mentioned in the description the Android test case wobbles but this is a separate issue.

Test Environment

MacOS Big Sur: 11.1 Beta 1
Xcode: 12.2 Beta
Java Version: 1.8.0_242
Android NDK: 21.3.6528147
Node.js: 12.18.1
""NPM":"5.0.0","CLI":"8.1.1""
Pixel XL (10.0) Sim

@ssjsamir ssjsamir merged commit b4f6c3e into tidev:master Dec 11, 2020
@jquick-axway
Copy link
Contributor Author

@ssjsamir , the following PR will address that touch drag wobble issue.
#12340

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants