Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Studio: Replace button loading indicator with full window one #362

Merged
merged 19 commits into from
Jul 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions e2e/sites.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,18 +37,19 @@ test.describe( 'Servers', () => {
await modal.siteNameInput.fill( siteName );
await modal.addSiteButton.click();

const sidebarButton = sidebar.getSiteNavButton( siteName );
await expect( sidebarButton ).toBeAttached( { timeout: 30_000 } );
const siteTitle = sidebar.getSiteNavButton( siteName );
await expect( siteTitle ).toHaveText( siteName );

// Check the site is running
const siteContent = new SiteContent( session.mainWindow, siteName );
await expect( siteContent.siteNameHeading ).toBeAttached( { timeout: 30_000 } );
expect( await siteContent.siteNameHeading ).toHaveText( siteName );

// Check a WordPress site has been created
expect(
await pathExists( path.join( session.homePath, 'Studio', siteName, 'wp-config.php' ) )
).toBe( true );

// Check the site is running
const siteContent = new SiteContent( session.mainWindow, siteName );
expect( await siteContent.siteNameHeading ).toHaveText( siteName );

await siteContent.navigateToTab( 'Settings' );

expect( await siteContent.frontendButton ).toBeVisible();
Expand Down
15 changes: 9 additions & 6 deletions src/components/add-site.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { useI18n } from '@wordpress/react-i18n';
import { FormEvent, useCallback, useEffect, useState } from 'react';
import { useAddSite } from '../hooks/use-add-site';
import { useIpcListener } from '../hooks/use-ipc-listener';
import { useSiteDetails } from '../hooks/use-site-details';
import { generateSiteName } from '../lib/generate-site-name';
import { getIpcApi } from '../lib/get-ipc-api';
import Button from './button';
Expand All @@ -18,10 +19,10 @@ export default function AddSite( { className }: AddSiteProps ) {
const { __ } = useI18n();
const [ showModal, setShowModal ] = useState( false );
const [ nameSuggested, setNameSuggested ] = useState( false );
const { data } = useSiteDetails();

const {
handleAddSiteClick,
isAddingSite,
siteName,
setSiteName,
setProposedSitePath,
Expand All @@ -37,6 +38,8 @@ export default function AddSite( { className }: AddSiteProps ) {
loadingSites,
} = useAddSite();

const isSiteAdding = data.some( ( site ) => site.isAddingSite );
Copy link
Member

Choose a reason for hiding this comment

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

Nice!


const siteAddedMessage = sprintf(
// translators: %s is the site name.
__( '%s site added.' ),
Expand Down Expand Up @@ -82,10 +85,10 @@ export default function AddSite( { className }: AddSiteProps ) {
async ( event: FormEvent ) => {
event.preventDefault();
try {
closeModal();
await handleAddSiteClick();
speak( siteAddedMessage );
setNameSuggested( false );
closeModal();
} catch {
// No need to handle error here, it's already handled in handleAddSiteClick
}
Expand Down Expand Up @@ -117,16 +120,16 @@ export default function AddSite( { className }: AddSiteProps ) {
doesPathContainWordPress={ doesPathContainWordPress }
>
<div className="flex flex-row justify-end gap-x-5 mt-6">
<Button onClick={ closeModal } disabled={ isAddingSite } variant="tertiary">
<Button onClick={ closeModal } disabled={ isSiteAdding } variant="tertiary">
{ __( 'Cancel' ) }
</Button>
<Button
type="submit"
variant="primary"
isBusy={ isAddingSite }
disabled={ isAddingSite || !! error || ! siteName?.trim() }
isBusy={ isSiteAdding }
disabled={ isSiteAdding || !! error || ! siteName?.trim() }
katinthehatsite marked this conversation as resolved.
Show resolved Hide resolved
>
{ isAddingSite ? __( 'Adding site…' ) : __( 'Add site' ) }
{ __( 'Add site' ) }
katinthehatsite marked this conversation as resolved.
Show resolved Hide resolved
</Button>
</div>
</SiteForm>
Expand Down
10 changes: 2 additions & 8 deletions src/components/onboarding.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ export default function Onboarding() {
const { __ } = useI18n();
const {
setSiteName,
isAddingSite,
setProposedSitePath,
setSitePath,
setError,
Expand Down Expand Up @@ -114,13 +113,8 @@ export default function Onboarding() {
onSubmit={ handleSubmit }
>
<div className="flex flex-row gap-x-5 mt-6">
<Button
type="submit"
isBusy={ isAddingSite }
disabled={ !! error || isAddingSite }
variant="primary"
>
{ isAddingSite ? __( 'Adding site…' ) : __( 'Add site' ) }
<Button type="submit" variant="primary">
{ __( 'Add site' ) }
</Button>
</div>
</SiteForm>
Expand Down
5 changes: 5 additions & 0 deletions src/components/site-content-tabs.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { ContentTabOverview } from './content-tab-overview';
import { ContentTabSettings } from './content-tab-settings';
import { ContentTabSnapshots } from './content-tab-snapshots';
import Header from './header';
import { SiteLoadingIndicator } from './site-loading-indicator';

export function SiteContentTabs() {
const { selectedSite } = useSiteDetails();
Expand All @@ -23,6 +24,10 @@ export function SiteContentTabs() {
);
}

if ( selectedSite?.isAddingSite ) {
return <SiteLoadingIndicator selectedSiteName={ selectedSite.name } />;
}

return (
<div className="flex flex-col w-full h-full app-no-drag-region pt-8 overflow-y-auto">
<Header />
Expand Down
38 changes: 38 additions & 0 deletions src/components/site-loading-indicator.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { useI18n } from '@wordpress/react-i18n';
import { useEffect } from 'react';
import { useProgressTimer } from '../hooks/use-progress-timer';
import ProgressBar from './progress-bar';

export function SiteLoadingIndicator( { selectedSiteName }: { selectedSiteName?: string } ) {
const { __ } = useI18n();

const { progress, setProgress } = useProgressTimer( {
initialProgress: 20,
interval: 1500,
maxValue: 95,
} );

useEffect( () => {
const updateProgress = () => {
setProgress( ( prev ) => {
const increment = Math.random() * 10 + 5;
return Math.min( prev + increment, 95 );
} );
};

setProgress( 50 );
const interval = setInterval( updateProgress, 1000 );

return () => clearInterval( interval );
}, [ setProgress ] );

return (
<div className="flex flex-col w-full h-full app-no-drag-region pt-8 overflow-y-auto justify-center items-center">
<div className="w-[300px] text-center">
<div className="text-black a8c-subtitle-small mb-4">{ selectedSiteName }</div>
<ProgressBar value={ progress } maxValue={ 100 } />
<div className="text-a8c-gray-70 a8c-body mt-4">{ __( 'Creating site...' ) }</div>
</div>
katinthehatsite marked this conversation as resolved.
Show resolved Hide resolved
</div>
);
}
7 changes: 6 additions & 1 deletion src/components/site-menu.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { speak } from '@wordpress/a11y';
import { Spinner } from '@wordpress/components';
import { __, sprintf } from '@wordpress/i18n';
import { useEffect } from 'react';
import { useSiteDetails } from '../hooks/use-site-details';
Expand Down Expand Up @@ -113,7 +114,11 @@ function SiteItem( { site }: { site: SiteDetails } ) {
>
{ site.name }
</button>
<ButtonToRun { ...site } />
{ site.isAddingSite ? (
<Spinner className="!w-2.5 !h-2.5 !top-[6px] !mr-2" />
) : (
<ButtonToRun { ...site } />
) }
</li>
);
}
Expand Down
44 changes: 0 additions & 44 deletions src/components/tests/add-site.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -205,48 +205,4 @@ describe( 'AddSite', () => {
screen.getByDisplayValue( '/default_path/my-wordpress-website-mutated' )
).toBeVisible();
} );

it( 'should display a helpful error message when an error occurs while creating the site', async () => {
const user = userEvent.setup();
mockGenerateProposedSitePath.mockResolvedValue( {
path: '/default_path/my-wordpress-website',
name: 'My WordPress Website',
isEmpty: true,
isWordPress: false,
} );
mockCreateSite.mockImplementation( () => {
throw new Error( 'Failed to create site' );
} );
render( <AddSite /> );

await user.click( screen.getByRole( 'button', { name: 'Add site' } ) );
await user.click( screen.getByRole( 'button', { name: 'Add site' } ) );

await waitFor( () => {
expect( screen.getByRole( 'alert' ) ).toHaveTextContent(
'An error occurred while creating the site. Verify your selected local path is an empty directory or an existing WordPress folder and try again. If this problem persists, please contact support.'
);
} );
} );

it( 'should disable submissions while the site is being added', async () => {
const user = userEvent.setup();
mockGenerateProposedSitePath.mockResolvedValue( {
path: '/default_path/my-wordpress-website',
name: 'My WordPress Website',
isEmpty: true,
isWordPress: false,
} );
mockCreateSite.mockImplementationOnce( () => {
return new Promise( () => {
// no-op
} );
} );
render( <AddSite /> );

await user.click( screen.getByRole( 'button', { name: 'Add site' } ) );
await user.click( screen.getByRole( 'button', { name: 'Add site' } ) );

expect( screen.getByRole( 'button', { name: 'Adding site…' } ) ).toBeDisabled();
} );
} );
14 changes: 1 addition & 13 deletions src/hooks/use-add-site.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ export function useAddSite() {
const { __ } = useI18n();
const { createSite, data: sites, loadingSites } = useSiteDetails();
const [ error, setError ] = useState( '' );
const [ isAddingSite, setIsAddingSite ] = useState( false );
const [ siteName, setSiteName ] = useState< string | null >( null );
const [ sitePath, setSitePath ] = useState( '' );
const [ proposedSitePath, setProposedSitePath ] = useState( '' );
Expand Down Expand Up @@ -51,22 +50,13 @@ export function useAddSite() {
}, [ __, siteWithPathAlreadyExists, siteName, proposedSitePath ] );

const handleAddSiteClick = useCallback( async () => {
setIsAddingSite( true );
try {
const path = sitePath ? sitePath : proposedSitePath;
await createSite( path, siteName ?? '' );
} catch ( e ) {
Sentry.captureException( e );
setError(
__(
'An error occurred while creating the site. Verify your selected local path is an empty directory or an existing WordPress folder and try again. If this problem persists, please contact support.'
)
);
setIsAddingSite( false );
throw e;
katinthehatsite marked this conversation as resolved.
Show resolved Hide resolved
}
setIsAddingSite( false );
}, [ createSite, proposedSitePath, siteName, sitePath, __ ] );
}, [ createSite, proposedSitePath, siteName, sitePath ] );

const handleSiteNameChange = useCallback(
async ( name: string ) => {
Expand Down Expand Up @@ -103,7 +93,6 @@ export function useAddSite() {
handleAddSiteClick,
handlePathSelectorClick,
handleSiteNameChange,
isAddingSite,
error: siteWithPathAlreadyExists( sitePath ? sitePath : proposedSitePath )
? __(
'Another site already exists at this path. Please select an empty directory to create a site.'
Expand All @@ -130,7 +119,6 @@ export function useAddSite() {
handlePathSelectorClick,
siteWithPathAlreadyExists,
handleSiteNameChange,
isAddingSite,
siteName,
sitePath,
proposedSitePath,
Expand Down
72 changes: 63 additions & 9 deletions src/hooks/use-site-details.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
useState,
} from 'react';
import { getIpcApi } from '../lib/get-ipc-api';
import { sortSites } from '../lib/sort-sites';
import { useSnapshots } from './use-snapshots';

interface SiteDetailsContext {
Expand Down Expand Up @@ -64,12 +65,15 @@ function useSelectedSite( firstSiteId: string | null ) {
const [ selectedSiteId, setSelectedSiteId ] = useState< string | null >(
selectedSiteIdFromLocal
);
useEffect( () => {
if ( selectedSiteId ) {
localStorage.setItem( SELECTED_SITE_ID_KEY, selectedSiteId );
}
} );

return {
selectedSiteId: selectedSiteId || firstSiteId,
setSelectedSiteId: ( id: string ) => {
setSelectedSiteId( id );
localStorage.setItem( SELECTED_SITE_ID_KEY, id );
},
setSelectedSiteId,
};
}

Expand Down Expand Up @@ -159,11 +163,61 @@ export function SiteDetailsProvider( { children }: SiteDetailsProviderProps ) {

const createSite = useCallback(
async ( path: string, siteName?: string ) => {
const data = await getIpcApi().createSite( path, siteName );
setData( data );
const newSite = data.find( ( site ) => site.path === path );
if ( newSite?.id ) {
setSelectedSiteId( newSite.id );
// Function to handle error messages and cleanup
const showError = () => {
console.error( 'Failed to create site' );
getIpcApi().showMessageBox( {
type: 'error',
message: __( 'Failed to create site' ),
detail: __(
'An error occurred while creating the site. Verify your selected local path is an empty directory or an existing WordPress folder and try again. If this problem persists, please contact support.'
),
buttons: [ __( 'OK' ) ],
} );

// Remove the temporary site immediately, but with a minor delay to ensure state updates properly
setTimeout( () => {
setData( ( prevData ) =>
sortSites( prevData.filter( ( site ) => site.id !== tempSiteId ) )
);
}, 2000 );
};

const tempSiteId = crypto.randomUUID();
setData( ( prevData ) =>
sortSites( [
...prevData,
{
id: tempSiteId,
name: siteName || path,
path,
running: false,
isAddingSite: true,
phpVersion: '',
},
] )
);
setSelectedSiteId( tempSiteId ); // Set the temporary ID as the selected site

try {
const data = await getIpcApi().createSite( path, siteName );
const newSite = data.find( ( site ) => site.path === path );
if ( ! newSite ) {
showError();
return;
}
// Update the selected site to the new site's ID if the user didn't change it
setSelectedSiteId( ( prevSelectedSiteId ) => {
if ( prevSelectedSiteId === tempSiteId ) {
return newSite.id;
}
return prevSelectedSiteId;
} );
setData( ( prevData ) =>
prevData.map( ( site ) => ( site.id === tempSiteId ? newSite : site ) )
);
} catch ( error ) {
showError();
}
},
[ setSelectedSiteId ]
Expand Down
1 change: 1 addition & 0 deletions src/ipc-types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ interface StoppedSiteDetails {
supportsWidgets: boolean;
supportsMenus: boolean;
};
isAddingSite?: boolean;
}

interface StartedSiteDetails extends StoppedSiteDetails {
Expand Down
Loading