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

Sync page refactor. #2738

Merged
merged 27 commits into from
May 12, 2022
Merged

Sync page refactor. #2738

merged 27 commits into from
May 12, 2022

Conversation

JakePT
Copy link
Contributor

@JakePT JakePT commented May 3, 2022

Description of the Change

Rewrites the sync page JavaScript as a @wordpress/element (React) application to address #2704 and #2706.

Closes #2704, #2706.

Alternate Designs

I’ve taken the approach of building it as a React app, so I’ve basically also redone the markup and CSS. As part of that I’ve used a few standard WordPress components and also taken some liberties with the styling, to do things like making spacing more consistent. So basically in its current state there’s a bunch of visual changes compared to the current page that should be reviewed. Let me know if there’s things we’d prefer were more consistent with the existing page.

Note that with things like the stop button I’ve used the default ‘secondary’ style for buttons, because by sticking with the default styles for the standard button they can reflect the users’ admin color scheme (3rd and 4th pics). But let me know if we don’t care about that. I've similarly set the progress bar to match the admin color scheme.

Current page

elasticpress test_wp-admin_admin php_page=elasticpress-sync (1)

New page

elasticpress test_wp-admin_admin php_page=elasticpress-sync
elasticpress test_wp-admin_admin php_page=elasticpress-sync copy
elasticpress test_wp-admin_admin php_page=elasticpress-sync copy 2

An idea I tried on a lark was setting the color scheme to ‘ElasticPress red’ as a way of branding the core components. Note this hasn't been implemented.

elasticpress test_wp-admin_admin php_page=elasticpress-sync

Verification Process

From the new version testing user stories:

  • In any page, if user hovers the sync and settings button, it should show a description related to the button
  • If user clicks the sync button, it should redirect to the Sync page.
  • If user clicks the “Sync Now” button it should index everything without sending mappings.
  • If users click the sync button in multisite, network activated mode, all published posts across all sites should sync.
  • If user leaves page during sync, sync will stop. If user returns to sync page, user should be able to resume the sync.
  • If user tries to activate/deactivate features during a sync, they will be prevented.
  • If a user tries to WP-CLI sync during dashboard sync, they will be prevented .
  • If a user tries to sync during WP-CLI sync, they will be informed about the WP-CLI sync status and can stop it.
  • If a user tries to activate a feature that requires a reindex, a confirmation message will pop up.
  • If user clicks on “Delete all Data…” it will send mappings and reindex everything
  • If multiple indexables are enabled, they should not be affected when an indexable is being worked. Example: while indexing users, posts search should go through EP (pausing the sync may help to test this)

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 Sync page JavaScript.
Fixed - Issues where the Sync page would not show the correct status of a sync.

Credits

Props @JakePT

@JakePT JakePT requested a review from felipeelia May 3, 2022 13:49
@JakePT JakePT self-assigned this May 3, 2022
@JakePT JakePT linked an issue May 3, 2022 that may be closed by this pull request
2 tasks
@JakePT JakePT added this to the 4.2.0 milestone May 3, 2022
@JakePT JakePT marked this pull request as ready for review May 10, 2022 06:10
@JakePT JakePT changed the title WIP Sync page JS rewrite. Sync page refactor. May 10, 2022
@felipeelia felipeelia merged commit fdd5ace into develop May 12, 2022
@felipeelia felipeelia deleted the feature/2704 branch May 12, 2022 13:59
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.

BUG: Installation Workflow and initial sync Sync Page JS
2 participants