dependency: Update jQuery to latest #30345
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
BREAKING: remove the patch that was preventing width/height calculation changes
cypress
|
||||||||||||||||||||||||||||||||||||||||
| Project |
cypress
|
| Branch Review |
remove-jquery-patch
|
| Run status |
|
| Run duration | 21m 25s |
| Commit |
|
| Committer | Jennifer Shehane |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
6
|
Pending
|
1327
|
Skipped
|
0
|
Passing
|
29364
|
| View all changes introduced in this branch ↗︎ | |
UI Coverage
46.24%
|
|
|---|---|
|
Untested elements
|
188
|
|
Tested elements
|
166
|
Accessibility
92.55%
|
|
|---|---|
|
Failed rules
|
3 critical
8 serious
2 moderate
2 minor
|
|
Failed elements
|
897
|
…ypress into remove-jquery-patch
jennifer-shehane
changed the title
jennifer-shehane
added
the
type: breaking change
jennifer-shehane
changed the title
…Query is moving away from sizzle
mschile
reviewed
Nov 4, 2024
| @@ -1419,29 +1419,45 @@ find.matches = function( expr, elements ) { | ||
| }; | ||
|
|
||
| find.matchesSelector = function( elem, expr ) { |
There was a problem hiding this comment.
This change is essentially the same as below except instead of setting $.find.support.matchesSelector to false to force find being called, we now just directly check isUsingFocus to force the same behavior.
cacieprins
approved these changes
Nov 4, 2024
jennifer-shehane
commented
Nov 4, 2024
Comment on lines
+40
to
+41
| cy.findByTestId('playground-selector').clear() | ||
| cy.findByTestId('playground-selector').type('[data-cy-root]') |
There was a problem hiding this comment.
just fixing chaining of a test that was found to match our recommendations
jennifer-shehane
commented
Nov 4, 2024
Comment on lines
+544
to
+545
| // if not a sizzle or jQuery error, ignore it and let $el be null | ||
| if (!(sizzleRe.test(err.stack) || jQueryRe.test(err.stack))) throw err |
There was a problem hiding this comment.
jQuery no longer relies so much on sizzle, so these stack traces changed and are coming from jQuery now.
jennifer-shehane
commented
Nov 4, 2024
Comment on lines
+31
to
+39
| // width or height of DOM in pixels | ||
| this.scrollableContainerWidthHeight = 500 | ||
| this.elementWidthHeight = 100 | ||
| this.scrollBarWidthHeight = 15 | ||
|
|
||
| // divide by 2 to get the center | ||
| // browsers round up the pixel value so we need to round it | ||
| this.halfScrollPixels = Math.round((500 - 100) / 2) | ||
| this.halfScroll = Math.round((this.scrollableContainerWidthHeight - this.elementWidthHeight) / 2) | ||
| this.fullScroll = Math.round(this.scrollableContainerWidthHeight - this.elementWidthHeight) |
There was a problem hiding this comment.
Just cleaning this up to make these arbitrary numbers more understandable.
jennifer-shehane
commented
Nov 4, 2024
| @@ -168,7 +168,7 @@ describe('src/cypress/dom/visibility', () => { | |||
| } | |||
|
|
|||
| // ensure all tests run against a scrollable window | |||
| const scrollThisIntoView = add('<div style=`height: 1000px;` /><div>Should be in view</div>') | |||
| const scrollThisIntoView = add('<div style=`height: 1000px;`></div><div>Should be in view</div>') | |||
There was a problem hiding this comment.
jQuery no longer gracefully handles DOM that is not valid - so this would no longer create the accurate DOM to test against. This was just written wrong at some point.
jennifer-shehane
commented
Nov 4, 2024
Comment on lines
137
to
+138
| -function getWidthOrHeight( elem, dimension, extra ) { | ||
| +function augmentWidthOrHeight( elem, name, extra, isBorderBox, styles ) { | ||
| +function augmentWidthOrHeight( elem, dimension, extra, isBorderBox, styles ) { |
There was a problem hiding this comment.
Putting the old width/height patch back into 3.7.1 code
AtofStryker
approved these changes
Nov 4, 2024
28 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Additional details
Keeping the width/height patch we intended originally to remove
Bumping jQuery
This PR bumps jQuery to 3.7.1 which is latest. 4.0.0 of jQuery is still in Beta.
$.find.support.matchesSelectoris no longer available to set starting in jQuery 3.7.0. Instead, this logic was moved to a patch that now directly usesisUsingFocusto force the same behavior as before.Steps to test
All tests should pass
How has the user experience changed?
PR Tasks
cypress-documentation?type definitions?