Fixes #10

[cossssmin]

Removes the need for a menu container that covers the page. This way, we can catch the clicked target and close the menu as needed.

[electerious]

Looks good and works very well. Thanks! We just need to get around the mentioned onclick problem.

[cossssmin]

We just need to get around the mentioned onclick problem.

Not sure which one that is, is it not working as expected? If you're referring to this:

the user may try to right-click on a different element

That's already covered in the last commit (https://github.com/electerious/basicContext/pull/17/commits/3376994c9d9f584e1ea4043f9c94fd6616230f58). Clicking a different basicContext-enabled element while the menu is already open on one, produces the expected behavior: currently opened menu is removed, and the new one is opened on the new element that was clicked.

Or, maybe I'm missing something?

[electerious]

Not sure which one that is, is it not working as expected?

.onclick overwrites the original click-event of the parent element. We don't know the parent element. It could be anything. e.g. an element with an existing click-event and .onclick would overwrite it. addEventListener allows you to attach multiple functions for the same event without overwriting the existing ones. However, depending on how nested the button is that triggers the context and how the site uses event.stopPropagation(), it might fail to close in some situations. I guess this was the reason for me to use the transparent div. It has its downsides, but also eliminates a lot of edge cases.

The demos all work fine, because this case isn't covered.

I also see a problem with not blocking scrolling (previously done via overflow: hidden). This isn't covered in the demos, because you can't scroll the site in one of them. The context will either scroll with the site or stay fixed on its position, which will look kind of crazy for scrollable context menus.

[cossssmin]

Thanks for the explanation.

For the onclick, I don't really know what we could do.

For the overflow/scroll, could we have it enabled just in the case of scrollable context menus? Looking at the default browser context menu in Windows, indeed you can't scroll when it's opened; but then again, it also doesn't remove the scrollbar. So maybe setting overflow: hidden only for scrollable context menus, could be a compromise?

[rangitha]

@hellocosmin What's the status of this PR?

Is preventing default behaviour on right-click of basicContextContainer an intermediate solution for this problem?