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

Recommended: Front end JavaScript changes #56

Open
justintadlock opened this issue Jan 31, 2018 · 2 comments
Open

Recommended: Front end JavaScript changes #56

justintadlock opened this issue Jan 31, 2018 · 2 comments

Comments

@justintadlock
Copy link

Is jQuery necessary?

In your memberlite.js and navigation.js files, which are loaded on the front end, you have just a few dozen lines of code. Both of these scripts rely on jQuery. Is it really necessary in this case? It looks like this can all be rewritten in vanilla JS and CSS with minimal effort. Then, there'd be no need to load an additional 95kb file just to run your little bit of JS. It could potentially shave off some loading time for some of your users. Definitely something to think about.

You're also erroneously setting jquery as a dependency for skip-link-focus-fix.js in functions.php on line 23 (it doesn't rely on it):

wp_enqueue_script('memberlite-skip-link-focus-fix', get_template_directory_uri() . '/js/skip-link-focus-fix.js', array( 'jquery' ), MEMBERLITE_VERSION, true);

For the admin and customizer JS, this is a non-issue since jQuery is definitely going to be loaded there anyway.

Combining files:

I'd also consider combining memberlite.js, navigations.js, and skip-link-focus-fix.js for the production release.

@ideadude
Copy link
Member

ideadude commented Mar 7, 2018

I merged the 3 frontend JS into one file.

For now, we will still require jQuery here to save the time of refactoring, but updating status on this issue to work on that for a future release.

@kimcoleman
Copy link
Member

PR here offers a jQuery free solution: https://github.com/strangerstudios/memberlite/pull/117/files

This needs testing but will address the issue.

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

No branches or pull requests

3 participants