cbroeren / ember-context-menu

An ember addon for custom context-menu
https://cbroeren.github.com/ember-context-menu
MIT License
23 stars 15 forks source link

Add option to compute context items on init #23

Closed Ericky14 closed 7 years ago

Ericky14 commented 7 years ago

See pull request #22

cbroeren commented 7 years ago

I've seen your PR, unfortunately that wouldn't work. Would it work to do something like this in your component?:

contextItems: computed(function() {
  return [];
})
Ericky14 commented 7 years ago

That would not work because context items is computed only once.

I am trying to write a file manager component and I need the context items to change each time I right click somewhere. if I just used computed, it will only be computed the first time.

Why would the PR not work? I have tried it and it fixed my problem.

cbroeren commented 7 years ago

You're right the items will only compute the first time, on init, when you just use the compute like my example above. The way ember is working, you should define an attribute to observe and trigger the compute function to recalculate. I'm using it myself for checking whether an object is selected (only selected objects can trigger the contextMenu in my application, like the example on the ember-context-menu testpage):

contextItems: computed('object.selected', function() {
  if (this.get('object.selected')) {
    return [{ ... }];
  }
  return [];
});
Ericky14 commented 7 years ago

That's not what I meant.

In the test page, you have to select before right clicking. I meant selecting at the same time as right clicking.

cbroeren commented 7 years ago

Now I understand what you mean. That's something else than you said before. I'm ok with adding support for calling the _contextMenu function before the context-menu functionality.

cbroeren commented 7 years ago

See #25. Please let me know if that's ok for you. In this way, it's very generic, and everyone who wants to have some actions on the rightclick as well as the context-menu, can use it.

cbroeren commented 7 years ago

Published it on version 0.3.1

Ericky14 commented 7 years ago

I don't think the scope of the contextMenu is passed to the private function since this is undefined when _contextMenu is called.

Do you think it could be called like a normal function instead of using invoke?

Also, I had found a work-around for the problem I had, just thought I'd mention it too. You can simply use contextMenu inside your own component and then after doing what you need to do, do this._super(...arguments) and it works.

cbroeren commented 7 years ago

That's right, I guess you don't need the scope of the contextMenu function, but of the component to get any other attributes right? Defining a function directly on the component, won't allow you to use a this.get / this.set (only the default javaScript functions will work that way, like contextMenu, click, etc..). In order to make that work, you need to define the function as a computed property:

  _contextMenu: computed('selectionHandler', 'file', function() {
    let file = this.get('file');
    // get any attribute you need with this

    return (e) => {
        selectionHandler.select(file, e);
    };
})

And you're absolutely right about overwriting the contextMenu function and using the this.super. It even could be the most clean way :+1:

May I assume your problemen is fixed now, so I can close this issue?

Ericky14 commented 7 years ago

Yes you can, thank you for helping.

cbroeren commented 7 years ago

NP, always available for some help. Please let me know if you find something else in using this addon.