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.

Sign up for GitHub

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

Jump to bottom

Tooltip/Popover container option is not implemented #17755

Closed
BenjinYap opened this issue Oct 1, 2015 · 10 comments
Closed

Tooltip/Popover container option is not implemented #17755

BenjinYap opened this issue Oct 1, 2015 · 10 comments
Labels
has-pr js v4
Milestone
v4.0.0-alpha.6

Comments

@BenjinYap
Copy link

V4 documentation shows the container option exists but it is not actually implemented in the code.
http://v4-alpha.getbootstrap.com/components/tooltips/#options
https://github.com/twbs/bootstrap/blob/v4-dev/js/src/tooltip.js#L28

Also found 2 unit tests which attempt to use the option even though it is not implemented.
https://github.com/twbs/bootstrap/blob/v4-dev/js/tests/unit/tooltip.js#L331
https://github.com/twbs/bootstrap/blob/v4-dev/js/tests/unit/tooltip.js#L661
@tim-peterson
Copy link

I'd like to contribute to this module, but can anyone summarize why just copy/pasting the v3 code wasn't done?

I'm a first-time contributor, so missing the context for this "oversight".

@amites
Copy link

amites commented Apr 13, 2016

issue seems to be

https://github.com/twbs/bootstrap/blob/v4-dev/js/src/tooltip.js#L275

I'm gonna make a fork and see if I can time box a fix

@amites
Copy link

amites commented Apr 14, 2016 •
edited
Loading

I'm out of time to hack on this -- it's not a simple issue given the Tether dependency

  • changing the appendTo is ignored
  • moving when appendTo is triggered also ignored
  • moving DOM element after creation gets real hacky and doesn't display correctly (this is where I stopped)

happy to share notes with anyone that picks up this ticket

@cvrebert cvrebert mentioned this issue May 17, 2016
Closed
@cvrebert cvrebert mentioned this issue Aug 27, 2016
Closed
@andyexeter
Copy link
Contributor

Has there been any movement on this issue? Seems to have gone a bit quiet but the container option still isn't even in the source code for popover.js or tooltip.js in v4.0.0-alpha.4

@rutger1140
Copy link

@andyexeter I also bumped into this issue right now with the same findings as you. Too bad there is no fix available for this yet.

@andyexeter
Copy link
Contributor

@lekkerduidelijk hopefully two people bumping the issue will create a bit of movement on it :)

@amites when you say "changing the appendTo is ignored" what do you mean? If you change the appendTo to document.getElementById('my-id') and not document.body it still uses the body as the container?

@andyexeter andyexeter mentioned this issue Sep 15, 2016
Closed
@andyexeter
Copy link
Contributor

andyexeter commented Sep 15, 2016 •
edited
Loading

I wasn't experiencing the issue @amites was seeing with the appendTo being ignored. I've submitted a PR (#20725) but one of the tests is failing at the moment because it's passing an element as the container option instead of a string or boolean. Just waiting on feedback before I either update the test, or update the docs/code to accept an element.

@andyexeter
Copy link
Contributor

andyexeter commented Sep 15, 2016 •
edited
Loading

Looks like I spoke too soon with the previous comment. In my testing, certain elements were working fine as the container and others had the popup moved to the body after this._tether was set.

I had a look through the Tether source code and read this PR and I know what's going on now.

Tether moves the popover/tooltip to the body if the container's position property is not static. I guess there just needs to be a mention of this added to the docs for the option.

@zacechola zacechola mentioned this issue Sep 19, 2016
Closed
@mdo mdo added this to the v4.0.0-alpha.6 milestone Oct 23, 2016
@mdo mdo added the has-pr label Nov 26, 2016
@mdo mdo mentioned this issue Nov 27, 2016
Closed
@mdo
Copy link
Member

mdo commented Nov 27, 2016

Closed with manual merge of #20725.

@mdo mdo closed this as completed Nov 27, 2016
@s-filko
Copy link

s-filko commented Aug 9, 2017 •
edited
Loading

Tether moves the popover/tooltip to the body if the container's position property is not static.

@andyexeter is right.
But I found out that all parent nodes of the container must be static too.
If at least one of them is not static it will cause the popover/tooltip will be appended to the <body> element.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
has-pr js v4
Projects
None yet
Milestone
v4.0.0-alpha.6
Development

No branches or pull requests
8 participants
@amites @mdo @rutger1140 @cvrebert @tim-peterson @s-filko @andyexeter @BenjinYap