-
Notifications
You must be signed in to change notification settings - Fork 19
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
Font Awesome library is loaded for every admin page load #86
Comments
Thanks for jumping in here, @bfintal. I'm open to suggestions about how to balance the competing concerns here. Up to now, this "always load on admin pages" behavior has been a feature, rather than a bug, because of expecting that any plugin, theme, or user template may use Font Awesome icons anywhere on the site. If Font Awesome is being loaded as a global icon service, then I think that means it needs to be present by default, instead of absent by default--even on admin pages. In an earlier iteration of this plugin, I had mistakenly not had it loading on admin pages, and an issue was posted about that because expected icons were missing on admin views. One possibility might be some advanced feature that allows for selective loading for users who want the power (and responsibility) of optimizing for that. Thoughts? |
I see, that makes sense if a lot of devs are using is in the admin. I'm assuming that only devs are using it in various placed in the admin for their own uses. Might be a good idea to explore whether this could be made into a filter that a plugin/theme can use to enable the loading on the admin pages where they needed the icons, so that it won't have to load for every other admin page that don't use them. Maybe people can do something like this to enable the loading of the FA library in the admin: add_filter( 'font-awesome/admin_enqueue_fa_library', function( $do_enqueue ) {
$screen = get_current_screen();
if ( $screen->base === 'toplevel_where_needed' ) {
return true;
}
return $do_enqueue;
} ); Devs would have to transition to this method though. |
I do like the idea of using a filter to control this. Because one of the goals of this plugin is to ease compatibility with older themes and plugins, and they can't be assumed to know about this plugin, it still seems that we'd need loading on admin screens to be enabled by default. That is, if no filters were present, it gets loaded. This also avoids breaking changes. But if a developer wanted to try to be more assertive about optimizing--to stop loading except on the specified screen, then your code sample could be modified to do so: add_filter( 'font-awesome/admin_enqueue_fa_library', function( $do_enqueue ) {
$screen = get_current_screen();
if ( $screen->base === 'toplevel_where_needed' ) {
return true;
} else {
return false; // this
}
} ); I may just be catching up to what you're already thinking... However, it occurs to me now that this could create an escalation scenario between clients of this API who compete to be the last filter processed in order not to be disabled by others. If more than one of them uses this filter presumptuous to turn on only itself and turn off everyone else, then the only solution for a dev to play defensively would be to try place this filter hook at a higher priority level to win against the others. I'd hope to avoid that situation. What if the filter's API were adjusted to allow such an optimization while avoiding such a race? The client code would say: "load on my screen and no others, except for others who've also made an explicit request."
We could conceivably also add an advanced settings option to this plugin's own settings page so that the site owner could intentionally select for this optimization, without writing code. If they know that there are no other themes or plugins that need Font Awesome to load on admin screens, they could check a box that would add such a filter to disable loading on admin screens. |
I only have the FA plugin installed in my site, and if you go into any admin page - for example the users page
wp-admin/users.php
, the FA library is loaded. Ideally, this should only be loaded when necessary.Here's what I see in the page source:
The text was updated successfully, but these errors were encountered: