Add popoverHtmlUnsafe directive. by NateRedding · Pull Request #2387 · angular-ui/bootstrap · GitHub
Skip to content
This repository was archived by the owner on May 29, 2019. It is now read-only.

Add popoverHtmlUnsafe directive.#2387

Closed
NateRedding wants to merge 1 commit into
angular-ui:masterfrom
NateRedding:master
Closed

Add popoverHtmlUnsafe directive.#2387
NateRedding wants to merge 1 commit into
angular-ui:masterfrom
NateRedding:master

Conversation

@NateRedding

Copy link
Copy Markdown

Popover appears to be missing a directive corresponding to the tooltip-html-unsafe directive.

I have added tests similar to tooltip-html-unsafe, and updated the documentation. I also tried to name my commit similarly to how I see other commits named.

@kentcdodds

Copy link
Copy Markdown

@NateRedding

Copy link
Copy Markdown
Author

This accomplishes the same functionality as #2187, but does so following the same pattern as tooltip-html-unsafe.

@kentcdodds

Copy link
Copy Markdown

Yeah, looking at the code I think this is a great (and consistent) implementation. Looks like the build passed. Could someone please merge this?

@NateRedding

Copy link
Copy Markdown
Author

This is also a duplicate of #2276, but this PR includes unit tests and documentation updates.

@chrisirhc

Copy link
Copy Markdown
Contributor

This approach doesn't allow you to add directives or bindings in the content. Is this preferred over #1848 ?

@NateRedding

Copy link
Copy Markdown
Author

It looks like your approach would be preferable. This implementation was intended to complete the framework with popovers in the same fashion as was done with tooltips. For my (unfortunate) case of getting the HTML content that comes with my data into popovers, I believe #1848 provides an approach that would work as well. So, if that one is going to be merged, we shouldn't need this one.

@wesleycho

Copy link
Copy Markdown
Contributor

@wesleycho wesleycho closed this Mar 31, 2015
@karianna karianna added this to the 0.13.0 milestone Mar 31, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants