wp-media / wp-rocket

Performance optimization plugin for WordPress
https://wp-rocket.me
GNU General Public License v2.0
688 stars 215 forks source link

Defer JS compatibility with Elementor sticky state #5387

Open DahmaniAdame opened 2 years ago

DahmaniAdame commented 2 years ago

Before submitting an issue please check that you’ve completed the following steps:

Describe the bug When Elementor sticky element is enabled with Defer JS, it might cause the following console error:

Uncaught TypeError: this.$element.sticky is not a function
    at child.activate (frontend.min.js?ver=3.3.1:2:26467)
    at e.each.r.<computed> [as activate] (frontend-modules.min.js?ver=3.6.8:2:9686)
    at child.run (frontend.min.js?ver=3.3.1:2:26794)
    at e.each.r.<computed> [as run] (frontend-modules.min.js?ver=3.6.8:2:9686)
    at child.onInit (frontend.min.js?ver=3.3.1:2:27215)
    at e.each.r.<computed> [as onInit] (frontend-modules.min.js?ver=3.6.8:2:9686)
    at Module.trigger (frontend-modules.min.js?ver=3.6.8:2:11045)
    at e.each.r.<computed> [as trigger] (frontend-modules.min.js?ver=3.6.8:2:9686)
    at init (frontend-modules.min.js?ver=3.6.8:2:9875)
    at child.Module (frontend-modules.min.js?ver=3.6.8:2:11125)

In addition to the sticky state not working, it will also break other frontend functions, like the submenu for example.

https://drive.google.com/file/d/1mdPwuvV8Wa0dxykVJvK_fj18tVCQHI3Y/view?usp=drivesdk

Excluding the following fixes the issue:

/jquery-?[0-9.](.*)(.min|.slim|.slim.min)?.js
/elementor-pro/assets/lib/sticky/jquery.sticky.min.js

Possibly the root cause of - https://github.com/wp-media/wp-rocket/issues/4648

In such a case, the same exclusions need to be added to Delay JavaScript Execution as well.

I will leave it to the QA to dig in and confirm the random aspect + the fix.

To Reproduce Steps to reproduce the behavior:

  1. Enable a sticky element on Elementor
  2. Enable defer JS without any exclusions
  3. See error

Expected behavior There shouldn't be any errors when the sticky state is used with Defer JS.

Screenshots If applicable, add screenshots to help explain your problem.

Additional context Ticket - https://secure.helpscout.net/conversation/1966571981/359434/

Backlog Grooming (for WP Media dev team use only)

camilamadronero-zz commented 1 year ago

related https://secure.helpscout.net/conversation/2001170340/366775/

NataliaDrause commented 1 year ago

Related: https://secure.helpscout.net/conversation/2002820685/367101/

NataliaDrause commented 1 year ago

Exclusions are heavily affecting the performance.

Alternatively, it is possible to disable "sticky" menu in Elementor: image Full-screen image: https://i.imgur.com/Oow9nr0.png

This way, only jQuery script would need to be excluded from the Load JavaScript Deferred option. No exclusions would be needed for the Delay JavaScript Execution, so performance is not affected as much.

UPDATE: after further testing, the Delay JS exclusions were still needed, so only disabling sticky menu did not help much in improving performance for the above-mentioned case.

NataliaDrause commented 1 year ago

Related: https://secure.helpscout.net/conversation/2014721817/369753?folderId=3864740

CarlPotak commented 1 year ago

It took me 3 weeks to figure out the fix for this while still keeping my sticky to the top button on each page. The submenus werent working, the submenu dropdown arrow wasnt showing, and the mobile menu toggle icon was not showing up correctly and instead of three stacked horizontal lines, it was just showing an empty box as a placeholder.

Even though no errors were showing in the browser developers console, I was able to identify the elements themselves and add them to the CSS Safelist under the File Optimization tab/page. Now my menu works flawlessly on mobile and desktop. The following are what ended up on my CSS Safelist:

/wp-content/themes/hello-theme-child-master/style.css elementor-nav-menu-main elementor-nav-menucontainer elementor-nav-menu--layout-horizontal elementor-menu-toggleicon--open.eicon-menu-bar elementor-menu-toggle elementor-nav-menu--dropdown

I also had to exclude many JavaScript files from delay execution. These were the URLS and keywords: /jquery-?0-9.(.min|.slim|.slim.min)?.js /jquery-migrate(.min)?.js /header-footer-elementor/inc/js/frontend.js js-(before|after) (?:/wp-content/|/wp-includes/)(.) /elementor/ /elementor-pro/ /wp-includes/js/imagesloaded.min.js /wp-includes/js/underscore.min.js /wp-includes/js/jquery/ui/core.min.js /wp-includes/js/backbone.min.js /elementor-pro/assets/lib/smartmenus/jquery.smartmenus.min.js /elementor-pro/assets/js/preloaded-elements-handlers.min.js /essential-addons(-for)?-elementor(-lite)?/(.)(.min)?.js elementorAdminBarConfig elementorCommonConfig elementorWebCliConfig elementorDevToolsConfig elementorProFrontendConfig elementorFrontendConfig /ele-custom-skin(.*)/assets/js/

Hope this helps anyone else experiencing this issue.

Pirenko commented 1 year ago

Hi, In my case it was enough to add these exclusions https://nimb.ws/1YocO2 Text below if you want to copy/paste:

First textarea: /wp-content/plugins/elementor-pro/assets/lib/sticky/jquery.sticky.min.js?ver=3.9.2

Second textarea: /wp-content/plugins/elementor-pro/assets/lib/sticky/jquery.sticky.min.js?ver=3.9.2 /jquery-?0-9.(.min|.slim|.slim.min)?.js js-(before|after) (?:/wp-content/|/wp-includes/)(.*)

Kind regards, Pirenko

DahmaniAdame commented 1 year ago

@CarlPotak @Pirenko, both of you are excluding the compatibility pattern:

js-(before|after)
(?:/wp-content/|/wp-includes/)(.*)

They include Elementor's JS files that would cause the issue.

I suggest you test without the above pattern, as using them will only delay external scripts :)

If you experience any errors after removing them, refer to https://docs.wp-rocket.me/article/1560-delay-javascript-execution-compatibility-exclusions to see if you have are using any of the theme/plugins on the list and apply their respective exclusions.

If you can't find your theme/plugins or the errors persist after apply the recommended exclusions, open a support ticket so we can have a closer look :)

Pirenko commented 1 year ago

Hi @DahmaniAdame, Thanks for your reply! If I remove that pattern I get a scripting error on the console. Most likely because jQuery is not loaded yet. But you are right... So I went ahead and adjusted the second textarea setting... Here's what I have now: /wp-content/plugins/elementor-pro/assets/lib/sticky/jquery.sticky.min.js?ver=3.9.2 /jquery-?0-9.(.min|.slim|.slim.min)?.js

It still works just fine like this! Thanks, Pirenko

vukojevicc commented 8 months ago

Thank you @NataliaDrause