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

Add panel to display registered assets #879

Open
wants to merge 19 commits into
base: develop
Choose a base branch
from

Conversation

crstauf
Copy link
Contributor

@crstauf crstauf commented Jul 27, 2024

No description provided.

@johnbillion
Copy link
Owner

Neat, I like it. Thanks. I think I would be happy to have this panel available without the QM_SHOW_ALL_ASSETS constant being required.

@crstauf
Copy link
Contributor Author

crstauf commented Jul 27, 2024

Great! Will make that change.

@crstauf
Copy link
Contributor Author

crstauf commented Jul 29, 2024

@johnbillion Are you opposed to adding a sort to the Handle column? I'd prefer that to forcing it to be sorted, as there might be a situation where seeing the order that assets are registered is helpful.

@crstauf
Copy link
Contributor Author

crstauf commented Jul 29, 2024

Hmmm, forgot sorting was limited to numbers. 🤔

@johnbillion
Copy link
Owner

It should be possible to sort the handle column yeah

@crstauf
Copy link
Contributor Author

crstauf commented Jul 29, 2024

@johnbillion I can't add you as a reviewer unfortunately, but please take a peek here: crstauf#1.

@johnbillion
Copy link
Owner

I would like to get this in, but it's triggering a PHP warning on the front end.

Trying to access array offset on value of type null	
wp-includes/script-loader.php:2068
wp_style_loader_src()
apply_filters('style_loader_src')
do_action('shutdown')
shutdown_action_hook()

The error is inside wp_style_loader_src() which attempts to read the url property on an admin colour scheme that doesn't exist, presumably because admin scheme colours aren't registered on the front end.

The same problem might occur for other styles or scripts that are only intended for the admin area or only for the front end.

Thoughts?

@crstauf
Copy link
Contributor Author

crstauf commented Nov 27, 2024

@johnbillion This seems to only occur because $_wp_admin_css_colors isnt' set. Adding the below solves the error:

global $_wp_admin_css_colors;

if ( ! isset( $_wp_admin_css_colors ) || is_null( $_wp_admin_css_colors ) ) {
	$_wp_admin_css_colors = [ 'fresh' => (object) [ 'url' => '' ] ];
}

Is that an acceptable solution from your perspective?

@johnbillion
Copy link
Owner

My worry is whether this will result in errors for other third-party scripts and styles that are enqueued in the admin area and don't expect to be processed in the front end, and vice versa. Probably uncommon, but it's hard to tell.

@crstauf
Copy link
Contributor Author

crstauf commented Nov 27, 2024

@johnbillion If the asset is globally registered, but only enqueued on one side, all of the necessary data is still available. If an asset is registered only on one side, then it won't be listed.

This error only occurs because there's a dynamic component to colors that's handled in wp_style_loader_src(), which no themes or plugins would be able to do (without editing core).

Really I think the way WordPress handles this should be changed, but that's a separate conversation.

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

Successfully merging this pull request may close these issues.

2 participants