Skip to content

Conversation

@gcsecsey
Copy link
Contributor

@gcsecsey gcsecsey commented Jan 23, 2026

Related issues

Proposed Changes

When the deleted site was the selected site, the tab should reset to "Overview". However, calling setSelectedTab('overview') from the site details hook didn't work reliably. The ContentTabsProvider wraps SiteDetailsProvider, so when both states update, the site change renders the component before the tab update takes effect, causing the TabPanel to remount with the old tab value.

This PR moves the state update call from the onDeleteSite callback to SiteContentTabs, and adds logic to compute an effectiveTab value at render time. The component tracks the previous site ID in state and, during render, checks if the site changed AND the previous site was deleted. If both conditions are true, it uses 'overview' as the effective tab for initialTabName. A useEffect then syncs this back to the tab context.

Testing Instructions

  1. Start Studio with npm start
  2. Create three sites (Site 1, Site 2, Site 3)
  3. Delete Site 3
  4. While the deletion is going on, quickly select Site 2 and switch to the "Sync" tab or any other tab
  5. Check that after the deletion, Site 2 and the previously selected tab remain selected
  6. Delete Site 2, and stay on the Settings tab until the deletion is completed
  7. Check that Site 1 on the Overview tab is selected after the deletion
CleanShot.2026-01-26.at.11.09.23.mp4

Pre-merge Checklist

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

- Update the logic to reset the selected tab to 'overview' only if the currently selected site no longer exists and a selection change has occurred.
- Improve clarity in comments regarding selection changes and tab resets.
@gcsecsey gcsecsey requested a review from a team January 23, 2026 16:19
@gcsecsey gcsecsey marked this pull request as draft January 23, 2026 16:36
@wpmobilebot
Copy link

wpmobilebot commented Jan 23, 2026

📊 Performance Test Results

Comparing 27d6e4d vs trunk

site-editor

Metric trunk 27d6e4d Diff Change
load 2904.00 ms 2901.00 ms -3.00 ms 🟢 -0.1%

site-startup

Metric trunk 27d6e4d Diff Change
siteCreation 8082.00 ms 7103.00 ms -979.00 ms 🟢 -12.1%
siteStartup 3938.00 ms 3935.00 ms -3.00 ms 🟢 -0.1%

Results are median values from multiple test runs.

Legend: 🟢 Improvement (faster) | 🔴 Regression (slower) | ⚪ No change

Copy link
Member

@sejas sejas left a comment

Choose a reason for hiding this comment

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

I tested it and it mostly worked, the only issue is that my selected site changed from one existing site to another automatically.
Steps to reproduce:

  • Delete site 3
  • Quickly switch to site 2
  • Observe that Studio switches automatically to the first site instead of keeping you in the existing site 2.

Here is a recording:

test-deleting-sites-and-tabs.mp4

@gcsecsey gcsecsey requested a review from a team January 26, 2026 11:46
@gcsecsey gcsecsey marked this pull request as ready for review January 26, 2026 11:51
Copy link
Member

@sejas sejas left a comment

Choose a reason for hiding this comment

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

@gcsecsey , thanks for making the changes. I tested it and it works as expected, althoug I think we added more changes in site-content-tabs whcih I think are not necesssary.

I created this alternate PR using yours as base and it seems to work. #2480

Copy link
Member

@sejas sejas left a comment

Choose a reason for hiding this comment

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

Works as expected! Great work!

Comment on lines 32 to 50
// Track previous site ID in state so we can safely access it during render
const [ prevSiteId, setPrevSiteId ] = useState( selectedSite?.id );

// Compute effective tab at render time
// This ensures TabPanel gets the correct initialTabName on the first render after deletion
const siteChanged = prevSiteId !== selectedSite?.id;
const prevSiteWasDeleted = prevSiteId && ! sites.some( ( site ) => site.id === prevSiteId );
const effectiveTab = siteChanged && prevSiteWasDeleted ? 'overview' : selectedTab;

// Update prevSiteId selectedTab after render
useEffect( () => {
if ( siteChanged ) {
setPrevSiteId( selectedSite?.id );
}

if ( effectiveTab !== selectedTab ) {
setSelectedTab( effectiveTab as TabName );
}
}, [ siteChanged, selectedSite?.id, effectiveTab, selectedTab, setSelectedTab ] );
Copy link
Member

@sejas sejas Jan 27, 2026

Choose a reason for hiding this comment

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

Maybe we could isolate these new hooks into separate custom hook.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @sejas that's a good suggestion. I moved it to a new hook in 27d6e4d.

@gcsecsey gcsecsey requested a review from a team January 27, 2026 15:06
Copy link
Contributor

@epeicher epeicher left a comment

Choose a reason for hiding this comment

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

Thanks @gcsecsey for solving this! I have tested it, and it works as expected. The site selected and the tab selected remains after the site deletion completes. I have also confirmed that if no other sites are selected, the first site in the Overview tab is opened. Changes also LGTM! :shipit:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants