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

[TIMOB-19878] Orientation Change is not correctly detected on Android #9525

Closed
wants to merge 4 commits into from

Conversation

maggieaxway
Copy link
Contributor

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

Test case:

var win = Ti.UI.createWindow();
win.backgroundColor= 'red';
win.layout = 'vertical';

var label1 = Ti.UI.createLabel({
  color: 'white',
  font: { fontSize:48 },
  text: 'orientation',
  textAlign: Ti.UI.TEXT_ALIGNMENT_CENTER,
  top: 30,
  width: Ti.UI.SIZE, height: Ti.UI.SIZE
});

var label2 = Ti.UI.createLabel({
  color:'white',
  font: { fontSize:48 },
  text: 'orientation number',
  textAlign: Ti.UI.TEXT_ALIGNMENT_CENTER,
  top: 30,
  width: Ti.UI.SIZE, height: Ti.UI.SIZE
});

win.add(label1);
win.add(label2);

Ti.Gesture.addEventListener('orientationchange', gestureChange);

function gestureChange(e) {
    console.log('orientation change...');
    console.log(e);
    label1.text = e.source.landscape ? 'landscape' : 'portrait';
    label2.text = 'orientation = ' + e.source.orientation;
}
win.open();

@sschueller
Copy link

Thank you for fixing this.

@jquick-axway
Copy link
Contributor

@maggieaxway, @garymathews,

This change collides with TIMOB-24537 which we haven't done yet. The issue here is that there is a platform parity issue. The orientation returned by iOS and Windows Phone is the device orientation, but Android returns the app's orientation. This is something we document, but we were planning on changing in Titanium 7.0.0 since it is a breaking change. Providing an app orientation is not that useful to a developer because they can already determine this by comparing the window's width/height and listening to the Window's "postlayout" event. But as it stands now, developers have no means of detecting device orientation on Android.

What really needs to be changed is our TiBaseActivity OrientationEventListener related code here...
https://github.com/appcelerator/titanium_mobile/blob/master/android/titanium/src/java/org/appcelerator/titanium/TiBaseActivity.java#L711

@jquick-axway
Copy link
Contributor

jquick-axway commented Oct 12, 2017

Also note that we need to test on a large Android tablet since these devices typically have an "upright" orientation of "landscape". Meaning that the Java ROTATION_0 constant will be relative to landscape instead of portrait like it normally is on a phone.

Edit:
I have confirmed that the following devices have an upright/natural orientation of landscape. (Note that they need to be physical devices. The Android emulator cannot replicate this behavior.)

  • Samsung Nexus 10
  • Samsung Galaxy Tab 3

@jquick-axway
Copy link
Contributor

We also need to test split-screen mode on Android 7 or higher. The reason the old TiOrientationHelper.convertRotationToTiOrientationMode() method wrongly assumes the upright orientation based on the given width/height of the window. This method won't work for split-screen mode. For example, if you do split-screen while holding the device "landscape", both apps will be displayed in "portrait" form side-by-side.

}

}

Copy link
Contributor

Choose a reason for hiding this comment

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

We should revert the TiOrientationHelper code back to what it was originally because we still need the width/height check for large Android tablets whose ROTATION_0 is landscape. We also should not use orientation and isPublic globals.

So, revert this file and add the following new function...

public static int getWindowTiOrientationModeFrom(Activity activty)
{
	int orientation = ORIENTATION_UNKNOWN;
	if (activity != null) {
		WindowManager windowManager = activity.getWindowManager();
		if (windowManager != null) {
			Display display = windowManager.getDefaultDisplay();
			if (display != null) {
				DisplayMetrics dm = new DisplayMetrics();
				display.getMetrics(dm);
				orientation = convertRotationToTiOrientationMode(
						display.getRotation(), dm.widthPixels, dm.heightPixels);
			}
		}
	}
	return orientation;
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Also note that I call the new method getWindowTiOrientationModeFrom() to specify that it's fetching the orientation of the application window. In 7.0.0, we'll look into fetching device orientation, but it may still be useful to know the window orientation as well for our own internal purposes.

int width = dm.widthPixels;
int height = dm.heightPixels;
return TiOrientationHelper.convertRotationToTiOrientationMode(display.getRotation(), width, height);
return TiOrientationHelper.orientation;
Copy link
Contributor

Choose a reason for hiding this comment

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

Change the isPortrait(), isLandscape(), getPortrait(), getLandscape(), and getOrientation() methods to use the new TiOrientationHelper.getWindowTiOrientationModeFrom() method I've suggested in my comments below. We want to calculate it everytime and not depend on external code to do it for us via globals.

You're definitely correct in removing usage of the getConfiguration() method call since that will fail to report orientation changes between "landscapeLeft" -> "landscapeRight" and vice-versa (this is by Google's design).

So, a call to isPortrait() should look like this...

public boolean isPortrait()
{
	Activity activity = TiApplication.getAppRootOrCurrentActivity();
	int orientation = TiOrientationHelper.getWindowTiOrientationModeFrom(activity);
	return (orientation == ORIENTATION_PORTRAIT) || (orientation == ORIENTATION_PORTRAIT_REVERSE);
}

And a call to isLandscape() should look something like this...

public boolean isLandscape()
{
	Activity activity = TiApplication.getAppRootOrCurrentActivity();
	int orientation = TiOrientationHelper.getWindowTiOrientationModeFrom(activity);
	return (orientation == ORIENTATION_LANDSCAPE) || (orientation == ORIENTATION_LANDSCAPE_REVERSE);
}

@jquick-axway
Copy link
Contributor

Closing this PR in favor of #9585 which includes above mentioned breaking change from app orientation to device orientation. (Will match iOS and Windows' behavior.)

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

3 participants