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

Assets loaded on all pages #135

Closed
paulschreiber opened this issue Jul 27, 2018 · 15 comments
Closed

Assets loaded on all pages #135

paulschreiber opened this issue Jul 27, 2018 · 15 comments

Comments

@paulschreiber
Copy link
Contributor

The assets (inline CSS, CSS, JS) are loaded on every page.

On the front end, should only be loaded on pages where the shortcodes ([pgn], [fen], etc.) are present.

On the backend, they should only be loaded on post-new.php and edit.php.

@yo35
Copy link
Owner

yo35 commented Aug 5, 2018

I agree on the fact that the plugin assets should be loaded only if required. Though, I have two questions/remarks regarding how this should be achieved by a WordPress plugin:

  1. The function to use to insert a JavaScript asset is wp_enqueue_script (documented here). Documentation says that this method "should be called using the wp_enqueue_scripts action hook [or] the admin_enqueue_scripts action hook. [...] Calling it outside of an action hook can lead to problems". Following this recommendation leads to have the assets loaded on every page. So, would it be possible to have a WP developer clarify the status of this wp_enqueue_script function, and in particular whether it is safe or not to call it from anywhere? (in particular from a shortcode-rendering callback)

  2. I'm concerned with themes that dynamically load posts through AJAX requests (see theme Caos for example). If the content that is dynamically loaded requires some specific assets, how it is possible to be sure that these assets will be properly loaded in the destination page?

@paulschreiber
Copy link
Contributor Author

The MathJax-LaTex plugin has solved the problem this way:

  • Add a class constant (public static $add_script;)
  • In the initialization function, set it to true if force-loading is enabled.
  • Set it to true whenever the shortcode is present.
  • In the function that enqueues the scripts, return early if $add_script is false.

@paulschreiber
Copy link
Contributor Author

Here's a rough attempt at this:
784f65f

This hasn't been run/tested.

@yo35
Copy link
Owner

yo35 commented Aug 15, 2018

It seems that the constraint to have wp_enqueue_script only called from the wp_enqueue_scripts action hook has been repealed. See ref: https://developer.wordpress.org/reference/functions/wp_enqueue_script/#comment-1602
This is good news.

Though, I'm still concerned with AJAX-based themes.

@paulschreiber
Copy link
Contributor Author

@yo35 Was my work in 784f65f helpful?

@yo35
Copy link
Owner

yo35 commented Sep 24, 2018

@paulschreiber Sorry for the long delay in replying.

It seems that it is now possible to have a more straightforward approach, as describe in: http://scribu.net/wordpress/conditional-script-loading-revisited.html
(see also the comment that mention this link in https://developer.wordpress.org/reference/functions/wp_enqueue_script)

I'll refactor the code of the plugin to follow this pattern, but I've no time for the moment.

@paulschreiber
Copy link
Contributor Author

That blog post is from 2013. I think the strategy I employ above is simpler.

@paulschreiber
Copy link
Contributor Author

@yo35 Can you take a look at this?

@yo35
Copy link
Owner

yo35 commented Feb 9, 2019

Hi. This is done for the front end in version 5.5 (just released).

Additional refactoring must be done beforehand for the back end.

@paulschreiber
Copy link
Contributor Author

Thanks!

@paulschreiber
Copy link
Contributor Author

I see this was reverted. Please try the method I suggested.

@yo35
Copy link
Owner

yo35 commented Mar 3, 2019

It has been reverted for CSS, because it seems to be incompatible with some themes (although it appears to work with most of them) : see #145.
The feature is still there for JS scripts.

The solution you proposed in 784f65f does not work (whatever the theme), because the callbackShortcodeXXX(..) functions are called after RPBChessboardScripts::register() and RPBChessboardStylesheets::register() during page rendering.

@paulschreiber
Copy link
Contributor Author

Can you work on a fix for this you like? It's adding uncompressed CSS to all sorts of pages, even when the chess content isn't present.

This check catches some of the cases:

<?php
function rpbchessboard_init_js_css() {
	if ( ! is_single() ) {
		return;
	}

@yo35
Copy link
Owner

yo35 commented Jun 20, 2020

@paulschreiber Your approach consisting in not inserting CSS/JS if !is_single() will not work. The CSS/JS inserted by the plugin is mandatory as soon as the page contains any content with either shortcode [pgn] or [fen]. Obviously rejecting pages based on is_single() would result in potentially ill-rendered content.

@yo35
Copy link
Owner

yo35 commented Jun 20, 2020

As of version 5.7 (just released), I've re-introduced lazy-loading of CSS assets on frontend, although it is still not clear whether this approach is the right approach.

An option is provided in the "settings" page to fallback to the "always-enqueue-CSS-and-JS strategy" if any problem is encountered with. And there WILL be issues with some themes such as the one used in #145 , or plugins such as bbPress (see #65).

I'm quite surprised I cannot find any proper directives of the WP architects on how to solve this issue. The use-case "provide a custom shortcode that needs specific CSS/JS to be rendered property, and ensure those CSS/JS are enqueued only on the pages with this custom shortcode" seems quite common among the WP plugins though...

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