Skip to content

Reset component state when switching sites - #1247

Merged
fredrikekelund merged 1 commit into
trunkfrom
f26d/reset-state-when-switching-sites
Apr 23, 2025
Merged

Reset component state when switching sites#1247
fredrikekelund merged 1 commit into
trunkfrom
f26d/reset-state-when-switching-sites

Conversation

@fredrikekelund

@fredrikekelund fredrikekelund commented Apr 23, 2025

Copy link
Copy Markdown
Contributor

Related issues

Proposed Changes

In #775, we worked on an issue caused by React state not resetting when selecting another site in Studio. This happens because the key prop passed to TabPanel references only the active tab and not the selected site.

@katinthehatsite raised another issue in #1242 (comment) that I suspect is related to the same thing.

This PR fixes the problem by including the selected site ID in the key prop passed to TabPanel.

See the React docs for more details on resetting state with a key prop.

Testing Instructions

Ideally, @katinthehatsite would retry the same steps as in #1242 (comment) and ensure that it works as expected now.

For other testers, do a light smoke test of the Sync tab, the Previews tab, the Import/Export tab, and the Settings tab. While this should be a low-risk change, it also has far-reaching effects on state handling.

Pre-merge Checklist

  • Have you checked for TypeScript, React or other console errors?
@fredrikekelund
fredrikekelund requested review from a team and katinthehatsite April 23, 2025 08:57
@fredrikekelund fredrikekelund self-assigned this Apr 23, 2025

@epeicher epeicher left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the changes! They make sense to me and after some general testing, I have not found any issues. LGTM! :shipit:

CleanShot.2025-04-23.at.11.33.38.mp4

@wojtekn wojtekn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Works fine in my tests.

@katinthehatsite katinthehatsite left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice, thanks for the fix!

I (of course 🤦 ) was testing something else in the meantime so I removed the expired sites on my end so I can not test with the original issue. But from what I saw earlier, the changes make sense to me.

@fredrikekelund
fredrikekelund merged commit 06bab99 into trunk Apr 23, 2025
@fredrikekelund
fredrikekelund deleted the f26d/reset-state-when-switching-sites branch April 23, 2025 10:04
bgrgicak pushed a commit that referenced this pull request Apr 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants