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

Method to check if page is scrolled to the bottom #646

Merged
merged 4 commits into from Apr 19, 2018

Conversation

Projects
None yet
5 participants
@pavelpp

pavelpp commented Dec 18, 2017

Proposed changes

I found it quite useful in my tests with lazy loading of elements to check if the end of the page is reached.

Checklist

  • Checkstyle and unit tests pass locally with my changes by running gradle check chrome htmlunit command
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)
@coveralls

This comment has been minimized.

coveralls commented Dec 18, 2017

Coverage Status

Coverage decreased (-0.02%) to 64.291% when pulling 99c32d1 on pavelpp:page-bottom-check into 014b738 on codeborne:master.

@codecov-io

This comment has been minimized.

codecov-io commented Dec 18, 2017

Codecov Report

Merging #646 into master will decrease coverage by 0.05%.
The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #646      +/-   ##
============================================
- Coverage     60.49%   60.43%   -0.06%     
  Complexity      771      771              
============================================
  Files           148      148              
  Lines          2749     2750       +1     
  Branches        270      270              
============================================
- Hits           1663     1662       -1     
- Misses          981      983       +2     
  Partials        105      105
Impacted Files Coverage Δ Complexity Δ
src/main/java/com/codeborne/selenide/Selenide.java 0% <0%> (ø) 0 <0> (ø) ⬇️
...e/selenide/impl/WebDriverThreadLocalContainer.java 79.71% <0%> (-0.73%) 30% <0%> (ø)

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 014b738...f11a95a. Read the comment docs.

@coveralls

This comment has been minimized.

coveralls commented Dec 19, 2017

Coverage Status

Coverage decreased (-0.02%) to 64.291% when pulling 33987ec on pavelpp:page-bottom-check into 014b738 on codeborne:master.

@coveralls

This comment has been minimized.

coveralls commented Dec 19, 2017

Coverage Status

Coverage decreased (-0.06%) to 64.255% when pulling f11a95a on pavelpp:page-bottom-check into 014b738 on codeborne:master.

@asolntsev

This comment has been minimized.

Contributor

asolntsev commented Dec 19, 2017

@pavelpp Can you describe how are going to use this method? What is the real case?
Probably it's better to add method Selenide.shouldBe(atBottom) or Selenide.scrollTo(bottom)?

@pavelpp

This comment has been minimized.

pavelpp commented Dec 21, 2017

@asolntsev shouldBe methods would also be helpful for some cases, but my use case is completely different. One thing is expecting that after a certain action you are at the bottom of the page, and another checking state to decide what to do next. My PR is about the latter. I'm using it a lot with lazy loading when I need to perform some action on the elements which are being loaded into dom while I scroll the page, so essentially it helps with looping

while(!atBottom()){
    scrollPixels(0,200);
    //do something
}
@BorisOsipov

This comment has been minimized.

Member

BorisOsipov commented Dec 28, 2017

I am not sure about such changes..
I think it isn't good idea to place such js scripts to Selenide.java.
Yeah it is good script(especially if it works for chrome\firefox and other browsers as well ) and probable I will use it, but in very specific cases...

@pavelpp

This comment has been minimized.

pavelpp commented Dec 28, 2017

@BorisOsipov , I was looking for a better place for this method, and couldn't find anything. Perhaps we could add a new utility class, something like ScrollActions or similar?

@BorisOsipov

This comment has been minimized.

Member

BorisOsipov commented Dec 28, 2017

@pavelpp Yeah it makes sense. @asolntsev what do yo think?

@asolntsev

This comment has been minimized.

Contributor

asolntsev commented Jan 9, 2018

@pavelpp @BorisOsipov Yes, probably ScrollActions is a good name.

I was also thinking that some of existing static methods in class Selenide are also too specific, and (in theory) should also be moved to dedicated classes:

  • confirm()
  • dismiss()
  • prompt()
  • onConfirmReturn()
  • getJavascriptErrors()
  • getWebDriverLogs()
  • getSelectedRadio()
  • getUserAgent()
  • zoom()

Should we have a single class for all of them or many different small classes?

@BorisOsipov

This comment has been minimized.

Member

BorisOsipov commented Jan 10, 2018

Should we have a single class for all of them or many different small classes?

I think yes, it will be better to separate them.

@pavelpp

This comment has been minimized.

pavelpp commented Jan 10, 2018

@asolntsev yes, it makes sense, though, I think it shouldn't be part of this PR

@asolntsev asolntsev added this to the 4.11.2 milestone Apr 19, 2018

@asolntsev asolntsev merged commit 68f3875 into selenide:master Apr 19, 2018

1 of 4 checks passed

codecov/patch 0% of diff hit (target 60.49%)
Details
codecov/project 60.43% (-0.06%) compared to 014b738
Details
coverage/coveralls Coverage decreased (-0.06%) to 64.255%
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@asolntsev asolntsev self-assigned this Apr 19, 2018

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