The resource button is triggering twice

[karthickrk1]

Subject of the issue/enhancement/features

The resources button is triggering twice and displaying the drawer twice. When I track the click event of this resource button, I notice that the event is triggering twice and opening the drawer twice. Visually, we may not be able to observe it, but if you track the code in the debugger, you can see that the resource button is being triggered twice.

Your environment

• version (AT/Framework) : 5.31.11
• which browser and its version : Chrome 114.0.5735.199
• device(s) + operating system(s) : Window 10

Steps to reproduce

Create a course and enable the adapt-contrib-resources extension. Provide all the necessary details for the resources extension. Preview the course and click on the resource button. The drawer will open.

Expected behaviour

The resource button should trigger once when clicked.

Actual behaviour

The resource button triggers twice when clicked.

Screenshots (if you can)

If you observe the screenshot, you will notice that the showDrawer function is being triggered twice. As a result, the drawer is also appearing twice.

[swashbuck]

First off, I believe this is a bug with adapt-contrib-core rather than Resources.

So, clicking the drawer button triggers the toggleDrawer event. This is present on the button as data-event="toggleDrawer".

https://github.com/adaptlearning/adapt-contrib-core/blob/6a0014b60a2d3f5ea20f867ff06f4bc936bac094/templates/nav.hbs#L23

Drawer listens for this event and responds by calling toggle() and then open() if the drawer is closed. This then calls showDrawer() in the drawerView.js.

I believe the problem is when Resources calls drawer.triggerCustomView (deprecated in favor of openCustomView). Then, openCustomView in drawerView.js calls showDrawer() a second time.

I think a simple fix would be to exit early from showDrawer() if the drawer is already open using if (this.isOpen) return;

https://github.com/adaptlearning/adapt-contrib-core/blob/6a0014b60a2d3f5ea20f867ff06f4bc936bac094/js/views/drawerView.js#L112-L113

showDrawer(emptyDrawer, position = null) {
  if (this.isOpen) return;

  this.setDrawerPosition(position);

@oliverfoster What do you think?

[oliverfoster]

Yup. Sounds fine @swashbuck It might have other consequences though, this is as you'd be fixing a bug from lower down the callstack up in the common / intersecting function. It might be better to fix triggerCustomView and/or move resources away from it if it's deprecated? Perhaps? Fix the bug at its origin.

Just as a sidenote @karthickrk1 it's possible to turn on source mapping in the AAT so that you don't have to debug in a minified file. I think it's in the project settings.

[swashbuck]

Just as a sidenote @karthickrk1 it's possible to turn on source mapping in the AAT so that you don't have to debug in a minified file. I think it's in the project settings.

@oliverfoster Thanks, I learned something new today. To clarify, it's in the Configuration Settings rather than Project Settings:

[swashbuck]

Created #117 to replace the deprecated function name.

Still looking into the core issue here, but it seems to be present in at least some other plugins (e.g. Glossary).

[swashbuck]

Ok, looked into this a bit further. Some of this might be rehash from above.

When clicking the main drawer button, the toggleDrawer event is fired due to data-event="toggleDrawer" and then toggle() is called (which eventually calls showDrawer). This happens with any plugin that uses the dedicated drawer button (e.g. Resources, Glossary) via drawer.addItem(). Then, viewing the custom drawer view calls showDrawer a second time.

For plugins that open the drawer directly (e.g. Language Picker) and are not added to the DrawerCollection via drawer.addItem(), drawer's toggle() is not called. So, showDrawer is only called once.

I don't think that this has any crucial negative effects. Some observations:

1. Pro: The drawer elements are not being duplicated in the DOM.
2. Con: drawer:opened is possibly triggered multiple times: Once when the drawer is opened and once when the custom View (e.g. Resources) is displayed. So, other scripts that rely on this event may find it to be an unreliable indicator of when the drawer is actually opened.
3. Con: Some drawer attributes like aria-expanded may be unnecessarily reset to their existing values, which decreases code efficiency.

Perhaps showDrawer() can be reworked or split into multiple methods since the name is a bit confusing? This method is currently responsible for several different functions like hiding/showing the drawer's Back button. hideDrawer() should be refactored at the same time for consistency.