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

Add popoverHtmlUnsafe directive. #2387

Closed
wants to merge 1 commit into from

Conversation

NateRedding
Copy link

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

If this build passes, could we please include this? It's sort of ridiculous how many PRs have been submitted for this functionality and how little love this feature has gotten. Please merge this.

@NateRedding
Copy link
Author

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

@kentcdodds
Copy link

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
Author

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

@chrisirhc
Copy link
Contributor

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

@NateRedding
Copy link
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
Contributor

Now that #1848 is merged, this can now be closed as completed :) .

@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