search

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

Simplifies Ajax on both the JS and PHP sides. #103

Closed pbiron closed 4 years ago

pbiron commented 4 years ago

Fixes #102, #94

This simplifies Ajax:

  1. on the PHP side, combines both enable/disable handlers into a single toggle handler
  2. on the JS side, reduces it down to a single click handler
    • also removes all HTML injection. that is, the markup for all "states" is output from PHP when the screen is originally rendered and the JS basically just shows/hides the relevant parts depending on whether it is enabling or disabling

It also addresses the "unexpected/quite annoying jump/shift" when the Ajax is processing, as reported in #94. It does that with the proposed solution in https://github.com/WordPress/wp-autoupdates/issues/94#issuecomment-623570523

Also fixes several other WPCS-related issues and an a11y problem where @aria-label wasn't correct in one place.

If this gets accepted, then we can close #99.

pbiron commented 4 years ago

While this is ready to be reviewed, it's not quite ready to be merged. It still needs at least 2 things:

  1. delay showing the spinning icon for a short time, so that if the Ajax completes really quickly would be less "jumpiness", as described in https://github.com/WordPress/wp-autoupdates/issues/94#issuecomment-622572079
  2. remove the reliance on wpAjax.unserialize()

I've got some other things to attend to, I'll get to those as soon as I can.

pbiron commented 4 years ago

@audrasjb Can you please review the way I did the strings in wp_autoupdates_toggle_auto_updates()? I not sure they are done in a way that is helpful to translators.

pbiron commented 4 years ago

Thinking this is ready to merge with couple of tiny changes/improvements. Can be refined more while or after the core patch if needed.

I'm just about done with replacing wpAjax.unserialize() with @data attributes. Will push that shortly.

pbiron commented 4 years ago

@azaozz I think it can be merged now, and any other changes that are necessary can be done in a separate PR.