All: Resolve most jQuery Migrate warnings #1919
Conversation
mgol
force-pushed
the
migrate-warnings
branch
2 times, most recently
from
eacb8e3 to
e2a60e3
Compare
| @@ -331,7 +331,7 @@ $.widget( "ui.tooltip", { | |||
| position( positionOption.of ); | |||
| clearInterval( delayedShow ); | |||
| } | |||
| }, $.fx.interval ); | |||
| }, 13 ); | |||
There was a problem hiding this comment.
Do we want a comment here why its 13?
There was a problem hiding this comment.
Good point. This is copied from the $.fx.interval value & follows the advice from https://github.com/jquery/jquery-migrate/blob/master/warnings.md#jqmigrate-jqueryfxinterval-is-deprecated
ui/safe-offset.js
Outdated
|
|
||
| // Simulate a jQuery short-circuiting when there are no client rects reported | ||
| // which usually means a disconnected node. This check in jQuery is meant just | ||
| // for IE but UI depends on it. |
There was a problem hiding this comment.
Just for my understanding: this check is implemented in jQuery for IE only, bit UI needs it in every browser, right?
There was a problem hiding this comment.
Not exactly. jQuery has a check meant for IE but it's run by all browsers so by pure coincidence it makes UI not hit errors here. This is about these lines:
https://github.com/jquery/jquery/blob/3.5.1/src/offset.js#L98-L104
There was a problem hiding this comment.
How about:
| // for IE but UI depends on it. | |
| // for IE but is applied in all browsers & UI depends on it. |
?
There was a problem hiding this comment.
I still don't understand why we need this in jQueryUI when its already in jQuery... sorry, my mind is a little slow today ;)
There was a problem hiding this comment.
See https://jquery.com/upgrade-guide/3.0/#breaking-change-invalid-input-to-the-offset-method
jQuery 3.0.0 & newer require DOM elements with the getBoundingClientRect method. It's just that jQuery needs to run a check for getClientRects() due to the comment above the lines I linked, to workaround an IE issue. We shouldn't rely on this check for other purposes.
Also, jQuery Migrate warns if you use offset() in such a way.
That said, now I see the discussion in jquery/jquery#2310 so maybe I need to re-read it & make sure Migrate is not warning too much.
There was a problem hiding this comment.
Ahhh, got it. Thanks for the explanation and your patience :-)
There was a problem hiding this comment.
@fnagel Thank you for raising these concerns! This change will most likely not be needed as we'll update Migrate instead.
This also adds a new internalthis part is now extracted to #1922.$.ui.__safeOffset__method to provide a similar support to past jQuery versions while avoiding Migrate warnings.