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

Refactor backend JS to remove jQuery dependency #2664

Merged
merged 10 commits into from
Mar 25, 2022
Merged

Conversation

JakePT
Copy link
Contributor

@JakePT JakePT commented Mar 18, 2022

Description of the Change

This PR refactors the JavaScript for the Settings, Weighting, and Network Sites screens, as well as the JavaScript for handling admin notice dismissals, to remove jQuery as a dependency. It also fixes the occasional bug in the original scripts.

For the ElasticPress indexing switches in the Network Sites screen I took this as an opportunity to replaced the custom switch with the core WordPress toggle control for more consistency with the WordPress UI and better accessibility and handling of a busy state to avoid race conditions I encountered with rapid switching.

elasticpress test_wp-admin_network_sites copy php
elasticpress test_wp-admin_network_sites php

Closes #2074

Verification Process

For the Settings screen, verify that:

  • Switching between tabs works.
  • Switching between tabs appropriately updates the label and description of the host field.
  • When switching between tabs you should be able to make changes to the host field without losing the value in the other tab.

For the Weighting screen, verify that:

  • Adjusting the slider updates the number value in the field label.
  • Unchecking the checkbox should disable the slider and update the number value to 0.
  • Checking the checkbox should re-enable the slider and restore the number value to reflect the slider position.

For the Network Sites screen, verify that:

  • The ElasticPress Indexing column has a toggle control in each row.
  • Switching the toggle on and off should update the label to read "On" or "Off".
  • After switching the toggle the toggle should be disabled while the AJAX request is running.
  • After changing the toggle the new value should be reflected when refreshing the page.

For admin notices, verify that:

  • When a notice is dismissed it should not reappear when refreshing or navigating to another page.

Checklist:

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests passed.

Changelog Entry

  • Changed: Refactored remaining admin scripts to remove jQuery as a dependency.

Credits

Props @JakePT

@JakePT JakePT requested a review from felipeelia March 18, 2022 06:49
@JakePT JakePT self-assigned this Mar 18, 2022
@JakePT
Copy link
Contributor Author

JakePT commented Mar 18, 2022

@felipeelia With the failing tests I fixed one with a small tweak to to the test to reflect the change in markup, but the other tests that are still failing don't appear to be related. Let me know if that's wrong.

@felipeelia felipeelia assigned felipeelia and unassigned JakePT Mar 18, 2022
@felipeelia
Copy link
Member

@JakePT Actually the input.click() was preventing the site from being indexed. I've pushed a new commit and if tests pass we can sync on the best time to merge this one. Thanks!

@felipeelia felipeelia added this to the 4.1.0 milestone Mar 25, 2022
@felipeelia felipeelia merged commit bca42e9 into develop Mar 25, 2022
@felipeelia felipeelia deleted the feature/2074 branch March 25, 2022 13:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor backend JS to remove jQuery dependency
2 participants