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.

Sign up for GitHub

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

Device Orientation Manipulation #133

Merged
merged 9 commits into from
May 25, 2017
Merged

Device Orientation Manipulation #133

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
4f9d94d
add orientation change screen to test app
DanielMSchmidt May 21, 2017
1cfc199
add orientation change e2e test
DanielMSchmidt May 21, 2017
e37a429
add UIDeviceOrientation type for method invocation
DanielMSchmidt May 23, 2017
78111e4
add setOrientation to device
DanielMSchmidt May 23, 2017
7bc9b52
refactor earl grey call and orientation constants
DanielMSchmidt May 25, 2017
d0c6f3b
move setOrientation implementation to simulator
DanielMSchmidt May 25, 2017
41b24d9
add unit tests for orientation change
DanielMSchmidt May 25, 2017
582103f
refactor call to EarlGrey
DanielMSchmidt May 25, 2017
8590f44
document new api surface for orientation change
DanielMSchmidt May 25, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions detox/ios/Detox/MethodInvocation.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,24 @@ + (id) getValue:(id)value withType:(id)type onError:(void (^)(NSString*))onError
if (![value isKindOfClass:[NSDictionary class]]) return nil;
return [MethodInvocation invoke:value onError:onError];
}
if ([type isEqualToString:@"UIDeviceOrientation"])
Copy link
Member

Choose a reason for hiding this comment

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

The main advantage of the remote invocation mechanism is that we don't need to add any other new call to the protocol

I believe we didn't make the invocation mechanism easily accessible for extension, and this is our fault.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does this mean I could have written invoke.IOS.UIDeviceOrientation('UIDeviceOrientationLandscapeLeft') or what would the JS call need to look like in this case?

{
if (value == nil) {
return nil;
}

if ([value isEqualToString:@"landscape"])
{
return [NSNumber numberWithLong:UIDeviceOrientationLandscapeLeft];
}

if ([value isEqualToString:@"portrait"])
{
return [NSNumber numberWithLong:UIDeviceOrientationPortrait];
}

return nil;
}
return nil;
}

Expand Down
11 changes: 11 additions & 0 deletions detox/src/devices/Device.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
const fs = require('fs');
const _ = require('lodash');
const argparse = require('../utils/argparse');
const invoke = require('../invoke');

class Device {
constructor(client, session, deviceConfig) {
Expand Down Expand Up @@ -42,6 +43,16 @@ class Device {
await this.client.sendUserNotification(params);
}

async setOrientation(orientation) {
Copy link
Member

Choose a reason for hiding this comment

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

Device.js is platform agnostic, it's a base class (will be the base class for Android devices as well). This is a simulator specific logic, hence needs to be in Simulator.js

Copy link
Member

Choose a reason for hiding this comment

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

I see there are no unit tests here, the build will fail on lack of coverage.

Copy link
Member

Choose a reason for hiding this comment

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

and it did
https://travis-ci.org/wix/detox/builds/235424799#L2820

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I will move it to Simulator.js and add some unit tests

// orientation is 'landscape' (meaning left side portrait) or 'portrait' (non-reversed)
const call = invoke.call(
Copy link
Member

Choose a reason for hiding this comment

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

you can invoke the EarlGrey functions directly instead

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So I would do

invoke.IOS.EarlGrey(
         'rotateDeviceToOrientation:errorOrNil:',
        invoke.IOS.UIDeviceOrientation(orientation)
);

or how would it have to look like?

Copy link
Member

@rotemmiz rotemmiz May 25, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

The ObjC code looks like this:

[EarlGrey rotateDeviceToOrientation:UIDeviceOrientationLandscapeLeft errorOrNil:nil];

JS code pass UIDeviceOrientation which is an enum, so I guess the easiest way is to pass NSInteger with the position of the desired orientation enum.

Looking at UIDevice.h, which holds these enums

typedef NS_ENUM(NSInteger, UIDeviceOrientation) {
    UIDeviceOrientationUnknown,
    UIDeviceOrientationPortrait,            // Device oriented vertically, home button on the bottom
    UIDeviceOrientationPortraitUpsideDown,  // Device oriented vertically, home button on the top
    UIDeviceOrientationLandscapeLeft,       // Device oriented horizontally, home button on the right
    UIDeviceOrientationLandscapeRight,      // Device oriented horizontally, home button on the left
    UIDeviceOrientationFaceUp,              // Device oriented flat, face up
    UIDeviceOrientationFaceDown             // Device oriented flat, face down
} __TVOS_PROHIBITED;

For UIDeviceOrientationPortrait pass pass invoke.IOS.NSInteger(1)
For UIDeviceOrientationLandscapeLeft pass invoke.IOS.NSInteger(3)

I think this should work ...
Of course, for readability, we'd like to keep this enum values in JS as well.

So it would look something like this:

let invocation = invoke.IOS.EarlGrey(
         'rotateDeviceToOrientation:errorOrNil:',
        invoke.IOS.NSInteger(orientationEnum)
await invocationManager.execute(invocation);
);

I hope it'll work :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey, thanks for the insight, got it working without any native extension (yay 🎉 ). Though the call you describe doesn't work, it leads to an object like

{
    type: "EarlGrey",
    value: "rotateDeviceToOrientation:errorOrNil:",
    id: 5
}

This leads to a "target is invalid" error, which, to me, looks a bit like a "bug", right?

Copy link
Member

Choose a reason for hiding this comment

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

I spent try my code, it's reasonable I mistaken.
What is the working syntax then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I just wanted to state that calling

invoke.IOS.EarlGrey(
         'rotateDeviceToOrientation:errorOrNil:',
        invoke.IOS.NSInteger(orientationEnum)
);

would be the most readable and easiest way, I will try to make it work, the tests will tell me if I did something wrong then ;)

invoke.IOS.EarlGrey(''),
'rotateDeviceToOrientation:errorOrNil:',
invoke.IOS.UIDeviceOrientation(orientation)
);
await new invoke.InvocationManager(this.client).execute(call);
}

async shutdown() {
return await Promise.resolve('');
}
Expand Down
24 changes: 24 additions & 0 deletions detox/test/e2e/f-simulator.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,28 @@ describe('Simulator', () => {
await element(by.label('Say Hello')).tap();
await expect(element(by.label('Hello!!!'))).toBeVisible();
});

describe('device orientation', () => {
Copy link
Member

Choose a reason for hiding this comment

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

awesome stuff!

beforeEach(async() => {
await device.reloadReactNative();
await element(by.label('Orientation')).tap();

// Check if the element whichs input we will test actually exists
await expect(element(by.id('currentOrientation'))).toExist();
});

it('OrientationLandscape', async () => {
await device.setOrientation('landscape');

await expect(element(by.id('currentOrientation'))).toHaveText('Landscape');
});

it('OrientationPortrait', async() => {
// As default is portrait we need to set it otherwise
await device.setOrientation('landscape');
await device.setOrientation('portrait');

await expect(element(by.id('currentOrientation'))).toHaveText('Portrait');
});
});
});
1 change: 1 addition & 0 deletions detox/test/index.ios.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ class example extends Component {
{this.renderScreenButton('Stress', Screens.StressScreen)}
{this.renderScreenButton('Switch Root', Screens.SwitchRootScreen)}
{this.renderScreenButton('Timeouts', Screens.TimeoutsScreen)}
{this.renderScreenButton('Orientation', Screens.Orientation)}
</View>
);
}
Expand Down
33 changes: 33 additions & 0 deletions detox/test/src/Screens/Orientation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import React, { Component } from 'react';
import {
Text,
View,
TouchableOpacity
} from 'react-native';

export default class Orientation extends Component {

constructor(props) {
super(props);
this.state = {
horizontal: false
};
console.log('Orientation react component constructed (console.log test)');
}

detectHorizontal({nativeEvent: {layout: {width, height,x,y}}}) {
this.setState({
horizontal: width > height
});
}

render() {
return (
<View onLayout={this.detectHorizontal.bind(this)} style={{flex: 1, paddingTop: 20, justifyContent: 'flex-start', alignItems: 'center'}}>
<Text testID="currentOrientation" style={{fontSize: 25, marginTop: 50}}>
{this.state.horizontal ? 'Landscape' : 'Portrait'}
</Text>
</View>
);
}
}
4 changes: 3 additions & 1 deletion detox/test/src/Screens/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import WaitForScreen from './WaitForScreen';
import StressScreen from './StressScreen';
import SwitchRootScreen from './SwitchRootScreen';
import TimeoutsScreen from './TimeoutsScreen';
import Orientation from './Orientation';

export {
SanityScreen,
Expand All @@ -15,5 +16,6 @@ export {
WaitForScreen,
StressScreen,
SwitchRootScreen,
TimeoutsScreen
TimeoutsScreen,
Orientation
};