vadikom / smartmenus

Advanced jQuery website menu plugin. Mobile first, responsive and accessible list-based website menus that work on all devices.
http://www.smartmenus.org
MIT License
594 stars 164 forks source link

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

Open Htbaa opened 9 months ago

Htbaa commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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.