Skip to content

Conversation

@nick318
Copy link

@nick318 nick318 commented Jun 5, 2017

I suggest an idea of getting element from Supplier<? extends WebElement>.
It can be helpful in different situations:

  • If we have method that return some div of block and we want to find some element from that block we can write something like that:
    WebElement el = getDiv();
    $(el).$(By.id("input").setValue("text");
    But in this case we have a problem if something goes wrong with first element 'el', I mean if that element updates, we will get StaleElementReferenceException and Selenide cannot re-init that element.

But have a look at that example:
$(this::getDiv).$(input).setValue("text").

In that case, if something goes wrong, Selenide will invoke the given supplier and can re-init first element and others elements as well.

@asolntsev What do you think?

@asolntsev
Copy link
Member

@Icok It's an interesting idea. Yes, probably we should do it.

But can you describe a real case? How could your getDiv() method be implemented?
Can't you just use $("selector")?

@nick318
Copy link
Author

nick318 commented Jun 12, 2017

@asolntsev To answering to your question:
-- Can't you just use $("selector")?
The problem is if someone already implemented methods that return WebElement.
Sometimes yes I can, but I think it is not necessary restriction to return everywhere only 'By' objects.

-- How could your getDiv() method be implemented?
some examples:

getDiv() {
return $$("collection").stream()
                .filter(elem -> elem.is(visible))
                .filter(elem -> elem.getText().equals("need text"))
                .findFirst().orElseThrow(() -> new AssertionError("Not found div"));
}
getDiv() {
   //some actions to click by 3-rd libraries like Robot or Sikuli.
  return driver.switchTo().activeElement(); //It helpful in IE browser, because it does not work by locators sometimes
}

@asolntsev
Copy link
Member

@Icok The build is failing. There are some checkstyle issues. Please execute ./gradle check in your branch.

Copy link
Member

@asolntsev asolntsev left a comment

Choose a reason for hiding this comment

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

Approved, but please make the build green.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 64.11% when pulling a47a83f on iCok:add_supplier_init into b7fe2c7 on codeborne:master.

@codecov-io
Copy link

Codecov Report

Merging #544 into master will decrease coverage by 0.51%.
The diff coverage is 83.33%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #544      +/-   ##
============================================
- Coverage     60.53%   60.02%   -0.52%     
+ Complexity      772      768       -4     
============================================
  Files           148      149       +1     
  Lines          2752     2764      +12     
  Branches        272      272              
============================================
- Hits           1666     1659       -7     
- Misses          981      992      +11     
- Partials        105      113       +8
Impacted Files Coverage Δ Complexity Δ
src/main/java/com/codeborne/selenide/Selenide.java 0% <0%> (ø) 0 <0> (ø) ⬇️
...a/com/codeborne/selenide/impl/ElementSupplier.java 90.9% <90.9%> (ø) 5 <5> (?)
.../com/codeborne/selenide/commands/GetInnerHtml.java 60% <0%> (-40%) 2% <0%> (-1%)
...in/java/com/codeborne/selenide/impl/Navigator.java 59.37% <0%> (-6.25%) 19% <0%> (-2%)
.../java/com/codeborne/selenide/ex/ErrorMessages.java 82.75% <0%> (-3.45%) 16% <0%> (-1%)
.../codeborne/selenide/impl/ScreenShotLaboratory.java 51.68% <0%> (-2.81%) 31% <0%> (-3%)
...ain/java/com/codeborne/selenide/impl/Describe.java 78.2% <0%> (-2.57%) 27% <0%> (-1%)
...e/selenide/impl/WebDriverThreadLocalContainer.java 80.43% <0%> (-2.18%) 30% <0%> (-1%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b7fe2c7...a47a83f. Read the comment docs.

@nick318
Copy link
Author

nick318 commented Oct 28, 2017

@asolntsev Done

@BorisOsipov
Copy link
Collaborator

I am closing it due inactivity. Feel free to start conversation again if you would mind to add such changes.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants