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

Create a New Tab in WP Rocket #6877

Closed
Agathemed opened this issue Aug 20, 2024 · 16 comments · Fixed by #7047
Closed

Create a New Tab in WP Rocket #6877

Agathemed opened this issue Aug 20, 2024 · 16 comments · Fixed by #7047
Assignees
Labels
effort: [M] 3-5 days of estimated development time type: enhancement Improvements that slightly enhance existing functionality and are fast to implement
Milestone

Comments

@Agathemed
Copy link

Description and assets available here

@piotrbak piotrbak added the type: enhancement Improvements that slightly enhance existing functionality and are fast to implement label Aug 20, 2024
@piotrbak piotrbak added this to the 3.17.1 milestone Aug 20, 2024
@JerissCloudCenter
Copy link

JerissCloudCenter commented Sep 26, 2024

Just out of curiosity as a user : what is this about ?

@piotrbak piotrbak removed this from the 3.17.1 milestone Oct 6, 2024
@Khadreal
Copy link
Contributor

Scope a solution

In this file we should add a new method for the new section title and add that newly created method here file

Create a new page in views/settings/page-sections/ and add this page to the Render class

Add the method created in Render class to this section

$this->render_tutorials_section();

Estimation
[M]

@Khadreal Khadreal added the effort: [M] 3-5 days of estimated development time label Oct 10, 2024
@MathieuLamiot
Copy link
Contributor

Chances are we will want to have similar tabs in other plugins. Can we think of a solution that would be easily sharable and maintainable across repos?

@remyperona
Copy link
Contributor

Most of the implementation should be re-usable definitely, as we already have the same thing for Imagify planned: wp-media/imagify-plugin#895

@hanna-meda
Copy link
Contributor

hanna-meda commented Oct 24, 2024

Thank you, @Khadreal, for your work on this one. These are the initial findings:

  1. Clicking on Install from Imagify modal, the page breaks, with error:
    GET https://rocketlabsqa.ovh/wp-admin/admin-post.php?action=install_imagify_from_partner_wp-rocket&_wpnonce=cdb2406366&_wp_http_referer=https%3A%2F%2Frocketlabsqa.ovh%2Fwp-admin%2Foptions-general.php%3Fpage%3Dwprocket 400 (Bad Request)
  2. From documentation, Expected: Clicking the Learn More link will open a new tab with the URL. Currently: Links open in same tab
  3. Activating Termly while BackWPup is still not installed, the Install button from BackWPup will show broken:
    Image
  4. These 2 are logging in the debug.log:
    [24-Oct-2024 13:18:48 UTC] PHP Warning: Undefined array key "cta" in /var/www/rocketlabsqa.ovh/htdocs/wp-content/plugins/wp-rocket/views/settings/page-sections/plugins.php on line 56 [24-Oct-2024 13:18:48 UTC] PHP Warning: Trying to access array offset on null in /var/www/rocketlabsqa.ovh/htdocs/wp-content/plugins/wp-rocket/views/settings/page-sections/plugins.php on line 56

@hanna-meda
Copy link
Contributor

hanna-meda commented Oct 25, 2024

@Khadreal, After checking the latest commits, please find below my testing updates:

  1. Behavior changed >
    I. First time clicking the "Install" from under Imagify, the plugin is installed and activated (but not activated as we need to provide a valid API key)
    II. User activates with a valid API key
    III. BUT on Our Plugins screen, CTA from under Imagify keeps showing "Install".
    IV. Hit "Install" button again => same page break and error
  2. Based on documentation, the default order should be:
    Optimize Performance
    Boost Traffic
    Protect & Secure
    Current default order is:
    Boost Traffic
    Protect & Secure
    Optimize Performance
    Image

@Agathemed
Copy link
Author

@hanna-meda About this:

First time clicking the "Install" from under Imagify, the plugin is installed (but not activated as we need to provide a valid API key)

On WP Rocket Image Optimization tab, when clicking Install Imagify, we install AND activate the Imagify plugin. Then, the user needs to create an account or to add their API key.
I would expect the same behavior here.

@MathieuLamiot
Copy link
Contributor

@Khadreal if the above request from @Agathemed can not be done quickly (typically less than a day), then we'll move this to another Github issue. We must find a way to wrap up this PR quickly to avoid further delaying the release. Let me know how you see things on this. Thanks

@hanna-meda
Copy link
Contributor

hanna-meda commented Oct 28, 2024

@Khadreal, @MathieuLamiot, I re-checked (with the latest commits on plugin-family, thank you @jeawhanlee) and can confirm that all 5 points have been fixed. I still have a few cases to verify, but overall this looks good.

cc @Agathemed

@hanna-meda
Copy link
Contributor

@Khadreal, there might be 2 more tweaks to be fixed. The first point seems actionable, while for the second, we’ll need input from @Agathemed. Could you please take a look at the second point?

  1. For all plugins except BackWPup, clicking the “Install” button installs and activates the plugin while keeping the user on the “Our Plugins” screen. However, with the BackWPup plugin, the user is redirected to the BackWPup dashboard (wp-admin/admin.php?page=backwpup). Expected: The user should still be on the "Our Plugins" tab, with BackWPup showing as activated.

  2. The CTAs from BackWPup & Termly plugins (Protect & Secure section) are not aligned. Should we have them aligned? P.S. They don't appear aligned in the Mockup either.
    Image

@Agathemed
Copy link
Author

Good point @hanna-meda, yes they should be aligned.

@hanna-meda
Copy link
Contributor

Then, @Khadreal, can you take care of both points? Thank you!

@jeawhanlee
Copy link
Contributor

@hanna-meda For the first point, I don't know if there's much that can be done on our side as this is a default behaviour of backwpup, they automatically redirect to a welcome page on activation.
https://github.com/wp-media/backwpup/blob/37d6020719d15e0cabb88d915c54c68886e4296c/inc/class-install.php#L139
So we might need to provide compatibility directly on backwpup for our plugins. Is this something we want to work on? @MathieuLamiot

@Khadreal
Copy link
Contributor

@hanna-meda fixed the button alignment and the first points has been answer by @jeawhanlee

@MathieuLamiot
Copy link
Contributor

thanks @jeawhanlee. I'll follow up with Laurent about this for a later version of BackWPUp then. Thanks for the details about this behavior 🙏

@wpgaurav
Copy link

wpgaurav commented Nov 3, 2024

It's great to see this new tab. Is there a way to remove your recommended plugins from /plugin-install.php?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort: [M] 3-5 days of estimated development time type: enhancement Improvements that slightly enhance existing functionality and are fast to implement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants