-
Notifications
You must be signed in to change notification settings - Fork 3.4k
perf: improve performance of pinning snapshots during hover / pin #32951
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
base: develop
Are you sure you want to change the base?
Conversation
…tions (used for snapshots)
cypress
|
||||||||||||||||||||||||||||||||||||||||
| Project |
cypress
|
| Branch Review |
snapshot-reduce-get-computed-style
|
| Run status |
|
| Run duration | 19m 19s |
| Commit |
|
| Committer | Jennifer Shehane |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
|
|
0
|
|
|
11
|
|
|
1098
|
|
|
4
|
|
|
26697
|
| View all changes introduced in this branch ↗︎ | |
Warning
Partial Report: The results for the Application Quality reports may be incomplete.
UI Coverage
45.74%
|
|
|---|---|
|
|
186
|
|
|
161
|
Accessibility
98%
|
|
|---|---|
|
|
4 critical
8 serious
2 moderate
2 minor
|
|
|
101
|
| @@ -0,0 +1,137 @@ | |||
| import { AutIframe } from './aut-iframe' | |||
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.
I don't think these are in an optimal form for unit tests, but it doesn't seem we have vitest or any other convention here but Cypress tests. I'd like to have a test for this, so welcome to if there's a better way to do this.
| @@ -0,0 +1,58 @@ | |||
| import { getElementDimensions } from './dimensions' | |||
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.
Another 'unit' test
| const style = `border: none !important; margin: 0 !important; padding: 0 !important; position: absolute; top: ${top}px; left: ${left}px; width: ${width}px; height: ${height}px; background-color: black; z-index: 2147483647;` | ||
|
|
||
| const div = document.createElement('div') | ||
|
|
||
| div.className = '__cypress-blackout' | ||
| div.style.cssText = style | ||
| $body.append(div) |
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.
Probably not helpful much, but looked into this function a bit for the blackout of screenshots.
|
|
||
| const getElementDimensions = ($el: JQuery<HTMLElement>) => { | ||
| const el: HTMLElement = $el.get(0) | ||
|
|
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.
We should consolidate these but I didn't want to get distracted with this.
|
Looks good. Some thoughts on further performance improvement potential:
|
|
@cacieprins Love the ideas. I hadn't thought about intersection observer but that could be perfect - might need more robust tests. It's just painting SO many highlights that aren't even on the screen. I'd probably prioritize perf improvements that apply to both run mode and open mode over diving deeper into the snapshot improvements though. |
|
Oh I have some legit failures I need to fix in CT tests. |
| **Performance:** | ||
|
|
||
| - Limits the number of matched elements that are tested for visibility when added to a command log entry. Fixes a crash scenario related to rapid successive DOM additions in conjunction with a large number of elements returned from a query. Addressed in [#32937](https://github.com/cypress-io/cypress/pull/32937). | ||
| - Improved performance when viewing command snapshots in the Command Log. Element highlighting is now significantly faster, especially when highlighting multiple elements or complex pages. This is achieved by reducing redundant style calculations and batching DOM operations to minimize browser reflows. Addressed in [#32951](https://github.com/cypress-io/cypress/pull/32951). |
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.
Bug: Duplicate changelog entry in two versions
The same performance improvement entry for PR #32951 appears in both version 15.7.1 (line 8) and version 15.7.0 (line 17). Since version 15.7.0 was already released on 11/19/2025, this entry should only appear in the unreleased version 15.7.1. The duplicate entry in 15.7.0 needs to be removed.
Additional details
Improves performance of hovering or pinning a command to highlight elements within a snapshot. The improvements would be most evident when the command queries A LOT of elements. There are a few strategies employed here:
getComputedStylesSteps to test
How has the user experience changed?
Before
When hovering/clicking to pin a snapshot that targets 3,000 elements completely locks up the UI for 15+ seconds. You cannot hover or pin any other snapshots during this time - or see any highlights.
Screen.Recording.2025-11-14.at.12.42.00.PM.mov
After
When hovering/clicking to pin a snapshot that targets 3,000 element - highlights and pinning shows up within 2 seconds (although ALL of the elements are not done highlighting). You can see when all of the elements finish highlighting in my video where the console.log logs when it's complete.
Screen.Recording.2025-11-14.at.12.38.48.PM.mov
You can click and hover around fairly quickly (feels a little laggy) since the previous batch of highlighting now cancels when you go to highlight other snapshots.
Screen.Recording.2025-11-14.at.12.48.46.PM.mov
PR Tasks
cypress-documentation?type definitions?Note
Speeds up Command Log snapshot highlighting by caching computed styles, batching DOM/offset updates, and canceling stale operations; adds tests and updates changelog, plus minor driver blackout and dimensions optimizations.
DocumentFragment, then position in rAF batches usingsetOffset; cancel stale runs via an operation id._addElementBoxModelLayersnow accepts optionaldimensions, avoidsgetComputedStyle, appliestransform/zIndexfromgetElementDimensions, stores positions indata-*for later offsetting; removed internal offset sizing helper.highlightElfilters hidden/zero-size via cachedoffsetWidth/offsetHeightanddisplay;removeHighlightsuses native query/remove; DOM queries during restore are cached;_replaceHeadStylesaccepts optional$head.getElementDimensionscallsgetComputedStyleonce, returns padding/border/margin,display,transform,zIndex, andoffsetWidth/offsetHeight;setOffsetandgetOffsetoptimized (cached style, defaultView guard).getElementDimensionsand_addElementBoxModelLayersto assert singlegetComputedStylecall and no extra calls when dimensions provided.blackout: replace string-built HTML with nativedivcreation andcssTextassignment.dom/dimensions: cachegetComputedStyleonce; read numeric styles from cached declaration.15.7.1(also noted under15.7.0).Written by Cursor Bugbot for commit 267486d. This will update automatically on new commits. Configure here.