- Notifications
You must be signed in to change notification settings - Fork20.6k
Offset: simplify jQuery#offsetParent method#1968
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| // 3) For body or html element, i.e. in case of the html node - it will return itself | ||
| // | ||
| // but those exceptions were never presented as a real life use-cases | ||
| // and might be considered as more preferable results |
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.
These exceptions would be better to put in the documentation too or instead of
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.
If these are pathological cases that don't occur in real life, I am not sure why we are commenting them anyway. The result might change in a refactoring and these comments would no longer be true, but people might have read them as documented and supported.
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.
How about i will add a notice about it, like "But this is undefined and undocumented behavior, could be changed at any point" or something like that.
It feels weird to know about these edge cases, but not put some info about them somewhere.
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.
It really does seem like this is documentation. I am okay with putting a comment like "this is not guaranteed" nearby though and we can hope we don't need to change it.
* It seems, check for html element (and previously for body element) was redundant* Simplify "return" statement* Add comment about potential errors that didn't find themselves in real life app
Seems like this should addressgh-1765 as well since that will change the same code. |
* It seems, check for html element (and previously for body element) was redundant* Simplify "return" statement* Add comment about potential errors that didn't find themselves in real life appClosesgh-1968
was redundant
This code would work even in IE6 quirks mode