Skip to content

Commit 7101550

Browse files
sejassejas
authored
Simplify edit site state when called from context menu in the sidebar (#1807)
* Remove setTimeout and move edit modal state to site-content-tabs * Move the edit modal is open state to use site details context * fix tests * fix tests
1 parent d1fa0db commit 7101550

File tree

7 files changed

+500
-433
lines changed

7 files changed

+500
-433
lines changed

‎src/components/content-tab-settings.tsx‎

Lines changed: 4 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
import { DropdownMenu, MenuGroup, Button } from '@wordpress/components';
22
import { moreVertical } from '@wordpress/icons';
33
import { useI18n } from '@wordpress/react-i18n';
4-
import { PropsWithChildren, useEffect, useRef } from 'react';
4+
import { PropsWithChildren } from 'react';
55
import { CopyTextButton } from 'src/components/copy-text-button';
66
import DeleteSite from 'src/components/delete-site';
77
import { useGetWpVersion } from 'src/hooks/use-get-wp-version';
88
import { getIpcApi } from 'src/lib/get-ipc-api';
99
import { decodePassword } from 'src/lib/passwords';
10-
import EditSiteDetails, { EditSiteDetailsRef } from 'src/modules/site-settings/edit-site-details';
10+
import EditSiteDetails from 'src/modules/site-settings/edit-site-details';
1111
import { useAppDispatch } from 'src/stores';
1212
import {
1313
certificateTrustApi,
@@ -16,8 +16,6 @@ import {
1616

1717
interface ContentTabSettingsProps {
1818
selectedSite: SiteDetails;
19-
shouldOpenEditModal?: boolean;
20-
onEditModalOpened?: () => void;
2119
}
2220

2321
function SettingsRow( { children, label }: PropsWithChildren< { label: string } > ) {
@@ -31,15 +29,10 @@ function SettingsRow( { children, label }: PropsWithChildren< { label: string }
3129
);
3230
}
3331

34-
export function ContentTabSettings( {
35-
selectedSite,
36-
shouldOpenEditModal,
37-
onEditModalOpened,
38-
}: ContentTabSettingsProps ) {
32+
export function ContentTabSettings( { selectedSite }: ContentTabSettingsProps ) {
3933
const dispatch = useAppDispatch();
4034
const { __ } = useI18n();
4135
const { data: isCertificateTrusted } = useCheckCertificateTrustQuery();
42-
const editSiteRef = useRef< EditSiteDetailsRef >( null );
4336
const username = 'admin';
4437
// Empty strings account for legacy sites lacking a stored password.
4538
const storedPassword = decodePassword( selectedSite.adminPassword ?? '' );
@@ -57,28 +50,14 @@ export function ContentTabSettings( {
5750
await dispatch( certificateTrustApi.util.invalidateTags( [ 'CertificateTrust' ] ) );
5851
};
5952

60-
// Open edit modal when requested from context menu
61-
useEffect( () => {
62-
if ( shouldOpenEditModal ) {
63-
setTimeout( () => {
64-
editSiteRef.current?.openModal();
65-
onEditModalOpened?.();
66-
}, 100 );
67-
}
68-
}, [ shouldOpenEditModal, onEditModalOpened ] );
69-
7053
return (
7154
<div className="p-8 ltr:pr-4 rtl:pl-4">
7255
<div className="flex justify-between items-center mb-4">
7356
<h3 role="heading" className="text-black text-sm font-semibold">
7457
{ __( 'Site details' ) }
7558
</h3>
7659
<div className="flex items-center gap-1">
77-
<EditSiteDetails
78-
ref={ editSiteRef }
79-
currentWpVersion={ wpVersion }
80-
onSave={ refreshWpVersion }
81-
/>
60+
<EditSiteDetails currentWpVersion={ wpVersion } onSave={ refreshWpVersion } />
8261
<DropdownMenu
8362
icon={ moreVertical }
8463
label={ __( 'More options' ) }

‎src/components/site-content-tabs.tsx‎

Lines changed: 2 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import { TabPanel } from '@wordpress/components';
22
import { useI18n } from '@wordpress/react-i18n';
3-
import { useEffect, useState } from 'react';
43
import { ContentTabAssistant } from 'src/components/content-tab-assistant';
54
import { ContentTabImportExport } from 'src/components/content-tab-import-export';
65
import { ContentTabOverview } from 'src/components/content-tab-overview';
@@ -17,34 +16,10 @@ import { cx } from 'src/lib/cx';
1716
import { ContentTabSync } from 'src/modules/sync';
1817

1918
export function SiteContentTabs() {
20-
const { selectedSite, data: localSites, setSelectedSiteId } = useSiteDetails();
19+
const { selectedSite, data: localSites } = useSiteDetails();
2120
const { importState } = useImportExport();
2221
const { tabs, selectedTab, setSelectedTab } = useContentTabs();
2322
const { __ } = useI18n();
24-
const [ shouldOpenEditModal, setShouldOpenEditModal ] = useState( false );
25-
26-
useEffect( () => {
27-
const handleEditSiteRequest = ( event: CustomEvent ) => {
28-
const { siteId } = event.detail;
29-
const targetSite = localSites.find( ( site ) => site.id === siteId );
30-
31-
if ( ! targetSite ) {
32-
return;
33-
}
34-
35-
if ( siteId !== selectedSite?.id ) {
36-
setSelectedSiteId( siteId );
37-
}
38-
39-
setSelectedTab( 'settings' );
40-
setShouldOpenEditModal( true );
41-
};
42-
43-
window.addEventListener( 'edit-site-request', handleEditSiteRequest as EventListener );
44-
return () => {
45-
window.removeEventListener( 'edit-site-request', handleEditSiteRequest as EventListener );
46-
};
47-
}, [ selectedSite?.id, localSites, setSelectedTab, setSelectedSiteId ] );
4823

4924
if ( ! localSites.length ) {
5025
return <EmptyStudio />;
@@ -87,13 +62,7 @@ export function SiteContentTabs() {
8762
{ name === 'overview' && <ContentTabOverview selectedSite={ selectedSite } /> }
8863
{ name === 'previews' && <ContentTabPreviews selectedSite={ selectedSite } /> }
8964
{ name === 'sync' && <ContentTabSync selectedSite={ selectedSite } /> }
90-
{ name === 'settings' && (
91-
<ContentTabSettings
92-
selectedSite={ selectedSite }
93-
shouldOpenEditModal={ shouldOpenEditModal }
94-
onEditModalOpened={ () => setShouldOpenEditModal( false ) }
95-
/>
96-
) }
65+
{ name === 'settings' && <ContentTabSettings selectedSite={ selectedSite } /> }
9766
{ name === 'assistant' && <ContentTabAssistant selectedSite={ selectedSite } /> }
9867
{ name === 'import-export' && <ContentTabImportExport selectedSite={ selectedSite } /> }
9968
</div>

‎src/components/site-menu.tsx‎

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { __, sprintf } from '@wordpress/i18n';
55
import { useEffect } from 'react';
66
import { Tooltip } from 'src/components/tooltip';
77
import { useSyncSites } from 'src/hooks/sync-sites';
8+
import { useContentTabs } from 'src/hooks/use-content-tabs';
89
import { useImportExport } from 'src/hooks/use-import-export';
910
import { useSiteDetails } from 'src/hooks/use-site-details';
1011
import { isMac, isWindows } from 'src/lib/app-globals';
@@ -113,8 +114,16 @@ function ButtonToRun( { running, id, name }: Pick< SiteDetails, 'running' | 'id'
113114
);
114115
}
115116
function SiteItem( { site }: { site: SiteDetails } ) {
116-
const { selectedSite, setSelectedSiteId, startServer, stopServer, deleteSite, loadingServer } =
117-
useSiteDetails();
117+
const {
118+
selectedSite,
119+
setSelectedSiteId,
120+
startServer,
121+
stopServer,
122+
deleteSite,
123+
loadingServer,
124+
setIsEditModalOpen,
125+
} = useSiteDetails();
126+
const { setSelectedTab } = useContentTabs();
118127
const isSelected = site === selectedSite;
119128
const { isSiteImporting, isSiteExporting } = useImportExport();
120129
const { isSiteIdPulling } = useSyncSites();
@@ -203,11 +212,11 @@ function SiteItem( { site }: { site: SiteDetails } ) {
203212
} )();
204213
break;
205214
case 'edit-site':
206-
window.dispatchEvent(
207-
new CustomEvent( 'edit-site-request', {
208-
detail: { siteId: site.id },
209-
} )
210-
);
215+
if ( site.id !== selectedSite?.id ) {
216+
setSelectedSiteId( site.id );
217+
}
218+
setSelectedTab( 'settings' );
219+
setIsEditModalOpen( true );
211220
break;
212221
case 'delete': {
213222
const DELETE_BUTTON_INDEX = 0;

‎src/components/tests/content-tab-settings.test.tsx‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,13 +267,28 @@ describe( 'ContentTabSettings', () => {
267267
updateSite,
268268
startServer,
269269
stopServer,
270+
isEditModalOpen: false,
271+
setIsEditModalOpen: jest.fn(),
270272
} );
271273

272274
const { rerender } = renderWithProvider(
273275
<ContentTabSettings selectedSite={ selectedSite } />
274276
);
275277
expect( screen.getByText( '8.3' ) ).toBeVisible();
276278
await user.click( screen.getByRole( 'button', { name: 'Edit site' } ) );
279+
( useSiteDetails as jest.Mock ).mockReturnValue( {
280+
selectedSite: { ...selectedSite, running: false } as SiteDetails,
281+
updateSite,
282+
startServer,
283+
stopServer,
284+
isEditModalOpen: true,
285+
setIsEditModalOpen: jest.fn(),
286+
} );
287+
rerenderWithProvider( rerender, <ContentTabSettings selectedSite={ selectedSite } /> );
288+
await waitFor( () => {
289+
expect( screen.getByRole( 'dialog' ) ).toBeInTheDocument();
290+
} );
291+
277292
const dialog = screen.getByRole( 'dialog' );
278293
expect( dialog ).toBeVisible();
279294
await user.selectOptions( within( dialog ).getByLabelText( 'PHP version' ), '8.2' );
@@ -283,6 +298,16 @@ describe( 'ContentTabSettings', () => {
283298
} )
284299
);
285300

301+
( useSiteDetails as jest.Mock ).mockReturnValue( {
302+
selectedSite: { ...selectedSite, running: false } as SiteDetails,
303+
updateSite,
304+
startServer,
305+
stopServer,
306+
isEditModalOpen: false,
307+
setIsEditModalOpen: jest.fn(),
308+
} );
309+
rerenderWithProvider( rerender, <ContentTabSettings selectedSite={ selectedSite } /> );
310+
286311
await waitFor( () => {
287312
expect( updateSite ).toHaveBeenCalledWith(
288313
expect.objectContaining( { phpVersion: '8.2' } )
@@ -313,13 +338,27 @@ describe( 'ContentTabSettings', () => {
313338
updateSite,
314339
startServer,
315340
stopServer,
341+
isEditModalOpen: false,
342+
setIsEditModalOpen: jest.fn(),
316343
} );
317344

318345
const { rerender } = renderWithProvider(
319346
<ContentTabSettings selectedSite={ selectedSite } />
320347
);
321348
expect( screen.getByText( '8.3' ) ).toBeVisible();
322349
await user.click( screen.getByRole( 'button', { name: 'Edit site' } ) );
350+
( useSiteDetails as jest.Mock ).mockReturnValue( {
351+
selectedSite: { ...selectedSite, running: true } as SiteDetails,
352+
updateSite,
353+
startServer,
354+
stopServer,
355+
isEditModalOpen: true,
356+
setIsEditModalOpen: jest.fn(),
357+
} );
358+
rerenderWithProvider( rerender, <ContentTabSettings selectedSite={ selectedSite } /> );
359+
await waitFor( () => {
360+
expect( screen.getByRole( 'dialog' ) ).toBeInTheDocument();
361+
} );
323362
const dialog = screen.getByRole( 'dialog' );
324363
expect( dialog ).toBeVisible();
325364
await user.selectOptions( within( dialog ).getByLabelText( 'PHP version' ), '8.2' );
@@ -329,6 +368,16 @@ describe( 'ContentTabSettings', () => {
329368
} )
330369
);
331370

371+
( useSiteDetails as jest.Mock ).mockReturnValue( {
372+
selectedSite: { ...selectedSite, running: true } as SiteDetails,
373+
updateSite,
374+
startServer,
375+
stopServer,
376+
isEditModalOpen: false,
377+
setIsEditModalOpen: jest.fn(),
378+
} );
379+
rerenderWithProvider( rerender, <ContentTabSettings selectedSite={ selectedSite } /> );
380+
332381
await waitFor( () => {
333382
expect( updateSite ).toHaveBeenCalledWith(
334383
expect.objectContaining( { phpVersion: '8.2' } )

‎src/hooks/use-site-details.tsx‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@ interface SiteDetailsContext {
4141
isDeleting: boolean;
4242
uploadingSites: { [ siteId: string ]: boolean };
4343
setUploadingSites: React.Dispatch< React.SetStateAction< { [ siteId: string ]: boolean } > >;
44+
isEditModalOpen: boolean;
45+
setIsEditModalOpen: React.Dispatch< React.SetStateAction< boolean > >;
4446
}
4547

4648
const defaultContext: SiteDetailsContext = {
@@ -58,6 +60,8 @@ const defaultContext: SiteDetailsContext = {
5860
isDeleting: false,
5961
uploadingSites: {},
6062
setUploadingSites: () => undefined,
63+
isEditModalOpen: false,
64+
setIsEditModalOpen: () => undefined,
6165
};
6266

6367
export const siteDetailsContext = createContext< SiteDetailsContext >( defaultContext );
@@ -462,6 +466,8 @@ export function SiteDetailsProvider( { children }: SiteDetailsProviderProps ) {
462466
setData( data.map( ( site ) => ( site.running ? { ...site, running: false } : site ) ) );
463467
}, [ data ] );
464468

469+
const [ isEditModalOpen, setIsEditModalOpen ] = useState( false );
470+
465471
const context = useMemo(
466472
() => ( {
467473
selectedSite: data.find( ( site ) => site.id === selectedSiteId ) || firstSite,
@@ -478,6 +484,8 @@ export function SiteDetailsProvider( { children }: SiteDetailsProviderProps ) {
478484
loadingSites,
479485
uploadingSites,
480486
setUploadingSites,
487+
isEditModalOpen,
488+
setIsEditModalOpen,
481489
} ),
482490
[
483491
data,
@@ -494,6 +502,8 @@ export function SiteDetailsProvider( { children }: SiteDetailsProviderProps ) {
494502
isDeleting,
495503
loadingSites,
496504
uploadingSites,
505+
isEditModalOpen,
506+
setIsEditModalOpen,
497507
]
498508
);
499509

0 commit comments

Comments
 (0)