-
Notifications
You must be signed in to change notification settings - Fork 224
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
Closes #5848: Elementor templates clearing cache in full #5956
base: develop
Are you sure you want to change the base?
Closes #5848: Elementor templates clearing cache in full #5956
Conversation
@wp-media/php At the state of this PR we are not clearing cache for related posts when elementor renders templates based on set conditions. Do you see a simple and efficient way of getting related posts for conditional display of template in elementor aside the grooming here? |
@wp-media/php Could you take a look at @jeawhanlee question? |
I don't think there is a simple way to handle those. Maybe these could be handled separately? |
@jeawhanlee Could we also include clearing the Used CSS (if enabled) of the related posts here? |
@piotrbak Yes we can |
@jeawhanlee The used CSS still isn't cleared when clicking the "Clear Used CSS" button. To reproduce:
Also, @wp-media/productrocket, if the Remove Unused CSS feature is not enabled and the display condition changes we won't clear the cache and we won't display a notice similar to the one when Remove Unused CSS is enabled. If for example, we set a popup to be displayed site-wide, that won't be displayed until the cache is cleared manually or by an automatic trigger. How should this be handled? |
Co-authored-by: Rémy Perona <[email protected]>
Co-authored-by: Rémy Perona <[email protected]>
…ring-cache-in-full
We should display a notice to prompt the user to clear the cache since it will cause an inconsistency if it isn't. |
@DahmaniAdame what should be the message for the new notice? |
|
…ates-clearing-cache-in-full' into enhancement/5848-elementor-templates-clearing-cache-in-full
…ring-cache-in-full
@CrochetFeve0251 I can see the message displayed when an Elementor's template is changed while Remove Unused CSS is disabled. Clicking the "Clear Cache" CTA will clear the whole cache even if the specific template was used for a subset of the site's pages, e.g., only posts or the front page. @piotrbak I believe this is not the intended behavior, is it? I'd expect:
By the way, the method you mentioned here doesn't seem to be used anywhere in the code. 🤔 |
Hello @CrochetFeve0251, @vmanthos, @piotrbak, @DahmaniAdame 👋 My understanding was that up until know, we did not display the arning message at the right time. It sounds reasonable to fix this not to wrong the user. I understand this part is OK now? @vmanthos, do you confirm? If there are follow-up improvements for the compatibility with Elementor, we should have dedicated issues with a well defined scope each. But this one is getting out of hand, so let's close it as soon as possible. Thanks |
@MathieuLamiot The scope is pretty straightforward: However, the number of going backs to QA is too high, and I don't think we can afford this now. Do you agree to block the issue and later on break it into two parts when we have time? The notice and auto Used CSS clearing is kind of useful and important here. |
Thanks @piotrbak 🙏 This message is indeed pretty clear but probably lost (at least for me) in all the discussions. We can block it if there is nothing usable quickly in this PR. Otherwise, we might be able to ship it as is as a first step? |
Notifications are displayed correctly @MathieuLamiot. Currently, we'll be clearing the whole cache when the user clicks the CTA button even if that's not necessary. In the future, we can improve this and make it more fine-grained. |
Description
Fixes full cache clearing when updating elementor templates.
Edit: We added the logic to support conditional display.
Fixes #5848
Type of change
Is the solution different from the one proposed during the grooming?
No
How Has This Been Tested?
Checklist: