Skip to content
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

Possible fix for #14013 #14015

Merged
merged 2 commits into from
Jul 6, 2014
Merged

Possible fix for #14013 #14015

merged 2 commits into from
Jul 6, 2014

Conversation

cvrebert
Copy link
Collaborator

@cvrebert cvrebert commented Jul 2, 2014

A possible fix for #14013.
Demo: http://jsfiddle.net/4pLdD/

CC: @fat for review

@cvrebert cvrebert added the js label Jul 2, 2014
@cvrebert cvrebert added this to the v3.2.1 milestone Jul 2, 2014
@BertrandBordage
Copy link

Awesome reactivity @cvrebert!
I tested your fix, and it works as expected!

@fat
Copy link
Member

fat commented Jul 6, 2014

that function has gotten pretty gnarly… do you mind separating the things out a bit… something like this?

Tooltip.prototype.getPosition = function ($element) {
    $element   = $element || this.$element

    var el     = $element[0]
    var isBody = el.tagName == 'BODY'
    var isSvg  = window.SVGElement && el instanceof window.SVGElement

    var elRect    = typeof el.getBoundingClientRect == 'function' ? el.getBoundingClientRect() : null
    var elOffset  = isBody ? { top: 0, left: 0 } : $element.offset()
    var scroll    = isBody ? document.documentElement.scrollTop || document.body.scrollTop : $element.scrollTop()
    var outerDims = isSvg ? null : {
      width:  isBody ? $(window).width()  : $element.outerWidth(),
      height: isBody ? $(window).height() : $element.outerHeight()
    }

    return $.extend({}, elRect, scroll, outerDims, elOffset)
}

@fat
Copy link
Member

fat commented Jul 6, 2014

otherwise lg, feel free to merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants