feat(tooltip): Added *-append-to-body attribute #552
Conversation
The tooltip and popover directives (through the $tooltip service) now support using an attribute in addition to the provider to set a particular tooltip or popover to use $body as the container for the popup element rather than the directive element's parent. Closes angular-ui#395.
|
This is a really straightforward change; assuming there are no objections, I'll push this to master in a while. |
|
@joshdmiller you are a machine today :-) LGTM, the only remark would be this one: I'm not sure we need to observe the append to body option, I don't expect people to calculate those dynamically. So we could just skip observe and $parse it once getting rid of a slight (probably negligible) perf hit. Bu this is minor. |
|
@pkozlowski-opensource I thought about that as I was coding it. I can conceive of no real use case where one would need to change this dynamically; I did it only for consistency. There are a few of these attributes that do not need to be observed, but since I figured the performance hit was negligible I opted for consistency with the others. Perhaps I should open a PR (after this release) to refactor the observations out of there for most of them. |
|
Yes, I agree that perf hit is probably negligible (but once again, we don't know till we measure :-)). Let's merge it as-is but I think it would be good to refactor it in the longer run to remove observations from attributes for which we don't expect dynamic values. My 3 cents anyway... |
|
Landed as d089626 |
The tooltip and popover directives (through the $tooltip service) now
support using an attribute in addition to the provider to set a
particular tooltip or popover to use $body as the container for the
popup element rather than the directive element's parent.
Closes #395.