Skip to content

Conversation

@jbedard
Copy link
Contributor

@jbedard jbedard commented Oct 19, 2016

I assume this previously worked (except IE<=11+ which crashed) before 0e4477c.

Also previously when if ( rect.width || rect.height ) failed it would return rect which would be the getBoundingClientRect() object containing top/bottom/left/right along with the width/height of 0. The top/left in there would probably be quite misleading?

Fixes gh-3267

@mention-bot
Copy link

@jbedard, thanks for your PR! By analyzing the history of the files in this pull request, we identified @NekR, @timmywil and @markelog to be potential reviewers.

@jquerybot
Copy link

Thank you for your pull request. It looks like this may be your first contribution to a jQuery Foundation project, if so we need you to sign our Contributor License Agreement (CLA).

📝 Please visit http://contribute.jquery.org/CLA/ to sign.

After you signed, the PR is checked again automatically after a minute. If there's still an issue, please reply here to let us know.


If you've already signed our CLA, it's possible your git author information doesn't match your CLA signature (both your name and email have to match), for more information, check the status of your CLA check.

@NekR
Copy link
Contributor

NekR commented Oct 19, 2016

So, do we want to report left/top for 0 sized elements? If not, then it can be simpler this:

if ( rect.width || rect.height ) {
   // ...
} else {
  return { left: 0, top: 0 }
}

@NekR
Copy link
Contributor

NekR commented Oct 19, 2016

Oh, nevermind, 0 sized elements still can provide some visible objects (child elements / text) so definitely should return correct offset.

@timmywil timmywil added this to the 3.2.0 milestone Oct 24, 2016
@gibson042
Copy link
Member

LGTM. 👍

@timmywil timmywil assigned timmywil and jbedard and unassigned timmywil Dec 12, 2016
@mgol
Copy link
Member

mgol commented Dec 12, 2016

LGTM

@markelog
Copy link
Member

We can now write LGTM with github buttons now :)

@jbedard jbedard merged commit 1777899 into jquery:master Dec 13, 2016
@lock lock bot locked as resolved and limited conversation to collaborators Jan 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Offset: report offset for zero size elements

9 participants