Fix #9427 - Dropdown menu mouseout ignored

[ncoden]

Fix #9427

What happens:
The hide delay of the visible dropdown is replaced by the delays of the lastly triggered dropdown, so the hide function is never called on the currently visible dropdown.

Solution:
Remember and cancel the delay inside each element. So a visible dropdown remember its hide delay even if several dropdown must be hidden.

Changes:

• Use this.delay instead of _this.delay
• Add a visual test

⚠️ I don't know the Foundation JS enough. I checked the dropdown work (even with different show/close delays options), but I can't ensure this is a correct fix.

[kball]

I don't think this fix is quite right... This changes us from putting the event handler on a shared JavaScript object (the Dropdown component) to putting individual handlers attached to the elements themselves, which may not get cleaned up properly.

This approach (breaking it down to the individual dropdowns) may be correct, but we should put these on the JavaScript object (e.g. _this.handlers[elem]) so that they are handled properly by our object cleanup during destruction.

[ncoden]

What do you mean by elem ? What can I use as index to identify the element ?

The other solution is an array {elem: this, delay: ...}, but it is much more complicated to find and delete a data.

[kball]

Sorry was thinking $elem... essentially using a hash (oh wait JS... Object) with the element as the key, rather than the element itself as a place to put delay

e.g. in init

this.delays = {};

and then in the code mentioned

clearTimeout(_this.delays[$elem])
_this.delays[$elem] = setTimeout(
...

Now all of our timeouts are on the JS object itself, so if this JS object gets destroyed they should get cleaned up appropriately.

[ncoden]

I already tried what you proposed, but we can't use a jQuery or DOM element as an index, because it is converted to a string "[object Object]".

See: https://codepen.io/ncoden/pen/qqYqYY

[kball]

bother. What if we do something like

var idx  = _this.$menuItems.indexOf($elem[0])
clearTimeout(_this.delays[idx])
_this.delays[idx] = setTimeout(
...

[ncoden]

I find it dirty :/
_this.$menuItems.indexOf($elem[0]) is not really an identifier on the element. There should be cases (if the element is moved ?) where it would crash.

[kball]

yeah... the more I'm fighting with this the less I see an ideal solution, or at least not a fast one... maybe just stash it on the elem as you have it but then add a cleanup in destroy()?

[ncoden]

Wasn't there components that has faced the same problem ? Attach datas to a specific element seems to me to be ordinary.

[kball]

@ncoden data isn't problematic, the issue I've run into in the past is with JS closures. You can end up in a situation where you have a reference from JS to elem and elem to JS where the browser doesn't know how clean it up correctly automatically.. we stash data on elements all the time, but try to house all of the logic inside a single attached object that we then explicitly clean up

[ncoden]

So if I had a set of functions to handle an array [ {elem:... , data:...} ] as a map, it could be reused in Foundation later ?

[kball]

@ncoden I'd say so.

[ncoden]

Ok. I will do this.

[ncoden]

So, I something that can be used like this:

var elemDatas = Foundation.ElemStore(this).datas;
elemDatas.delay = ...;
elemDatas.data = ...;
...
Foundation.ElemStore(this).destroy(); // or .delete()

Before I realized this is almost the same thing as jQuery $elem.data('delay', ...);: The only difference is jQuery .data take HTML5 data attributes as default value.

@kball Should I rather use jQuery .data ? I know you planned to drop jQuery, but I don't know why.

[kball]

It's fine to use .data. I want to refactor jQuery out at some point but that is going to be a long project and one more instance of data won't hurt.