cypress-io / cypress

Fast, easy and reliable testing for anything that runs in a browser.
https://cypress.io
MIT License
47.04k stars 3.19k forks source link

Proposal to create a new command to combine at least get & scrollIntoView #20103

Open clairevalant opened 2 years ago

clairevalant commented 2 years ago

What would you like?

Hello!

Quite often Cypress tests will pass locally but fail in our QA environment simply because an element is present but not scrolled into view. The should('be.visible') assertion can succeed locally but fail elsewhere due to differences in viewport size.

This error: Times out retrying after 10000ms: Expected to find element: [data-test="{exampleDataTest}"], but never found it can cause confusion around whether the element was even present on the page (as an aside, making this error more specific to the command like Element exists on page but is not in view might also be useful). Elements can be missing from a page for a number of reasons, meaning developers spend their time debugging this when the solution is a quick scrollIntoView().

Since most users would know to scroll to see an element I find this type of failure is to be overkill (unless immediate visibility on screen is specifically being tested for), and would make good use of a command that scrolls elements into view and checks their visibility automatically.

I proposed that we create and consistently use a new custom Cypress command to get an element, scroll it into view, and assert it is visible all at once. It will be called something like getAndScrollIntoView and essentially be the following:

cy.get(element)
    .scrollIntoView()
    .should('be.visible')

These three would be ideal for how we use Cypress, but even just a combination of get and scrollIntoView would be appreciated.

I will be implementing this custom command for my company's project, but would naturally prefer to use a built-in Cypress command. I also assume others may have has a similar issue with this!

Why is this needed?

Other

If there already exists a similar solution out of the box I would love to hear! Thanks!

BlueWinds commented 2 years ago

If we're giving an error saying the element doesn't exist when it simply isn't visible, that's definitely something we'd consider a bug, but I'm not seeing that to be the case.

it("works", () => {
  cy.document().then(d => {
    d.body.innerHTML = `
      <div style="height: 3000px"></div>
      <div class="exists">Hello</div>
    `
  })
  cy.get('.exists').should('be.visible')
});

it("gives informative error", () => {
  cy.document().then(d => {
    d.body.innerHTML = `
      <div style="height: 3000px"></div>
      <div class="hidden" style="display: none">Hello</div>
    `
  })
  cy.get('.hidden').should('be.visible')
});

The first test passes with expected <div.exists> to be visible, while the second one fails with This element <div.hidden> is not visible because it has CSS property: display: none, as I'd expect - nothing saying that the element doesn't exist when it's actually just hidden.

adamdehaven commented 1 year ago

I have a similar scenario; however, it's not technically a bug but rather a behavior that Cypress cannot work around given the available config settings.

We have body { overflow: hidden } with our main app contained inside an element that is position: fixed. A child of this fixed element is what actually scrolls for our app content.

Cypress errors when trying to click an element that does exist in the DOM, but is not in the viewport, because it will not properly scroll to the element (as it does if the body allows scrolling) and throws the error:

Timed out retrying after {time}: expected '{selector}' to be 'visible'

This element {selector} is not visible because its ancestor has `position: fixed` CSS
property and it is overflowed by other elements. 
How about scrolling to the element with `cy.scrollIntoView()`?

When I add cy.get(selector).scrollIntoView() the test successfully passes; however, I shouldn't have to manually chain the .scrollIntoView() action to our hundreds of e2e tests. I'm currently considering overriding the default cy.get() command but I don't love this either since it's not needed in all scenarios.

There should be a default config option (something like a global scrollIntoView: boolean) that tells Cypress to do this by default, just as a user would (e.g. a user cannot click on an element without first scrolling it into view in the first place).