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

SmartMenus should log errors to console, not use alert() #242

Open
Htbaa opened this issue Jan 31, 2024 · 2 comments
Open

SmartMenus should log errors to console, not use alert() #242

Htbaa opened this issue Jan 31, 2024 · 2 comments

Comments

@Htbaa
Copy link

Htbaa commented Jan 31, 2024

There are two alert() calls in the source code of SmartMenus:

The call to alert() stops rendering the page and depending on the website can appear multiple times. Instead of showing this error it should log to console with console.error(). That way visitors of a website aren't hindered by misconfiguration.

Edit: I have an existing website built with WordPress and Elementor Pro which seemingly uses this plugin (1.2.1 now, but before 1.1.x). There are no elements with the data-sm-options attribute yet the alert pops-up. It didn't do that with the 1.1.x version of SmartMenus.

@Htbaa
Copy link
Author

Htbaa commented Jan 31, 2024

Ah, my issue was that I wasn't setting the data-sm-options-attribute properly. I guess since version 1.2.x it has become more critical of correctness? Still, would be a better user experience to log to console.

Works
$('.elementor-nav-menu--main > ul').attr('data-sm-options', '{ "showTimeout": "0", "hideTimeout": "0" }');

Doesn't work
$('.elementor-nav-menu--main > ul').attr('data-sm-options', '{ showTimeout: 0, hideTimeout: 0 }');

@garhbod
Copy link

garhbod commented Feb 4, 2024

Thanks for the fix. The release is 2 years old so I'm guessing this only came up recently because something has changed in browsers definition of a valid JSON not from this package. But yes it would be nice if it was console.error() rather than alert()
You should try forking the repo fixing it and making a pull request.

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

No branches or pull requests

2 participants