Skip to content
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

Element visibility fixes #1242

Closed
13 of 15 tasks
jennifer-shehane opened this issue Jan 31, 2018 · 2 comments
Closed
13 of 15 tasks

Element visibility fixes #1242

jennifer-shehane opened this issue Jan 31, 2018 · 2 comments
Assignees
Labels
Epic Requires breaking up into smaller issues pkg/driver This is due to an issue in the packages/driver directory topic: visibility 👁 type: bug

Comments

@jennifer-shehane
Copy link
Member

jennifer-shehane commented Jan 31, 2018

There are multiple issues regarding elements being calculated as visible when they are hidden OR elements being calculated as hidden when they are visible. This issue is generally to be used to track the progress made for these fixes.

I think that it is interesting to note that jQuery ditched their visibility calculations in 3.0 and is likely worthwhile looking into why/how they made these decisions.

An element is considered now visible if it has a layout box returned from the DOM getClientRects() method, even if that box has a height and/or width of zero. This means that elements such as
or an empty element that don't have height are considered to be visible.
jquery/jquery#2227
jquery/jquery#2604

Issues

Not reproducible issues

Desired behavior:

  • Find a better way to more accurately determine visibility.
  • Always display reason why we determine something as visible or not visible.
  • Add tests for all failing cases above - get tests passing

Versions

Cypress: 3.3.1

┆Issue is synchronized with this Jira Bug by Unito

@jennifer-shehane jennifer-shehane added the Epic Requires breaking up into smaller issues label Jan 31, 2018
@jennifer-shehane jennifer-shehane self-assigned this Jun 10, 2019
@cypress-bot cypress-bot bot added the stage: ready for work The issue is reproducible and in scope label Jun 10, 2019
@jennifer-shehane jennifer-shehane added topic: visibility 👁 type: bug and removed stage: ready for work The issue is reproducible and in scope labels Jun 10, 2019
@jennifer-shehane
Copy link
Member Author

jennifer-shehane commented Jun 14, 2019

I've made a flowchart of our current implementation for visibility checks. This applies when using cy.get('el').should('be.visible'), etc as well as some other checks we do before actions are taken: .check(), .click(), .type() and some instances of .trigger()

nC0TrUIVMOLJObyg-F2826

Some notes upon review of the diagram on changes we want to make:

Separate out the "loose" checks from "strict" checks for visibility. Today is all "loose" checks -- "strict" checks concerns whether the element is visible within the viewport and the necessity to scroll to an element first.

"Loose" visibility checks

  • Should prove the element is hidden, fall through to visible if not.
  • Checks tiny heuristics of element - is visibility: collapse, is within css-path, etc., etc. etc.
  • Used for 'is.visible' assertions. Meant to represent if the element can be seen by a person without disabilities and 20/20 vision.

"Strict" visibility checks

  • Should prove the element is visible, fall through to hidden if not.
  • Checks just clientRect + elFromPoint and edge cases.
  • Used for actionability checks - some things are not really visible to the eye, but the mouse will click it if the user is on the coordinates and clicks, for example.

Questions

  • Do we introduce a new assertion? pseudo-selector? flag?
  • Is there a better naming convention for loose vs strict??
  • Screenshots + flowcharts for docs to describe "loose" versus "strict".

@jennifer-shehane
Copy link
Member Author

I'm going to close this issue for tracking - as there are only a couple of remaining issues that should be resolved when opacity: 0 is considered, which there is a PR for.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Epic Requires breaking up into smaller issues pkg/driver This is due to an issue in the packages/driver directory topic: visibility 👁 type: bug
Projects
None yet
Development

No branches or pull requests

3 participants