margin on Position target causes incorrect positioning

[timothykang]

This issue can be reproduced directly on the Position example, by changing

<Button bsStyle='primary' ref='target' onClick={this.toggle}>

to

<Button bsStyle='primary' ref='target' onClick={this.toggle} style={{marginLeft: '10px'}}>

This issue looks to be caused by the usage of dom-helpers/query/position (in utils/calculatePosition.js). dom-helpers/query/position mimics jQuery.position(), which does not account for margins.

I believe the correct course of action is to account for margins (i.e. add them to the offsets) in utils/calculatePosition.js. If the maintainers agree I'd be happy submit a PR.

[taion]

What does upstream TWBS do?

[jquense]

jquery may include margins in dimension calculations... I'll have to check

[timothykang]

It looks like TWBS uses jQuery.offset() when getting and setting positions (see js/tooltip.js, lines 295 + 370).

[devongovett]

See #175 for a fix.

[taion]

We actually can't do this without breaking Bootstrap compatibility. Bootstrap popovers actually do apply margins and depend on them for the positioning to look right.

This is sort of why we need to pull <Position> as-is into React-Bootstrap – it's just not in general a great primitive for anything other than TWBS overlays.

[qrohlf]

@taion unless I'm mistaken, popover margins are a seperate issue from this ticket which is talking about target element margins.

I've taken a stab at fixing the issue with target element margins, which you can see here: https://github.com/react-bootstrap/react-overlays/pull/206

Open question: should overlays be positioned inside or outside an element's margins by default? I can see a case for both options.