Skip to content

internal: fix assertion menu not displayed in the correct location #32172

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

Merged
merged 8 commits into from
Aug 11, 2025

Conversation

mabela416
Copy link
Contributor

@mabela416 mabela416 commented Aug 6, 2025

Additional details

  • Our website uses Astro components which defines placeholders for the content that should be injected into the components. Because of this we end up with npm install cypress text wrapped around an astro-slot element which is what's targeted when we right click this text in studio. Since the astro-slot element is just a wrapper for the injected content, it has 0 width and 0 height and is not visible so when we calculate the highlight styles for the assertions menu, we end up with 0 offsets for the top and the left. I thought for cases like this to look at the children of the element, if there any, and use the offsets of the children to get a better location for the assertions menu.

Steps to test

  • Visit your website on a test and open it in studio. When clicking on npm install cypress, the assertions menu should be displayed by the text instead of the top left corner. Also make sure clicking on other elements opens the assertions menu in the correct location
it('cypress.io', function(){
  cy.visit('https://www.cypress.io/')
})
Screen.Recording.2025-08-06.at.2.17.29.PM.mov

How has the user experience changed?

PR Tasks

cursor[bot]

This comment was marked as outdated.

@mabela416 mabela416 self-assigned this Aug 6, 2025
Copy link

cypress bot commented Aug 6, 2025

cypress    Run #64468

Run Properties:  status check passed Passed #64468  •  git commit 7af0b2a6a7: Merge branch 'release/15.0.0' into mabel/issue-11216-menu-location
Project cypress
Branch Review mabel/issue-11216-menu-location
Run status status check passed Passed #64468
Run duration 19m 32s
Commit git commit 7af0b2a6a7: Merge branch 'release/15.0.0' into mabel/issue-11216-menu-location
Committer mabela416
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 13
Tests that did not run due to a developer annotating a test with .skip  Pending 1101
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 26533
View all changes introduced in this branch ↗︎
UI Coverage  44.99%
  Untested elements 187  
  Tested elements 157  
Accessibility  97.71%
  Failed rules  4 critical   8 serious   2 moderate   2 minor
  Failed elements 110  

@jennifer-shehane jennifer-shehane self-requested a review August 6, 2025 20:44
@mabela416 mabela416 requested review from mschile and astone123 August 7, 2025 14:52
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.

Interesting. This does solve this specific issue. I'm unsure if this solution will work with all situations, but it's hard to test ahead of time until people use it and report it.

This update only impacts the highlightStyles however, so it's lowish risk if it doesn't work for every situation.

@mabela416
Copy link
Contributor Author

mabela416 commented Aug 7, 2025

Interesting. This does solve this specific issue. I'm unsure if this solution will work with all situations, but it's hard to test ahead of time until people use it and report it.

This update only impacts the highlightStyles however, so it's lowish risk if it doesn't work for every situation.

Yeah this won't work is if let's say the text is by itself so if 'npm install cypress' didn't have the svg icon with it because then it'd just be a TextNode and TextNodes would not show up as children of an element. I tried using childNodes instead but getBoundingClientRect() only works on HTML Elements and not TextNodes so I don't even know if/how we'd want to handle that case. I think that case would be really rare so yeah I think we can wait and see if people report any issues with it and see how we want to handle them.

@mabela416 mabela416 changed the base branch from release/15.0.0 to develop August 11, 2025 13:34
@jennifer-shehane jennifer-shehane merged commit d28bc0e into develop Aug 11, 2025
87 of 91 checks passed
@jennifer-shehane jennifer-shehane deleted the mabel/issue-11216-menu-location branch August 11, 2025 13:42
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