Skip to content
This repository was archived by the owner on May 29, 2019. It is now read-only.

tooltip-append-to-body="true" should be simply: tooltip-append-to-body #4495

Closed
gaultier opened this issue Sep 30, 2015 · 10 comments
Closed

Comments

@gaultier
Copy link

Hello,
When we use the tooltip element and we would like to append it to the body, we expect to use the option this way:

<p tooltip="my tooltip" tooltip-append-to-body>Hello</p>

But in fact the only possible usage is this way:

<p tooltip="my tooltip" tooltip-append-to-body="true">Hello</p>

This is counter-intuitive, hard to detect, and every other component does it the simple way.
This option should probably be used with the shortest syntax.

Could we either way add an example in the doc?

The only hint to use it that way is here: https://github.com/angular-ui/bootstrap/blob/master/src/tooltip/test/tooltip.spec.js#L538

@gaultier gaultier changed the title tooltip-append-to-body="true" should be simply tooltip-append-to-body tooltip-append-to-body="true" should be simply tooltip-append-to-body Sep 30, 2015
@gaultier gaultier changed the title tooltip-append-to-body="true" should be simply tooltip-append-to-body tooltip-append-to-body="true" should be simply: tooltip-append-to-body Sep 30, 2015
@icfantv
Copy link
Contributor

icfantv commented Sep 30, 2015

We don't have an $observe on the *-append-to-body attribute so we're not evaluating it for changes. I think this is a reasonable request. Please feel free to submit a PR w/ appropriate functional tests.

I ask that you please do the PR because there's nothing wrong with what we have now and it is working. Thus, it's probably going to be a very low priority for us to change.

@wesleycho
Copy link
Contributor

This would actually require a breaking change, and a potentially onerous one - these lines are the culprit. The change needed would be

            var appendToBodyVal = (prefix + 'AppendToBody') in attrs;
            appendToBody = appendToBodyVal || appendToBody;

I am inclined to reject this request.

@wesleycho wesleycho added this to the Purgatory milestone Oct 26, 2015
@ChrisMBarr
Copy link

This isn't true in all cases. I set up my app to set this option by default, like so:

angular.module("myApp")
    .config(["$tooltipProvider", ($tooltipProvider: angular.ui.bootstrap.ITooltipProvider) => {
        $tooltipProvider.options({
            appendToBody: true
        });
    }])

and I use tooltip-append-to-body="false" on the occasion when I need it.

@RobJacobs
Copy link
Contributor

Valid point by @ChrisMBarr, my vote: 👎

@wesleycho
Copy link
Contributor

Great point @ChrisMBarr - closing as invalid.

@gaultier
Copy link
Author

Either way I think a precision/example in the documentation would be nice because this option is present on none of the existing examples.

@deje1011
Copy link

totally agree with @gaultier, it would be nice to mention this in the documentation as it is not the expected behavior (in my opinion).
Currently it just says:
"tooltip-append-to-body: Should the tooltip be appended to $body instead of the parent element?"

Why can't the attribute be checked to be neither undefined nor false though?

@icfantv
Copy link
Contributor

icfantv commented Nov 26, 2015

@deje101, maintaining this OSS library requires a fair amount of work by many people. Please extend us the courtesy of searching both open and closed issues before commenting. There is an open issue from a few days ago, #4945, that is tracking this.

@deje1011
Copy link

Sorry @icfantv, I just randomly found this thread when I was searching for a solution on google. I will check for new issues before posting next time.

@icfantv
Copy link
Contributor

icfantv commented Nov 26, 2015

No worries and thanks.

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

No branches or pull requests

6 participants