Skip to content

Conversation

sejas
Copy link
Member

@sejas sejas commented Aug 15, 2025

Related issues

Proposed Changes

  • Remove useSiteSyncManagement
  • Implement connected sites as redux slice
  • Update tests to use the Redux store
  • No user-facing changes

Testing Instructions

  • Run npm start.
  • Go to the Sync tab.
  • Connect a site.
  • Observe that the site is connected.
  • Restart the app.
  • Observe that the connected site is still present.
  • Connect another site.
  • Observe that the newly connected site is unavailable.
  • Try to disconnect the site.
  • Observe that the site gets disconnected.

Pre-merge Checklist

  • Have you checked for TypeScript, React or other console errors?

@sejas sejas self-assigned this Aug 15, 2025
@sejas sejas changed the title Update/stu 420 move sync connected sites to redux Move Sync connected sites to redux slice Aug 15, 2025
@sejas sejas requested a review from a team August 15, 2025 10:12
Copy link
Contributor

@ivan-ottinger ivan-ottinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The proposed changes look great! I did not observe any regressions when testing connecting and reconnecting sites. 👍🏼

Considering the size of the PR it would be great if one more person could review it.

Copy link
Contributor

@gcsecsey gcsecsey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes look good to me too, and I also didn't encounter any regressions when testing. The connected sites are persisted between app restarts and I can connect multiple sites just as on the trunk version. 👍

Connecting a new site:

CleanShot.2025-09-02.at.14.56.21.mp4

After restarting the app, connecting and removing an additional site:

CleanShot.2025-09-02.at.15.05.15.mp4

@sejas sejas merged commit fd65621 into trunk Sep 2, 2025
12 checks passed
@sejas sejas deleted the update/STU-420-move-sync-connected-sites-to-redux branch September 2, 2025 22:14
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.

3 participants