Refactor deep link connection handling and connectSite mutation#2665
Refactor deep link connection handling and connectSite mutation#2665
Conversation
- Simplified the `useListenDeepLinkConnection` hook by removing unnecessary queries and directly using the `connectSite` function with updated parameters. - Updated the `handleConnect` function in `ContentTabSync` to accept `remoteSiteId` instead of a full site object, streamlining the connection process. - Enhanced the `connectSite` mutation in the Redux slice to handle remote site connections more effectively, including error handling for site not found scenarios.
📊 Performance Test ResultsComparing df3ea3a vs trunk site-editor
site-startup
Results are median values from multiple test runs. Legend: 🟢 Improvement (faster) | 🔴 Regression (slower) | ⚪ No change (<50ms diff) |
|
One this is that we might want to double-check this with https://github.com/Automattic/studio/pull/2375/commits as they are likely going to overlap 😅 |
|
Also, perhaps this PR already solves the issue from the other PR - I will give it a test now 👀 |
katinthehatsite
left a comment
There was a problem hiding this comment.
I did not finish testing everything yet but I noticed that there is a small delay for me when I click on Connect site or Connect another site:
Screen.Recording.2026-02-26.at.11.18.18.AM.mov
Initially, I thought that it was not connecting so I clicked multiple times before seeing it connect. In the second try, I click and waited and then it connected. I think if we have the small delay, we should probably set the Connect button to loading state to indicate to the user that the connection is happening.
| setSelectedRemoteSite( selectedSiteFromList ); | ||
| } else { | ||
| await handleConnect( selectedSiteFromList ); | ||
| await handleConnect( siteId ); |
There was a problem hiding this comment.
There are two more calls for handleConnect in this file in onPull and onPush - do we also need to adjust passing an id there now that it expects a number?
There was a problem hiding this comment.
Well spotted! That code might be called when using deeplinks and it needed to be updated accordingly! Done as part of f78b4fd
…bject The onPush and onPull callbacks were passing the full SyncSite object to handleConnect, which expects a number (remoteSiteId). This caused the connectSite mutation to silently fail when matching by ID.
Ok, this can wait for that PR to be merged, this PR is not urgent as it is a refactor |
Absolutely, that will improve the UX. I will compare also with trunk and check if there are differences, but anyway, adding a loading state will prevent confusion |
Related issues
Proposed Changes
useListenDeepLinkConnectionhook by removing unnecessary queries and directly using theconnectSitefunction with updated parameters.handleConnectfunction inContentTabSyncto acceptremoteSiteIdinstead of a full site object, streamlining the connection process.connectSitemutation in the Redux slice to handle remote site connections more effectively, including error handling for site not found scenarios.Testing Instructions
Connect a site via the Sync tab
Deep link connection
wp-studio://sync-connect-site?studioSiteId=<local_studio_id>&remoteSiteId=<remote_blog_id>Deep link with auto-open push
Publish site, and follow the process to create the siteAdd a new connected site
Pre-merge Checklist