Skip to content

Conversation

@katinthehatsite
Copy link
Contributor

@katinthehatsite katinthehatsite commented Jan 3, 2025

Related issues

Closes https://github.com/Automattic/dotcom-forge/issues/9759

Proposed Changes

This PR ensures that multiple import processes can be started at the same time from the Import / Export tab for different sites. It is currently possible with the drag and drop functionality but not with file selector.

Testing Instructions

  • Pull the changes from this branch
  • Start the app with npm start
  • Navigate to the Import/Export tab on Studio site A
  • Start the import process with file selector e.g. by clicking to select a file (and not drag and drop)
  • Switch to the Import/Export tab on Studio site B
  • Start the import process with file selector e.g. by clicking to select a file (and not drag and drop)
  • Confirm the second import starts

Pre-merge Checklist

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

@katinthehatsite katinthehatsite self-assigned this Jan 3, 2025
Copy link
Contributor

@fredrikekelund fredrikekelund left a comment

Choose a reason for hiding this comment

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

Nice catch, and this change works as expected 👍

However, I think it'd make sense to solve this at a more foundational level. The entire "main view" component tree should re-render when we switch sites, IMO. If it did, it would also resolve this problem, since React would instantiate a new <input>.

Changing the key prop passed to TabPanel accomplishes that:

diff --git a/src/components/site-content-tabs.tsx b/src/components/site-content-tabs.tsx
index c95f8b6..56cebb2 100644
--- a/src/components/site-content-tabs.tsx
+++ b/src/components/site-content-tabs.tsx
@@ -41,7 +41,7 @@ export function SiteContentTabs() {
                                        orientation="horizontal"
                                        onSelect={ ( tabName ) => setSelectedTab( tabName as TabName ) }
                                        initialTabName={ selectedTab }
-                                       key={ selectedTab }
+                                       key={ selectedSite.id + selectedTab }
                                >
                                        { ( { name } ) => (
                                                <div className="h-full">

Copy link
Contributor

@fredrikekelund fredrikekelund left a comment

Choose a reason for hiding this comment

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

@katinthehatsite
Copy link
Contributor Author

Sounds good, thanks for the review @fredrikekelund - I will merge this for now 👍

@katinthehatsite katinthehatsite merged commit 82be02a into trunk Jan 9, 2025
7 checks passed
@katinthehatsite katinthehatsite deleted the fix/import-can-not-be-started-on-multiple-sites branch January 9, 2025 13:10
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