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

Dynamic context menu open issue #95

Open
sherlock1982 opened this issue Mar 12, 2018 · 4 comments
Open

Dynamic context menu open issue #95

sherlock1982 opened this issue Mar 12, 2018 · 4 comments

Comments

@sherlock1982
Copy link

Currently I need to use dynamic context menu like this (approximately):


    public onContextMenu($event: MouseEvent, contact: Contact): void {

        $event.preventDefault();
        $event.stopPropagation();

        this.getContextMenuAction(contact).subscribe( (actions: ContextMenuAction[]) => {
            this.contextMenuActions = actions;
            setTimeout(() => {
                this.contextMenuService.show.next({
                    // Optional - if unspecified, all context menu components will open
                    contextMenu: this.basicMenu,
                    event: $event,
                    item: contact,
                });
            });
        })
    }

onContextMenu by itself is a click event somewhere on a button.

<context-menu #basicMenu>
    <ng-template *ngFor="let action of contextMenuActions" contextMenuItem (execute)="action.execute()">
        <i class="{{action.icon}}"></i><span>{{action.translationTag | translate:action.translationArgs}}</span>
    </ng-template>
</context-menu>

Though if I remove setTimeout two issues appear:

  1. First time context menu is shown without items.
  2. execute is not fired though menu is closed on click

Is there a reason for setTimeout call or am I doing something wrong?

@isaacplmann
Copy link
Owner

Short answer: Yes, you need the setTimeout.

Long answer:
I haven't seen anyone try to do something like this, but here's what I'm guessing is happening.

Without the setTimeout:

  1. this.contextMenuAction is initially undefined (or [] if you set it to that)
  2. <context-menu> sees no contextMenuItems
  3. Setting this.contextMenuActions does not immediately trigger change detection on the <context-menu> so it uses the old values.
  4. The first click shows no items.

With the setTimeout:

  1. and 2. same.
  2. Having setTimeout allows a change detection cycle to run in the <context-menu> component so that it picks up the new <ng-template contextMenuItem>s.
  3. The first click shows the new items.

I have no good explanation for why the execute isn't fired.

@sherlock1982
Copy link
Author

Thank you for the explanation.
What I was trying to achieve is this simple scenario:

  1. User clicks on some element
  2. Some async requests might step in to form contextMenuActions.
  3. Context menu opens with generated contextMenuActions.

I think it might make sense to update your example with Dynamic context menu. Cause it actually demonstrates static context menu. It's important that after contextMenuActions are generated new change cycle should take place.

@isaacplmann
Copy link
Owner

Yes, that's a valid use case and I agree that the <context-menu> should handle it more elegantly, but it currently doesn't. Want to submit a PR to update the docs? I'm always looking for help and you can write clear explanations.

@bergermanuel
Copy link

We're having the same issue with using dynamic context menus. When looking at the code we identified the issue being the ContentChildren Input inside the ContextMenuDirective. As you mentioned it's the changeDetection that didn't update the values between setting the items and opening the context menu.

We also tried to call the changedetection manually between the two lines of code but didn't work. Also calling setTimeout is not working properly. We have to use a specific time of milisecods to get it working.

      this.contextMenuActions = actions;
      this.triggerChange();

      if (!actions || !actions.length || actions.every(a => a.hidden)) {
        return;
      }

      setTimeout(() => {
        this.contextMenuService.show.next({
          event: event.event,
          contextMenu: this.contextMenuComponent,
          item: null
        });
        this.triggerChange();
      }, 450);

In my opinion it's a very common use case to have one contextmenu and setting the entries dynamically with an ngFor. We're looking for a clean way to solve this, do you have any suggestions for us?

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

3 participants