Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SatPopover is not honouring the popover anchor position #167

Open
julianobrasil opened this issue Jul 11, 2019 · 4 comments
Open

SatPopover is not honouring the popover anchor position #167

julianobrasil opened this issue Jul 11, 2019 · 4 comments

Comments

@julianobrasil
Copy link
Contributor

julianobrasil commented Jul 11, 2019

In the previous version of SatPopover, there was this method: SatPopoverAnchor.openPopover(). When called, this method opened the popover wherever the anchor was.

Now, as there is no openPopover()/closePopover() anymore, I tried to achieve the same effect with SatPopoverAnchor.popover.open(), but when we have a bunch of anchors inside a *ngFor, for example, the popover is always anchored to the last element (I think that was the reason for having the openPopover()/closePopover() methods in SatPopoverAnchor class in the previous version):

<ng-container *ngFor="let i of [1,2,3]">
  <button #anchor="satPopoverAnchor" 
          [satPopoverAnchor]="p" 
          (click)="anchor.popover.open()">Toggle</button>
</ng-container>

<sat-popover #p verticalAlign="below" hasBackdrop>
  <div>
    Hello!
  </div>
</sat-popover>

Record_2019_07_11_15_06_41_796

You can try it here: https://stackblitz.com/edit/ncstate-sat-popover-issues-xxiynn?file=app/app.component.html

Is there another way to do what I want to? I cannot upgrade SatPopover to the last version without this feature.

@julianobrasil
Copy link
Contributor Author

julianobrasil commented Jul 11, 2019

Apparently, this is the cause of this (intended or not) behavior:

@Input('satPopoverAnchor')
get popover() {
return this._popover;
}
set popover(val: SatPopover) {
if (val instanceof SatPopover) {
val.anchor = this;
} else {
// when a directive is added with no arguments,

@julianobrasil julianobrasil changed the title Anchor is not honouring the popover position Anchor is not honouring the popover anchor position Jul 11, 2019
@julianobrasil julianobrasil changed the title Anchor is not honouring the popover anchor position SatPopover is not honouring the popover anchor position Jul 11, 2019
@julianobrasil
Copy link
Contributor Author

julianobrasil commented Jul 11, 2019

I noticed I can achieve the same effect I had with the previous versions by reassigning the popover anchor in typescript code. And I can get a reference to the popover through the anchor I'm handling (yeah, it's confusing, but take a look the snippet below), but the API, in this particular case became worse than it was.

<ng-container *ngFor="let i of [1,2,3]">
  <button #anchor="satPopoverAnchor" 
          [satPopoverAnchor]="p" 
          (click)="_openMenu(anchor)">Toggle</button>
</ng-container>

<sat-popover #p verticalAlign="below" hasBackdrop>
  <div>
    Hello!
  </div>
</sat-popover>
  _openMenu(anchor: SatPopoverAnchor) {
    anchor.popover.anchor = anchor;
    anchor.popover.open();
  }

@jorroll
Copy link
Contributor

jorroll commented Jan 1, 2020

@julianobrasil just reading this. The issue you are experiencing is a known limitation of the current API (SatPopovers can only have a single anchor at a time).

See this note referencing it in the original PR:

It occurs to me, I don't actually know if the library currently supports the ability to associate multiple SatPopoverAnchors with the same SatPopover. If the library currently does support this, then this PR removes that feature (and, obviously, it would be important to add it back in).

  • The PR ended up getting merged without addressing this limitation.

Your workaround, to dynamically reassign the anchor in code, is the right solution 👍.

Edit: FWIW I think you could also accomplish the same thing in the template. i.e.

<ng-container *ngFor="let i of [1,2,3]">
  <button #anchor="satPopoverAnchor" 
          [satPopoverAnchor]="p" 
          (click)="p.anchor = anchor; p.open()">Toggle</button>
</ng-container>

<sat-popover #p verticalAlign="below" hasBackdrop>
  <div>
    Hello!
  </div>
</sat-popover>

But handling this in typescript is generally clearer.

@julianobrasil
Copy link
Contributor Author

@thefliik, thanks for your answer. I hope we get back any openPopover()/closePopover()-like methods soonish.

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

No branches or pull requests

2 participants