Skip to content

Conversation

@ndbroadbent
Copy link
Contributor

@ndbroadbent ndbroadbent commented Sep 29, 2020

I have some test cases where I need to call page.execute_script('window.scrollTo(0, 0)') before taking a screenshot. I'm using Tabler, which sets scroll-behavior: smooth;, so any scroll actions are smoothly animated. This is nice in production, but annoying during tests.

This proposed change will cause the scroll to always complete instantly without any animations. (This will only work when using the default selector_for value of '*', so that the rule matches the html element.)

scroll-behavior: auto is the default value, so I don't think this change should cause any problems.

I've added some tests, including an "inverse" test for Capybara.disable_animation = false. I think this important to show that scroll-behavior: smooth; does affect the scrolling speed, so that the test will have no false negatives.

EDIT: I just remembered another change that I made in my own version:

caret-color: transparent;

This is also very helpful during some of my screenshot diff tests where I'm checking that form inputs are rendered and filled in correctly.

Would it be ok to add caret-color: transparent;? I would love to start using the built-in AnimationDisabler from Capybara, but I really need this caret-color: transparent; as well. Or would it be possible to add the option to inject my own custom CSS?

EDIT 2: What do you think about this disable_animation_extra_properties change, so that I can set Capybara.disable_animation_extra_properties = 'caret-color: transparent;'?

@ndbroadbent ndbroadbent changed the title Add "html, body { scroll-behavior: auto; }" to Capybara::Server::AnimationDisabler Add "scroll-behavior: auto;" CSS to Capybara::Server::AnimationDisabler Sep 29, 2020
@ndbroadbent ndbroadbent changed the title Add "scroll-behavior: auto;" CSS to Capybara::Server::AnimationDisabler Add "scroll-behavior: auto" CSS to Capybara::Server::AnimationDisabler Sep 29, 2020
@ndbroadbent ndbroadbent force-pushed the scroll_behavior_auto branch 2 times, most recently from a137df2 to f829e43 Compare September 29, 2020 06:52
@ndbroadbent
Copy link
Contributor Author

ndbroadbent commented Sep 29, 2020

It looks like I'm seeing the same test failure as the master branch, but otherwise all the tests are passing.

@twalpole
Copy link
Member

Thanks for this - I've made one comment around the tests, and you also have rubocop style failures - https://travis-ci.org/github/teamcapybara/capybara/jobs/731188375. You can run `bundle exec rubocop' before committing

@ndbroadbent
Copy link
Contributor Author

Thanks for the review @twalpole! I've updated the tests with your suggestions, and also fixed all the style failures (sorry about those!)

@twalpole
Copy link
Member

twalpole commented Oct 4, 2020

Thanks! LGTM

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants