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

Integrate plugin suppression into plugins screen #5340

Closed
schlessera opened this issue Sep 5, 2020 · 11 comments · Fixed by #6959
Closed

Integrate plugin suppression into plugins screen #5340

schlessera opened this issue Sep 5, 2020 · 11 comments · Fixed by #6959
Labels
Changelogged Whether the issue/PR has been added to release notes. Compatibility Tool Developer Tools for AMP Debugging Groomed P1 Medium priority WS:Core Work stream for Plugin core
Milestone

Comments

@schlessera
Copy link
Collaborator

Feature description

On the plugins screen in the WordPress admin backend, I'd expect to see information about plugin suppression.

Some form of feedback should be shown in the row of each of the currently suppressed plugins. Clicking on that feedback should probably bring me to the plugin suppression section in the AMP Settings screen, as I'd see all contextual information there.

Also, the top filter row should also include the Suppressed for AMP (x) filter.


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

Implementation brief

QA testing instructions

Demo

Changelog entry

@schlessera schlessera added Compatibility Tool Developer Tools for AMP Debugging WS:Core Work stream for Plugin core labels Sep 5, 2020
@westonruter
Copy link
Member

westonruter commented Sep 5, 2020

This was also referenced in the original issue (#4477):

  • Alternatively, the UI to deactivate plugins in AMP responses could be integrated into the plugin list table.

@westonruter
Copy link
Member

I suppose we'd need to have a new column for “AMP Suppression” as well similar to how there is now a new column for “Automatic Updates”? Should the suppressed state be similarly toggleable as “Enable auto-updates” and “Disable auto-updates”?

image

@westonruter westonruter added this to the v2.2 milestone Feb 11, 2021
@jwold
Copy link
Collaborator

jwold commented Feb 16, 2021

  • AL > Simple integration to plugin screen to give info there, and in the AMP settings. Have filters at top to show installed plugins, inactive, etc. Could have one for suppressed plugins.
  • WR > Useful to get info out of AMP settings screen so users more likely to see it. If you have an advertisement plugin running, suppressed it, and come back later wondering where your ads are, and go to plugin to see, you see it's active, but if it doesn't say it's suppressed you'd be confused.
  • WR > Even saying "this is being suppressed on AMP pages" would be a nice thing to have.
  • AS > Info should be easily available from the database, so it's probably easy to integrate.
  • JW > New column or put status in description.
  • AS > Could also examine; I already did something like this for fatal error recovery mode in WordPress. So basically when a plugin throws a fatal error, then you can login to admin dashboard, and if you go to plugin screen it shows what plugins were paused in their execution to still make admin dashboard. Fatal error recovery mode forces execution of plugins, they remain active but not loaded on page load that is protected. Plugin screen shows that with a state and has filtered list at top of all plugins that are paused.
  • AS > Create fatal error in plugin active on site by renaming class name, add a few letters into class name in PHP file, if you do page reload you'd get white screen of death from WP, and then you can still login to admin backend, and it will have that plugin be paused.

@jwold
Copy link
Collaborator

jwold commented Feb 16, 2021

@jwold to pull a screenshot of fatal error from plugin to propose how AMP would handle this.

@kmyram kmyram added P1 Medium priority Groomed labels Feb 16, 2021
@jwold
Copy link
Collaborator

jwold commented Feb 16, 2021

Related from Alain: https://core.trac.wordpress.org/ticket/44458

@westonruter westonruter removed this from the v2.2 milestone Jul 3, 2021
@westonruter westonruter added this to the v2.2.2 milestone Feb 9, 2022
@dhaval-parekh dhaval-parekh self-assigned this Feb 24, 2022
@westonruter
Copy link
Member

@dhaval-parekh Instead of adding an entire new column, since we're not adding the ability to suppress the plugins from this screen I think it would be suitable just to add a “Suppressed on AMP Pages” link to the plugin meta links when a plugin is suppressed.

@pooja-muchandikar
Copy link

QA Passed ✅ for the feature implemented.

Plugin suppression notice is shown on plugins screen

image

--

Found one issue with the redirection link

If Suppressed on AMP Pages link is clicked from plugin screen, it should redirect to AMP settings > Plugin Suppression section. But currently it is redirecting to AMP settings > Review Section

Issue is specifically in Chrome Browser latest version
In Safari Browser latest version it is working fine.

Screen.Recording.2022-03-23.at.4.21.53.PM.mov

@maitreyie-chavan
Copy link
Collaborator

@dhaval-parekh please could you take a look at the issue mentioned in the QA notes?

@westonruter
Copy link
Member

If Suppressed on AMP Pages link is clicked from plugin screen, it should redirect to AMP settings > Plugin Suppression section. But currently it is redirecting to AMP settings > Review Section

I think this is a separate issue with the Settings screen. Namely, I think the Review section is loaded asynchronously after the scrolling has started so that once the scrolling ends up the Review section and not Plugin Suppression. So that would be a separate bug that can be addressed in another issue.

@westonruter
Copy link
Member

I can't reproduce the issue myself in Chrome.

@westonruter
Copy link
Member

I've filed that as #7005, and I'm moving this to QA Passed.

@westonruter westonruter added the Changelogged Whether the issue/PR has been added to release notes. label Apr 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelogged Whether the issue/PR has been added to release notes. Compatibility Tool Developer Tools for AMP Debugging Groomed P1 Medium priority WS:Core Work stream for Plugin core
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants