Allow to register an event when the contextual menu is hidden

[MissChocoe]

I need to perform something when the contextual menu is closed even without any action taken. I used a MutationObserver on my side right now to achieve that but find it a bit ugly. Would be nice to be able to know when a menu displayed with ctxmenu.show is hidden.

[nkappler]

Thanks for your feedback, that's a very valid feature request.

I am currently in the middle of a bigger refactoring, so I will need to investigate whether the implementation for this can be done before the refactoring is finished.

I thought of alternate ways to achieve this and while also not very pretty, I came up with this:
In the beforeRender callback of the menu, or in your case before calling ctxmenu.show, you could attach a transparent 'glass pane' as clickAwayHandler between the body and the menu.
In its onClick handler you would only need to call ctxmenu.hide and your own logic.

It's not as simple as a dedidacted API, but maybe it's easier than the MutationObserver.

---

Some questions about your feature request:
How would you imagine the usage of this event handler? I think there are three options:

1. Extend the attach/update/show method to register an individual onClose handler
2. Having a global onClose event that is thrown for every menu and is passed the menu's parent selector to identify the menu
3. A new dedicated API to attach onClose listeners via the same selectors menus are attached with

I think option 1 is my preferred option, although it is the most effort. I guess the other options may have limitations for menus that are opened via show, passing an event and not a selector or element.

Please let me know your thoughts.

[MissChocoe]

#1 indeed seems to be the best option. There's already a way to register on events for the menuItems; it would fit with the current patterns.

For #2 (and even with the current implementation), what would help is to be able to assign an id to the contextual menu or some custom class. When I tried to find the menu, I realized the "ctxmenu" class was already used by another node in my app. Luckily, it wasn't on the body.

If you do add the onClose, I imagine others will want the onShow! :)

Great work, I enjoy using your plugin. It is simple and flexible. Allowed me to clean up a lot of code that was used to create our previous menu.

[nkappler]

Great work, I enjoy using your plugin. It is simple and flexible. Allowed me to clean up a lot of code that was used to create our previous menu.

Thanks for the kind words, simplicity always was - and still is - one of the main goals of this project!

There's already a way to register on events for the menuItems;

Yes, I also thought about that, although these differ from your use-case in two important ways:

1. As you mentioned, they are menuItem specific, to define event listeners for the entire menu, we would need some kind of invisible "configuration item" (singleton), which seems cumbersome,
2. These are standard (i.e. EventTarget) events, there's barely any logic to these, it's just a wrapper around the browser API. Mixing in our own events feels wrong and might require extra logic to differentiate them and avoid name conflicts.

I think I'm going to deprecate the current signature of attach and update and replace the beforeRender parameter with a config object. Internally it can build the config object when a function is passed, so the change is not immediately breaking user code:

// deprecated:
declare function attach(target: string, ctxmenu: Array, beforeRender?: (menu: Array, event: MouseEvent) => Array)
declare function update(target: string, ctxmenu?: Array, beforeRender?: (menu: Array, event: MouseEvent) => Array)

// new:
declare function attach(target: string, ctxmenu: Array, config?: {
  onBeforeShow?: (menu: Array, event: MouseEvent) => Array;
  onShow?: Function,
  onBeforeHide?: Function,
  onHide?: Function,
  // etc. ?
})

This opens up the possibility to have more lifecycle handlers (you mentioned onShow, which basically was the beforeRender, but now we can have handlers before and after the menu is attached to the DOM) and the config object is infinitely expandable, we could also have browser event handlers (like onHover) in an event member, id, name, class or other attributes...

I'm not sure if I'm going to implement all of these explicitly, maybe I'm gonna add a generic attributes member.
After all, simplicity is very important to me and that applies also to the internals, not just the API...

This config option will of course also be available for the show method, although I already foresee some minor caveats, since there might not be a MouseEvent or a target for the handler, but I guess that's a problem for the application developer 😆

I am going to build a prototype in the next days and see where it leads me...
Again, please let me know if you have any feedback, or if you want to contribute.

[MissChocoe]

config is a good way to go. Method parameters that become huge is a bad habit in my own app that I'm trying to break!

Regarding the attributes member that would be applied to the ctx menu dom, it would solve another of my issues which is to customize the styling for some contextual menus by giving them ids (could use class but you would need to consider this a bit different not to remove your default classes). For the menuItems, I used div only to be able to add a class to it, so those too could benefit from this attribute. It would allow me to simplify my code. But I'm diverting from my original issue! ;)

I'd love to help but right now the timing isn't good. However, I'd be happy to test your solution in my environment if needed.

[nkappler]

there's the style property which basically is also just a wrapper around the vanilla JS attribute, but yes, using global CSS rules to target specific elements, currently is ... cumbersome.
You could use the style property as your identifier though, for example by setting a unique style, e.g. a z-index as ID { ... style: "z-index: 1", ...} and then using this as a selector: .ctxmenu [style="z-index: 1"]
It's a terrible hack, but it works 🤣 (z-index might be a terrible example, but I can't think of a better one right now)

There is a bunch of stuff I haven't considered initially cause I just needed a super simple context menu utility.
The more I think about it, the more use cases for the config object come to mind.

There's a bunch of stuff I still have planned and didn't start on, but I've made some progress in the refactoring and might begin a prototype implementation on sunday, alas I'm also very busy at the moment, so it might take a bit.

I'll keep you updated 😊

[nkappler]

adding a generic attribute support would also allow me to remove the style member alltogether, further simplifying the library.

The changes discussed so far, including deprecation changes, breaking changes and all the things that have already changed internally during the refactoring (not to mention a few bugs i found) feel like this all should be a major version bump, which I don't want to rush, but I'll try to get a solution ready for you as fast as possible...

[nkappler]

Hi @MissChocoe,

I've implemented a rough draft in the above mentioned commit, you can find it on the onHide branch.
Let me know if you need help installing, but the compiled sources (standalone and module) can be found there.

I've basically implemented all we've disussed so far,
config object replaces the onBeforeRender argument, although the old signature still works for now.

all members of the configobject are optional.
onBeforeShow is basically unchanged, it gets the menu config as a param and also needs to return one.
onShow has no params or return value yet, although I plan to pass the assembled DOM as a parameter.
onBeforeHide is called when the menu is still attached to the DOM, it receives the menu root container (HTMLUlElement)
onHide same as onBeforeHide but the menu has been removed from the DOM already.

both hide events receive the menu root element, however, they should already be able to know it from when it was attached.
This is not currently implemented but should be very easy and then we could instead pass the selected option, if possible.

---

I haven't had much time to test these changes myself, but I wanted to get something out of the door for you.
I would be surprised if everything works as I expect it to already 😄

I am very interested if this works for you and which of the above options are most useful to you, so please let me know your thoughts :)

[nkappler]

ok, fixed some bugs already... 🤣
Seems to do what it should, but please let me know if it fits your use-case.

[MissChocoe]

Thanks! I will try it out today!

[MissChocoe]

there's the style property which basically is also just a wrapper around the vanilla JS attribute, but yes, using global CSS rules to target specific elements, currently is ... cumbersome. You could use the style property as your identifier though, for example by setting a unique style, e.g. a z-index as ID { ... style: "z-index: 1", ...} and then using this as a selector: .ctxmenu [style="z-index: 1"] It's a terrible hack, but it works 🤣 (z-index might be a terrible example, but I can't think of a better one right now)

There is a bunch of stuff I haven't considered initially cause I just needed a super simple context menu utility. The more I think about it, the more use cases for the config object come to mind.

There's a bunch of stuff I still have planned and didn't start on, but I've made some progress in the refactoring and might begin a prototype implementation on sunday, alas I'm also very busy at the moment, so it might take a bit.

I'll keep you updated 😊

Somehow I missed that comment. Using style value to target elements is very creative! I would not have thought about it. I prefer my solution of adding a small div, looks less hacked!!! I could not use the style attributes directly because the style varies depending on the context.

[MissChocoe]

onHide works as expected and does fulfill my need. onBeforeHide and onShow are also called properly (and I see that you've already added the dom as a parameter of the onShow!) However, onBeforeShow doesn't seem to work.

[nkappler]

thanks for the feedback.

onBeforeShow currently is only called for menus that are permanently registered via attach or update.
I've noticed that yesterday, but fixing it is a little less trivial as implementing the other methods, due to some parameters that are getting passed around, but it's on my todo list.

I need to test it a bit more and I'll check how much effort it would be to implement the generic attribute config we talked about.

I'll probably release an update in the next few days :)

[MissChocoe]

Do as you see fit. We have working solutions for both (onHide and class attribute) so we will adjust. Using the new attributes config will require a bit more work so it won't be done for our current release but on the next one. Thanks for your work!

[nkappler]

Hi @MissChocoe,
I've pushed a small update to the onHide branch, fixing the issue that onBeforeShow wasn't working for ctxmenu.show.
I still need to rewrite parts of the documentation and improve the type declarations, before I can release the new version, but the implementation is done for now.

I have also written some unit tests for the entire new feature on the refactoring branch (which is also WIP), before cherry-picking the fix to the onHide branch, so I have a good feeling about the robustness of the implementation.

I have decided to postpone the attributes feature until the refactoring and all the tests are merged.
Feel free to test again, if you want and please let me know if something isn't working for you.

I assume I'll have the release ready in the next few days.

[nkappler]

Hi @MissChocoe

I've just officially released version 1.7.0, which includes this feature.
If you like this package, please consider giving it a star, it really helps with visibility.

Happy coding 😄

[MissChocoe]

Thank you very much, will include that in our next release!