WordPress / wp-autoupdates

Feature plugin building a UI for opting-in to plugin, theme, and core auto-updates.
https://wordpress.org/plugins/wp-autoupdates/
GNU General Public License v2.0
56 stars 18 forks source link

Add Ajax to Plugin and Themes Screen #61

Closed ronalfy closed 4 years ago

ronalfy commented 4 years ago

Resolves #65

This is a concept PR that demonstrates Ajax enabling/disabling of plugins. Would love to discuss this more and flesh it out more for themes as well if this concept is accepted.

autoupdates

Thanks,

Ronald Huereca

paaljoachim commented 4 years ago

Looks Good Ron..:)

ronalfy commented 4 years ago

Looks Good Ron..:)

Thanks! I'll try to attend the next meeting and bring this up.

pbiron commented 4 years ago

Related: https://wordpress.slack.com/archives/CULBN711P/p1585694448060400, #47

ronalfy commented 4 years ago

Thanks @pbiron

paaljoachim commented 4 years ago

So it sounds like it is outside the scope of this plugin. Perhaps this is more of a WordPress trac issue to bring Ajax into the Installed plugins screen so one can activate/deactivate plugins by using Ajax.

I am guessing that Ajax is being used when initially installing and activating a plugin. (Atleast it does not jump to the top of the screen when installing/activating a plugin for the first time.)

That means on the Plugins -> Install New seems to have Ajax activated. But that the Installed plugins screen does not.

pbiron commented 4 years ago

Thanks @pbiron

Thank you for the POC PR!

pbiron commented 4 years ago

So it sounds like it is outside the scope of this plugin. Perhaps this is more of a WordPress trac issue to bring Ajax into the Installed plugins screen so one can activate/deactivate plugins by using Ajax.

No, it's perfectly within scope for the plugin. The implementation may change when merged into core, but that's the same with a lot of what's in the plugin.

ronalfy commented 4 years ago

@paaljoachim @pbiron updated the PR for the themes screen on multisite. I'll do another test on single-site to make sure everything behaves as expected.

One thing I noticed is we need some do_action or apply_filters when anything is updated and removed for third-party plugin compatibility such as Easy Updates Manager (200,000+ installs) so they can keep track of what's enabled/disabled. I'm imagining another PR to accomplish this.

theme-screen

ronalfy commented 4 years ago

Also would be good to abstract out the HTML creation so Ajax and non-Ajax use the same markup.

ronalfy commented 4 years ago

This does not work for the themes screen on single-site. I'll be happy to do another PR to get that fixed with this PR as a dependency.

Needed:

  1. Abstraction of HTML for use in Ajax and non-Ajax settings.
  2. Uniform set of classes to target for JS integration.
  3. Uniform set of strings to i18n for enabled/disabled settings. I notice the themes screen on single-site has different verbiage for enabled/disabled states.

I'll be happy to discuss at the next open meeting.

Regards,

Ronald Huereca

pbiron commented 4 years ago

One thing I noticed is we need some do_action or apply_filters when anything is updated and removed for third-party plugin compatibility such as Easy Updates Manager (200,000+ installs) so they can keep track of what's enabled/disabled. I'm imagining another PR to accomplish this.

Thanx Ronald! Can you open an issue about other things we could do (e.g., other hooks we could/should add) to help and/or avoid conflicts with existing update plugins?

ronalfy commented 4 years ago

Thanx Ronald! Can you open an issue about other things we could do (e.g., other hooks we could/should add) to help and/or avoid conflicts with existing update plugins?

Done. Thank you. https://github.com/WordPress/wp-autoupdates/issues/63

ronalfy commented 4 years ago

@paaljoachim @pbiron I updated the PR to cover single-site themes.

This is ready for review/scrutiny :)

theme-single-site

pedro-mendonca commented 4 years ago

Suggestion for some more consistent wording:

imagem imagem

ronalfy commented 4 years ago

Suggestion for some more consistent wording:

imagem imagem

Yeah, we'll definitely want to tweak the wording. Thanks!

ronalfy commented 4 years ago

@pbiron shall I keep this PR up to date with changes, or wait until the 0.6 milestone to finalize?

pbiron commented 4 years ago

@pbiron shall I keep this PR up to date with changes, or wait until the 0.6 milestone to finalize?

It's probably easier to wait, but that's up to you.

Also, with regard to adding $hook === 'site-themes.php' to the conditional, see #69 which I just opened...so definitely wait to make that change.

ronalfy commented 4 years ago

@pbiron is core verbiage solid enough to begin updating this PR? Thanks!

pbiron commented 4 years ago

Thanx for the reminder Ronald, I was meaning to ping you.

Yes, feel free to update the PR against the current state of master (which is 0.5.1).

In particular, note that:

  1. the requested change to add $hook === 'site-themes.php' to the conditional is no longer necessary, because auto-updates are no longer shown on the screen
  2. the dashicon is no longer displayed (anywhere) so the animation of that isn't necessary any more
ronalfy commented 4 years ago

@pbiron this has been updated with the latest 0.5.1 changes.

audrasjb commented 4 years ago

Thanks all and particularly @ronalfy for all the great work on this pull request. Currently testing it, and all looks fine on my side for the moment… I have some accessibility concerns, but I think it’d be easier to handle them once the current workaround is merged.

ronalfy commented 4 years ago

@TimothyBJacobs @audrasjb @pbiron ready for re-review. Made the CSS changes and removes the filter_inputs. Thanks for allowing me to be apart of this.

whyisjake commented 4 years ago

💥