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

Extra Sidebar Widgets: CSS no longer enqueued on Elementor pages including a widget #39813

Open
jeherve opened this issue Oct 17, 2024 · 3 comments · May be fixed by #39820
Open

Extra Sidebar Widgets: CSS no longer enqueued on Elementor pages including a widget #39813

jeherve opened this issue Oct 17, 2024 · 3 comments · May be fixed by #39820
Assignees
Labels
[Feature] Extra Sidebar Widgets [Focus] Compatibility Ensuring our products play well with third-parties [Platform] Atomic [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ [Pri] Normal [Status] In Progress Triaged [Type] Bug When a feature is broken and / or not performing as intended

Comments

@jeherve
Copy link
Member

jeherve commented Oct 17, 2024

Impacted plugin

Jetpack

Quick summary

This is a long-standing issue with the Elementor plugin. I reported it here: elementor/elementor#29010

However, until #39518 this was a bug that was not noticeable since we enqueued all the CSS on all pages with the concatenated jetpack.css file. Now that we rely on each individual CSS file being enqueued, we're starting to notice the issue.


Plugins can register WordPress widgets using the Widgets API (register_widget and extending the WP_Widget class). When they do that, they can enqueue scripts and styles for their widget, by hooking wp_enqueue_scripts. A good practice when enqueuing custom scripts and styles is to only enqueue them when the widget is present on the page. That can be checked with the is_active_widget feature. Unfortunately, is_active_widget does not return true on pages where a widget has been added to the page via the Elementor editor.

Steps to reproduce

  1. Start by installing the latest version of the Elementor plugin.
  2. Add Jetpack, connect it to WordPress.com
  3. Go to Jetpack > Settings > Writing and enable the Extra Sidebar Widgets
  4. Go to Pages > Add New
  5. Open the Elementor Editor
  6. Add a new container.
  7. In that container, add the Top Posts widget (scroll down to the "WordPress" part of the inserter, you should see the widget there)
  8. Publish the page.
  9. View it on the frontend.

A clear and concise description of what you expected to happen.

I would expect the top-posts/style.css stylesheet to be enqueued on the page.

What actually happened

It is not.

Impact

Some (< 50%)

Available workarounds?

Yes, easy to implement

If the above answer is "Yes...", outline the workaround.

As a work-around, one can manually copy and paste the widget's CSS from this file into WordPress CSS editor.

Platform (Simple and/or Atomic)

Atomic, Self-hosted

Logs or notes

This was reported here:
https://wordpress.org/support/topic/grid-layout-doesnt-work-in-the-top-posts-and-pages-widget/

@jeherve jeherve added [Feature] Extra Sidebar Widgets [Focus] Compatibility Ensuring our products play well with third-parties [Pri] Normal [Type] Bug When a feature is broken and / or not performing as intended Triaged labels Oct 17, 2024
@github-actions github-actions bot added [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ [Platform] Atomic labels Oct 17, 2024
@jeherve
Copy link
Member Author

jeherve commented Oct 17, 2024

cc @kraftbj ; I'm not sure what our best work-around should be at this point. I'd be happy to have your opinion. Maybe we should remove the is_active_widget checks for now?

Looking at our list, this is going to impact quite a few widgets:

jetpack_display_posts_widget
gravatar-profile-widget
goodreads-widget
jetpack_social_media_icons_widget
jetpack-top-posts-widget
jetpack_image_widget
jetpack-my-community-widget
jetpack-authors-widget
eu-cookie-law-style
flickr-widget-style
jetpack-search-widget
jetpack-simple-payments-widget-style
jetpack-widget-social-icons-styles
wpcom_instagram_widget
milestone-widget

@kraftbj
Copy link
Contributor

kraftbj commented Oct 17, 2024

Hmm, so these checks in the impacted widgets with the result of each of these style files enqueing for any site with widgets enabled whether or not they're being used?

if ( is_active_widget( false, false, $this->id_base ) || is_customize_preview() ) {

I'm not a fan of that, but can appreciate it may be the quickest fix to restore the styling for impacted sites.

What about having a concatenated widgets.css of all of the widget CSS so at least it's only one file since they all would be enqueued anyhow with our fix?

I'd be curious if there was an alternative to is_active_widget that we could add in for those sites until there's an upstream fix.

jeherve added a commit that referenced this issue Oct 18, 2024
Fixes #39813

Instead of relying on a conditional check to see if the widget is active, let's call the widgets' enqueuing methods directly from within the widget display method. That should simplify things, and ensure compatiblity with systems that are not compatible with `is_active_widget`.
@jeherve
Copy link
Member Author

jeherve commented Oct 18, 2024

I'm thinking we may take a different approach. I opened #39820 for it. This is an approach we already follow for some of the other widgets, so I don't see why we couldn't do this for all widgets.

I haven't tested all the widgets I've modified yet, but if you want to give the PR a try, I'd appreciate it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Extra Sidebar Widgets [Focus] Compatibility Ensuring our products play well with third-parties [Platform] Atomic [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ [Pri] Normal [Status] In Progress Triaged [Type] Bug When a feature is broken and / or not performing as intended
Projects
Development

Successfully merging a pull request may close this issue.

2 participants