wp-media / wp-rocket

Performance optimization plugin for WordPress
https://wp-rocket.me
GNU General Public License v2.0
690 stars 216 forks source link

Divi compatibility: Notice to clear used CSS is missing after re-enabling RUCSS feature even when needed #6095

Closed MathieuLamiot closed 1 year ago

MathieuLamiot commented 1 year ago

Before submitting an issue please check that you’ve completed the following steps:

Describe the bug This issue has been spotted by QA while testing #6074 . Notices for Divi users to clear used CSS is not displayed when the Remove Unused CSS option is switched from deactivated to activated, even though it could be needed.

To Reproduce There are a couple of scenarios that are failing.

With Remove Unused CSS enabled and used CSS existing in the database:

  1. Disable Remove Unused CSS.
  2. Edit the global header in Divi's Theme Builder for any existing template.
  3. Enable Remove Unused CSS -> the notice is displayed. ✔️
  4. Disable Remove Unused CSS -> the notice isn't displayed. ✔️
  5. Enable Remove Unused CSS -> the notice is NOT displayed. ❌

For the second scenario, with Remove Unused CSS enabled and used CSS existing in the database:

  1. Disable Remove Unused CSS.
  2. Delete a template using the Divi Theme Builder -> the notice is not displayed. ✔️
  3. Enable Remove Unused CSS -> the notice is NOT displayed. ❌

Expected behavior When activating the Remove Unused CSS options, if a Divi template has been updated since the last update of used CSS, then the notice must be displayed.

Additional context This is a continuation of #6074

Acceptance Criteria

mostafa-hisham commented 1 year ago

I couldn't reproduce the problems in both scenarios I provided recordings of the test

https://github.com/wp-media/wp-rocket/assets/29744252/145d3d57-4a80-49b8-bd2f-e4bafa41e134

https://github.com/wp-media/wp-rocket/assets/29744252/2cd88b0f-2ddb-435a-bcb9-4fccbc8b2024

FYI: The recording and testing were done on fix/truncate-rucss-on-wp-rocket-options-changes branch of this PR because the fix is required for this feature to work. and it is not merged at this time

engahmeds3ed commented 1 year ago

@vmanthos @MathieuLamiot @piotrbak what do u think about that?

vmanthos commented 1 year ago

@engahmeds3ed Both me and @Mai-Saad tested this using 3.15 and the issues aren't there anymore. :tada: I'll close the issue.