Skip to content
This repository has been archived by the owner on Oct 16, 2023. It is now read-only.

Can't add additional modals dynamically #6

Open
zackkatz opened this issue Jul 18, 2018 · 7 comments
Open

Can't add additional modals dynamically #6

zackkatz opened this issue Jul 18, 2018 · 7 comments
Milestone

Comments

@zackkatz
Copy link

I'm trying to initialize modals for items added via an infinite scroll script. It isn't working, however, because var modal isn't accessible globally.

The scoped modal variable is throughout the script, like in setupModal() and setupTrigger() methods. There's no way to modify this important variable.

Ideally, I'd like a way to be able to add modals to the modal variable, or add a method that would reset modal, then run init() again.

zackkatz added a commit to zackkatz/accessible_modal_window that referenced this issue Jul 18, 2018
See scottaohara#6 for more information.

I know this isn't the best solution, but I'm not a great JS developer.

Another alternative would be to add something like this:

```
/**
	 * @param {Element} item
	 */
	ARIAmodal.addModal = function ( item ) {
		modal.push( item );	
	};
```

and then refactor `setupModal` so it runs only on one modal at a time, then add a new `setupModals` method that would loop through all the items in the `modal` array and run `setupModal` on them.

Again, I'm not great at JS. Thank you for the script, and please let me know if I can help with the PR.
@scottaohara
Copy link
Owner

Hey @zackkatz, thanks for the issue.

I like the idea, but I'm going to think on this a bit.

@zackkatz
Copy link
Author

I also exposed modal globally by converting everything to use ARIAmodal.modal—that allows modification externally. But this was the cleanest PR I could make :-)

@scottaohara
Copy link
Owner

Just wanted to check in and let you know I haven't forgotten about this issue, @zackkatz.

Going to try and make sure I give it another look next week. Also want to mention that I've been doing working on refactoring the script in general, and this feature, and a few more, are in the works as part of that refactor too.

@scottaohara scottaohara added this to the 4.0 milestone Jan 2, 2019
@drk4
Copy link

drk4 commented Jan 14, 2020

Hi,
I just wondered if you had been able to look at this issue yet?

@drk4
Copy link

drk4 commented Jan 15, 2020

I've got a working version having made a few changes.

Main changes are

var allyModal = (function ( w, doc, undefined ) {
...
return {
init: function() {
// Set document at point of init
body = document.body;
main = doc.getElementsByTagName('main')[0] || body;
modal = doc.querySelectorAll('[data-modal]');
children = doc.querySelectorAll('body > *:not([data-modal])');
ARIAmodal.organizeDOM();
ARIAmodal.setupTrigger();
ARIAmodal.setupModal();
ARIAmodal.autoLoad();
}
}

})( window, document );

allyModal.init();

This means I can call the allyModal.init() function anywhere and it takes the most uptodate DOM and runs at that point. Needed this so I can run it on html inserted from AJAX.

Let me know if you think there are issues with this approach.

@mxmason
Copy link

mxmason commented Feb 22, 2020

Hey @scottaohara, any updates on this issue? I'd be glad to step in and help!

@scottaohara
Copy link
Owner

been working on a big rewrite of the script which will address this issue. So should hopefully have something pushed within the next month or so.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants