-
Notifications
You must be signed in to change notification settings - Fork 573
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
Make screenshot of SelenideElement/WebElement which is inside iframe #705
Conversation
…t which is inside iframe
@andrejska For first why don't you cover this feature with tests? |
Codecov Report
@@ Coverage Diff @@
## master #705 +/- ##
============================================
+ Coverage 61.81% 63.48% +1.67%
- Complexity 827 862 +35
============================================
Files 154 154
Lines 2891 2977 +86
Branches 281 288 +7
============================================
+ Hits 1787 1890 +103
+ Misses 996 988 -8
+ Partials 108 99 -9
Continue to review full report at Codecov.
|
@andrejska ping! |
Added tests Added same behavior as for existing screenshoting functionality in case if element doesn't exist inside iframe Now taking into account vertical scroll bar for partially visible elements
Tests added
Difference between taking screenshot of element inside iframe and regular element screenshot is big, so reusability of this method will just add more complexity
For my specific use case I need a screenshot of webelement inside iframe
Behavior implemented, warning message and returning null, similar approach as in takeScreenshotAsImage |
@rosolko can you please check? |
@andrejska Hey, looks better now, but unfortunately you didn't add any tests for new command Also I want to clarify why method signature takes Also I'm not sure about such complex method |
…r when element is not visible
@rosolko did changes according to your latest comment, added missing tests & semantic change in order of parameters, please check |
@andrejska LGTM. |
+1 It looks nasty. No other objections. |
@rosolko, @BorisOsipop what are next steps :)? |
@andrejska Sorry for long response. I'll take a look at this PR today and if I can't decrease complexity of method fastly - I'll merge it. |
Style code.
@andrejska @rosolko Thanks for the PR! Merged to Selenide 4.11.2 |
Proposed changes
New feature: added possibility to make a screenshot of SelenideElement or WebElement which is inside iframe
Checklist
gradle check chrome htmlunit
command