moneyadviceservice/dough

Errors when trying to use VisibilityToggler

Closed this issue · 5 comments

We're try to use the VisibilityToggler module, but we get an error on page load when we include it:

Uncaught Element not supplied to MASModule constructor MASModule.js:17
MASModule MASModule.js:17
(anonymous function) VisibilityToggler.js:51
context.execCb require.js?body=1:1659
Module.check require.js?body=1:875
(anonymous function) require.js?body=1:1122
(anonymous function) require.js?body=1:133
(anonymous function) require.js?body=1:1165
each require.js?body=1:58
Module.emit require.js?body=1:1164
Module.check require.js?body=1:926
Module.enable require.js?body=1:1152
Module.init require.js?body=1:783
callGetModule require.js?body=1:1179
context.completeLoad require.js?body=1:1553
context.onScriptLoad require.js?body=1:1680

Should it be using MASModule.extend instead perhaps?

I've pushed an upgraded copy of VisibilityToggler to the dough branch 'js-toggler'

Thanks for that @jonwyatt-mas. It's fixed the "Element not supplied" exception, but we're now facing a different issue…

We have code like this:

<a data-mas-component="VisibilityToggler" data-mas-toggler=".toggle-me">Click me…</a>
<p class="toggle-me">…to toggle me!</p>

The example at the top of VisibilityToggler.js, and the code below it, lead us to believe clicking the link should add a class of "is-active" to both the <a> and the <p>. And clicking it again should remove the class from both.

However, we're only seeing the "is-active" get added to and removed from the <a>. The <p> is left untouched.

I'm not 100% accustomed with your code, but I think maybe line 64 is wrong. this.$el.find(…) searches for an element with the class .toggle-me inside the element with the data-mas-component="VisibilityToggler" attribute, which in our case is the <a>. So it'll never find it.

We could put the data-mas-component="VisibilityToggler" attribute onto a common ancestor of the two elements, but then we'd also have to move the data-mas-toggler=".toggle-me" to that ancestor (since line 64 calls this.attr('toggler') which assumes the data-attribute is on this.$el), and if we did that, then the script wouldn't know where the trigger elements are.

The code, as-is, appears to only work if the element that gets shown/hidden is actually inside the element that triggers the action. Is that the intention?

Hi @zarino - thanks for the detailed report. To your last paragraph, no that is not the intention and is a bug.

We'll get that fixed up asap.

I've refactored - #65

@zarino - #65 merged, you will need to adjust your markup - please use the test fixture in dough (path is in the comments at top of VisibilityToggler) for an example.