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

Improve popover directive #99

Closed
pkozlowski-opensource opened this issue Jan 26, 2013 · 12 comments
Closed

Improve popover directive #99

pkozlowski-opensource opened this issue Jan 26, 2013 · 12 comments

Comments

@pkozlowski-opensource
Copy link
Member

Opening the issue for the popover so we can discuss with code examples.

@pkozlowski-opensource
Copy link
Member Author

So, if we simply base the popover on the same principles as tooltip we could use it like so:

<div popover="{{mymodel.content}}" popover-heading="Sth {{mymodel.title}}">
...
</div>

another alternative:

<div popover-content="{{mymodel.content}}" popover-heading="Sth {{mymodel.title}}">
...
</div>

we could also support a custom template:

<div popover-content="{{mymodel.content}}" popover-heading="Sth {{mymodel.title}}" popover-template="url here">
...
</div>

To be discussed...

@ajoslin
Copy link
Contributor

ajoslin commented Jan 26, 2013

If we start with dialog, it would be something like this probably:

$scope.myPopover = $popover('my-template.html', options);
<div popover="myPopover" popover-placement="right">
  Pop me!
</div>

@joshdmiller
Copy link
Contributor

If the popover should remain mostly an enhanced tooltip, then I very much like the basic syntax with popover and popoverHeading attributes.

But when we allow customization of the template, confining the popover to the "header + body" style seems a little strange. It's a directive template, so the developer could just change it anyway, right? When would one app need multiple styles of the same "header + body" popover? I think this would be more useful for having the popover display something completely different.

So I think it may make more sense to accept the template as a complete replacement, ignoring all other passed attributes:

<div popover popover-template="url">
...
</div>

And the template would just be evaluated against the scope. When popoverTemplate is provided, the directive ignores its default templating because it doesn't need to know anything about what will appear inside the popover; the popover only ever needs to focus on the mechanics of making it appear and disappear.

So there would be the above usage in addition to the "default" usage, which has the default Bootstrap template:

<div popover="{{popoverTitle}}" popover-body="{{popoverBody}}">
...
</div>

But I'm not 100% here...

@pkozlowski-opensource
Copy link
Member Author

@joshdmiller if I'm reading you correctly you are saying that we can imagine 2 usage scenarios for a popover:

  • with a "standard" template where we just need to pass a title and content for the body
  • generic one where a user should be free to specify any template

In both cases we need to decide what to do with a scope. For the title+body usage an isolated scope probably makes sense (one created manually). For the generic usage (with a template) a simple child scope (non-isolated) should be the way to go, right?

The above discussion leads me to thinking that those 2 use-cases might be a bit different, implementation wise. So, maybe we should just implement a generic variant?

I don't know if I can add much more to the discussion from the theoretical point of view :-)

@joshdmiller
Copy link
Contributor

@pkozlowski-opensource Hmmm... I hadn't gotten so far as scope yet. :-)

But ... here's some simplified pseudo-code:

.directive( 'popover-popop', function () { // <- default tpl, isolate
  restrict: 'E',
  scope: { title: '@', body: '@', isOpen: '&', ... },
  template: 'popoverPopup.html' // <- bootstrap default
})
.directive( 'popover-popop-generic', function () { // <- custom tpl, isolate
  restrict: 'E',
  scope: { isOpen: '&', ... },
  template: 'popoverPopupGeneric.html', // <- shell to add css class on isOpen
  transclude: true
})
.directive( 'popover', function () { // <- popover, child scope
  scope: true,
  link: function ( scope, element, attr ) {
    var tpl,

        // plus title & body, of course...
        defaultTpl = $compile('<popover-popup></popover-popup>')( scope ); 

    scope.isOpen = false;

    attr.$observe( 'popoverTemplate', function {
      // the child scope is fine here...
      var contents = // load template from url
      tpl = angular.element(
        '<popover-popup-generic></popover-popup-generic>' 
      );
      tpl.html( contents );
      tpl = $compile( tpl )( scope );
    });

   // yada yada...

    function openPopover() {
      tpl = tpl || defaultTpl;
      element.after(tpl);

      // positioning, etc.

      scope.isOpen = true;
    }

    // yada yada...
  },
})

@alexhanh
Copy link

+1

ballmw added a commit to ballmw/angularui-bootstrap that referenced this issue Jan 31, 2013
@ProLoser
Copy link
Member

Has there been any thought about using inline DOM as the template?

I want to use popover exactly like how dropmenu is used. You simply declare the popup copy very close to the target and all angular binding is retained. Essentially the ONLY thing I really need this directive to do is open and close the popup and position it centered.

@joshdmiller
Copy link
Contributor

@ProLoser Yeah, I was just thinking about this actually. I think there are three use cases to tease out:

  1. provide text and it displays according to a template (what we already have)
  2. provide HTML and it is bound unsafely to the existing template
  3. provide html and have it used instead of the global template (I think what you're after)

In terms of 2, this I think we'll do with a separate directive (like popover-html-unsafe) since the extra directive is so cheap with the new $tooltip service. E.g. <a popover-html-unsafe="{{myServerContent}}">Click me</a>

For 3, though, I'm not sure of the best way to do this. How do you see this working? Would it essentially be the same (i.e. a stringified template passed as an attribute)?

@joshdmiller
Copy link
Contributor

I also forgot about a fourth case, which was opened as #220: the ability to provide a template URL. E.g. <a popover-template="'path/to/template.tpl.html'">Click Me</a>.

@aamax
Copy link

aamax commented Mar 15, 2013

I'm new to angularjs (so be nice). am trying to use angular bootstrap ui in a project and added a popover. it works pretty well but I need to be able to add
tags to the content to format things better. I also want to set up mouseover or hover events so the popup shows and closes on mouseover rather than just on click events. so far the
just shows up as text so I'm missing something. it seems that angular gets in the way of my events processing for mouseover and I haven't figured out how to set html to true to allow the formatting to work right. any ideas/help?

here's a basic gist for what I'm doing: https://gist.github.com/aamax/5173870
----- the following added a few days later:
I've been able to get over this issue by using angularstraps popover. ... FYI

@jhiemer
Copy link

jhiemer commented Mar 21, 2013

+1 I think this could work quite well using popover as the container for a date or color picker.

@pkozlowski-opensource
Copy link
Member Author

There are several items discussed in this issue so it is hard to say which one should be worked on. Closing this one as a duplicate of a more focused #220

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

7 participants