-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Element - take screenshot #2795
Conversation
detox/src/ios/expectTwo.js
Outdated
@@ -105,8 +108,9 @@ class InternalExpect extends Expect { | |||
} | |||
|
|||
class Element { | |||
constructor(invocationManager, matcher) { | |||
constructor(invocationManager, matcher, emitter) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would advise putting emitter before matcher (keeping matcher as the last arg)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does it matter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NVM, did so
detox/src/ios/expectTwo.test.js
Outdated
} | ||
}; | ||
|
||
expect(invocationManager.execute).toHaveBeenCalledWith(jsonOutput); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(: @noomorph helped
|
||
await element(by.id('switchOrientation')).tap(); | ||
|
||
const bitmapPath = await element(by.id('fancyElement')).takeScreenshot('fancy-element'); | ||
expectBitmapsToBeEqual(bitmapPath, screenshotAssetPath); | ||
}); | ||
|
||
it('should fail to take a screenshot of an off-screen element', async () => { | ||
it(':android: should fail to take a screenshot of an off-screen element', async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alon-ha Can we do this for iOS as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, but I don't see the reason. It's also an argument if to allow taking a screenshot of an element which is not visible on the screen. Should we do it or not? There are few questions here.. Let's discuss today
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No huge objections so far except for linting issues – but I plan to auto fix this after my PR with new lint rules passes.
6e42fdf
to
4d0beeb
Compare
Closes #2659. Added element screenshot functionality in iOS