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

fix: #29605 for display: contents parent visibility with correct width check #29680

Merged
merged 43 commits into from
Nov 6, 2024

Conversation

senpl
Copy link

@senpl senpl commented Jun 14, 2024

Additional details

I implemented solution proposed in #29224 still I have to add width and height check because without them element was still visible as with 0 width.
This time it is clear from most other commits and only core changes to make this work.

Steps to test

html

test cy.get('#child').should('be.visible')

How has the user experience changed?

Users has to use {force: true} to click such elements, checking thier visibility was not possible. Now it should be possible

PR Tasks

@cypress-app-bot
Copy link
Collaborator

@senpl
Copy link
Author

senpl commented Jun 27, 2024

now it looks like internal error in build for unit tests.

@senpl
Copy link
Author

senpl commented Jun 27, 2024

This looks like unit-tests fail during build
lib/ca #generateServerCertificateKeys generates certs for each host:
Error: Timeout of 2000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves.

Copy link
Member

@jennifer-shehane jennifer-shehane left a comment

Choose a reason for hiding this comment

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

@senpl There are failures in the visibility test in firefox. Please run these locally to ensure they pass.

@jennifer-shehane jennifer-shehane added the Cypress 14 Issues scoped for Cypress 14 label Sep 27, 2024
@senpl senpl marked this pull request as ready for review September 30, 2024 10:27
@jennifer-shehane jennifer-shehane changed the base branch from develop to release/14.0.0 September 30, 2024 15:28
@jennifer-shehane
Copy link
Member

Updated the base branch to go against release/14.0.0

v1w319

This comment was marked as off-topic.

v1w319

This comment was marked as off-topic.

@jennifer-shehane jennifer-shehane dismissed their stale review November 1, 2024 17:25

dismissing previous review

Copy link
Member

@jennifer-shehane jennifer-shehane left a comment

Choose a reason for hiding this comment

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

I believe this change looks good. I added some extra tests to ensure the elements that do not follow the display: contents rules are interpreted as hidden.

I don't actually think this would introduce breaking changes and would be just a fix, but I do think it's safer to make the changes in Cypress 14.

@jennifer-shehane jennifer-shehane merged commit a6ddd3d into cypress-io:release/14.0.0 Nov 6, 2024
63 of 65 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Cypress 14 Issues scoped for Cypress 14
Projects
None yet
5 participants