-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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: performance regression due to enabling ScopeMemoryCachePerContext #28327
Conversation
1 flaky test on run #52276 ↗︎
Details:
cypress/e2e/scaffold-component-testing.cy.ts • 1 flaky test • launchpad-e2e
Review all test suite changes for PR #28327 ↗︎ |
@@ -53,7 +53,7 @@ describe('src/cypress/dom/visibility', () => { | |||
expect(fn()).to.be.true | |||
}) | |||
|
|||
it('returns false window and body > window height', () => { | |||
it('returns false if window and body < window height', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what this title wrong before?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Things should only be scrollable if they're > than the height not <.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Basically this is the opposite of line 45.
Released in This comment thread has been locked. If you are still experiencing this issue after upgrading to |
Additional details
This fix basically reverses the commit that was done to try and prevent the font flooding. The only addition is a change to ensure that #28150 stays fixed (namely this commit).
Steps to test
n/a
How has the user experience changed?
n/a
PR Tasks
cypress-documentation
?type definitions
?