-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add customOpener configuration option and handle clicks #4
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution here. Please see my comments below:
Based on the options property name, which suggests “customOpener”, I would recommend passing a single DOM element to the opener argument. This approach would eliminate the need to loop through multiple opener elements, as it is uncommon to have more than one opener for a single dropdown menu.
Additionally, please see the following non-blocker issues as well:
- It seems you didn’t build and compile the final package for distributed files. See the repository readme file in creating dist files.
- No demo is added to the demo website.
- No documentation is added to the “Settings” table on the demo website.
Thank you @mahdiyazdani for the review. I applied all your suggested changes, including the update od the demo. Please review again |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @kkarpieszuk 🙌
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @kkarpieszuk 🙌
Fixes #3
Test steps:
<button id="foo">
customOpener: '#foo'
You can see this in real use on this branch of WPForms:
https://github.com/awesomemotive/wpforms-plugin/pull/8341/commits/12d59d1b75ea3717ae43784bfd913f9df7282016