Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow to pass Element itself to targetModal, not only 'id' of element #351

Merged
merged 2 commits into from
Mar 21, 2025

Conversation

pbek
Copy link
Contributor

@pbek pbek commented Jul 7, 2020

Enables usage of library in web-components.
See #65 (comment)

image

@gstvribs
Copy link

You need to do this same thing this.modal = typeof targetModal === "string" ? document.getElementById(targetModal) : targetModal at the closeModal function

@code3z
Copy link

code3z commented Feb 10, 2022

please merge this!

@dkniffin
Copy link
Contributor

@pbek I just took over maintenance of micromodal, and it looks like this is a popular request. I saw your comment where you're using this in production. Can you confirm whether @gstvribs's comment is true and we also need to update closeModal? It doesn't seem so, since that references this.modal

@pbek
Copy link
Contributor Author

pbek commented Mar 21, 2025

@dkniffin, it doesn't look that we ever needed to add that. We are running with https://github.com/digital-blueprint/toolkit/blob/main/packages/common/src/micromodal.es.js for 5 years now...

We just added / fixed some other things over the years, see https://github.com/digital-blueprint/toolkit/commits/main/packages/common/src/micromodal.es.js.

@pbek
Copy link
Contributor Author

pbek commented Mar 21, 2025

And thank you for taking over the project!

@dkniffin dkniffin merged commit 9663a51 into micromodal:master Mar 21, 2025
@dkniffin
Copy link
Contributor

Merged and pushed as v0.6.0

@dkniffin
Copy link
Contributor

I confirmed the code did need to be updated for show and hide. I updated show before the release, but I missed hide. I'm working on that now. It'll be in 0.6.1 soon.

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

Successfully merging this pull request may close these issues.

None yet

4 participants