My impressions on alpha.3 about popover HOT 15 CLOSED

I had never used ng-container to isolate template variables before. Didn't know it was possible to do it. This is perfect. It's never too late to learn the obvious. 😄

from popover.

did you ever figure out a good way to get the onOpen event working to show immediately?

No. I've noticed material tooltip has the same behavior. We are not alone 😄

In the back of my mind, I'm considering extending the anchor in ways like [satTooltipAnchorFor]="tooltipPopover" and [satMenuAnchorFor]="menuPopover"

You can be proud of your work. I wouldn't change it. It's so easy to use and so versatile at the same time. I doubt someone will ever complain.

Oh, and I haven't close this because I was not sure whether you were using it to track anything. IMO it can be closed. The onOpen thing is not directly related to satPopover. It's kind of a general js question.

from popover.

BTW, I really liked that speed dial example in stackblitz.com.

from popover.

What if we think in a way to send a component as a parameter dynammically, using portals, like mat-snackbar does (#23)? Or maybe a more self-contained approach, like matTooltip (that doesn't need an anchor)? I mean, the way it is built now, working like mat-menu, IMO it's not ready to work with mat-select (or mat-select isn't ready to work with sat-popover, mat-menu or anything like them, that has the possibility to react to hover events).

from popover.

I've built an working example on stackblitz.com: https://stackblitz.com/edit/popover-example

The red color under each option helps to show the problem by marking the sat-popover anchor. You'll have to hover the mouse exactly over the red region in each option to see the popover.

Then, if you uncomment the last lines in styles.scss, you can make it a lot better, but cannot get rid of the problem completely.

from popover.

Another important observation is that you will also notice that when the mat-select panel opens, the popover is not fired - you have to shift the mouse pointer a little bit to fire it.

Also, both mousenter and mouseleave have accessibility issues. The ideal events to trigger the popover should be the combination of the mouse events with focusin and focusout, which can be achieved by keyboard arrows (but without triggering twice the popover) [edited => in this case, I think there is an issue with mat-select, which doesn't emit anything nor give a clue about what is currently focused while you navigate throughout the options - maybe this is something that Kristiyan (crisbeto from Material team) should take care of].

This made me think whether making the hover (specifically) event an out-of-the-box feature isn't a good idea. Right now I haven't figured out a solution to fire the popover on select panel's open event (Should I have to code anything in mat-select's onOpen? what?). The panel is already opened, but it seems the mouseenter event was never emitted.

[edit => BTW, bootstraps's popover I'm currently using in my other project, also have the same behavior: until I move the mouse over the option, I don't see the popover]

from popover.

First, I'm so happy to see that you're putting it through its paces so quickly! The feedback is wonderful!

Re: option templating, I think ng-container would solve everything here. StackBlitz. I may have missed something about the need to encapsulate in a separate component, but if you just anchor directly to mat-option, it should position correctly.

Re: dynamically creating and controlling popovers, between ngFor and ViewChildren, I think most cases should be covered. I'd like to get to snackbar-like usage so more logic can be encapsulated inside a popover, but I'm more concerned about being able to easily set a scroll strategy or focus behavior.. and those pesky tests...

Re: hover/focus events, I'll try to play around with it sometime this week and see what I can come up with. If there are no ways to determine which option is currently active, I agree, there should be.

Maybe you can combine elementFromPoint and the select's onOpen? Just brainstorming...

from popover.

I've sent it to production. It'll be used for about 50 users (3 to 4 of them on a daily basis) and I'll keep an eye on it for eventual issues, but I honestly think that there will be any.

[edit]: "... honestly think that there WON'T be any"

from popover.

Haha yeah, placing ngFor or ngIf on ng-container blew my mind when I realized it was possible. I dunno if it'll help either, but reassigning things inside ngIf is another neat trick, especially if you don't want to keep subscribing all over your template,

<ng-container *ngIf="user$ | async; let user;">
  <div class="name">{{ user.name }}</div>
  <img [src]="user.img">
</ng-container>

I think I will still remove the default click behavior in the short term. A longer term goal may be to have something like

<div [satAnchorFor]="myPopover" satTogglePopoverOn="hover focus longpress">
  ...
</div>

<div [satAnchorFor]="differentPopover" satTogglePopoverOn="focus click">
  ...
</div>

from popover.

I think this would be nice because of some caveats, like disabling the backdrop on hover events.

In this case,what would be the default behavior:

1. Nothing: sat-popover would throw an exception if satTogglePopoverOn wasn't explicitly set?
2. Nothing: the popover simply isn't shown
3. One of the pre-built events

from popover.

Probably 2. I'm afraid though that it will get complicated to prioritize defaulted behavior if you have conflicting needs. As in,

• How does focus work with a focus trap?
• What if you want to open on mouseenter but then click to close (I've seen some discussions on /material2 about using mouseenter to quick-open menus)

A better solution may just be to briefly explain in the readme how to create a variety of behaviors, what the backdrop does, and StackBlitz demo(s) showing several common use cases. Now that you understand that the backdrop blocks hover events, it makes sense, right? I think if that were presented upfront in the README, it wouldn't have been so perplexing.

from popover.

How does focus work with a focus trap?

😲 I haven't thought of that...

A better solution may just be to briefly explain in the readme how to create a variety of behaviors, what the backdrop does, and StackBlitz demo(s) showing several common use cases.

I agree.

from popover.

@julianobrasil did you ever figure out a good way to get the onOpen event working to show immediately?

I think this issue can be closed unless I'm forgetting something. There is a blurb in the readme about how the backdrop can effect hover behavior, and it's now opt-in, which I think helps.

In the back of my mind, I'm considering extending the anchor in ways like [satTooltipAnchorFor]="tooltipPopover" and [satMenuAnchorFor]="menuPopover" where they have opinionated baked-in behavior. I figure the best option is to wait and see if people have issues with the current behavior, or find it annoying.

from popover.

Will, I've noticed today that the combination SatPopover + MatSelect is working just fine in the onOpen event (showing immediately when MatSelect's panel opens: we've been talking about it since #35 (comment)).

Have you done anything? Or have Material guys done?

[edited 1]: Or have Chrome guys done? I've checked out your old stackblitz example and it's working fine (Alpha.3).

[edited 2]: I think Chrome guys did something: the issue remains in Edge.

from popover.

Good catch! It did not work in v61 but does in v62

from popover.