Improved AdminX theme installation flow (#17175)

refs https://github.com/TryGhost/Team/issues/3349
TryGhost · Jul 3, 2023 · 3811999 · 3811999
1 parent 8b164b8
commit 3811999
Show file tree

Hide file tree

Showing 4 changed files with 144 additions and 58 deletions.
diff --git a/apps/admin-x-settings/src/components/settings/site/ThemeModal.tsx b/apps/admin-x-settings/src/components/settings/site/ThemeModal.tsx
@@ -8,12 +8,11 @@ import OfficialThemes from './theme/OfficialThemes';
 import PageHeader from '../../../admin-x-ds/global/layout/PageHeader';
 import React, {useState} from 'react';
 import TabView from '../../../admin-x-ds/global/TabView';
-import ThemeInstalledModal from './ThemeInstalledModal';
+import ThemeInstalledModal from './theme/ThemeInstalledModal';
 import ThemePreview from './theme/ThemePreview';
 import {API} from '../../../utils/api';
 import {OfficialTheme} from '../../../models/themes';
 import {Theme} from '../../../types/api';
-import {showToast} from '../../../admin-x-ds/global/Toast';
 import {useApi} from '../../providers/ServiceProvider';
 import {useThemes} from '../../../hooks/useThemes';
 
@@ -68,17 +67,29 @@ async function handleThemeUpload({
     let title = 'Upload successful';
     let prompt = <>
         <strong>{uploadedTheme.name}</strong> uploaded successfully.
-        Do you want to activate it now?
     </>;
 
+    if (!uploadedTheme.active) {
+        prompt = <>
+            {prompt}{' '}
+            Do you want to activate it now?
+        </>;
+    }
+
     if (uploadedTheme.errors?.length || uploadedTheme.warnings?.length) {
         const hasErrors = uploadedTheme.errors?.length;
 
         title = `Upload successful with ${hasErrors ? 'errors' : 'warnings'}`;
         prompt = <>
             The theme <strong>"{uploadedTheme.name}"</strong> was installed successfully but we detected some {hasErrors ? 'errors' : 'warnings'}.
-            You are still able to activate and use the theme but it is recommended to fix these {hasErrors ? 'errors' : 'warnings'} before you do so.
         </>;
+
+        if (!uploadedTheme.active) {
+            prompt = <>
+                {prompt}
+                You are still able to activate and use the theme but it is recommended to fix these {hasErrors ? 'errors' : 'warnings'} before you do so.
+            </>;
+        }
     }
 
     NiceModal.show(ThemeInstalledModal, {
@@ -120,17 +131,17 @@ const ThemeToolbar: React.FC<ThemeToolbarProps> = ({
                         prompt: (
                             <>
                                 The theme <strong>{themeFileName}</strong> already exists.
-                                Do you want to overwrite it ?
+                                Do you want to overwrite it?
                             </>
                         ),
                         okLabel: 'Overwrite',
                         cancelLabel: 'Cancel',
                         okRunningLabel: 'Overwriting...',
                         okColor: 'red',
                         onOk: async (confirmModal) => {
-                            confirmModal?.remove();
+                            await handleThemeUpload({api, file, setThemes});
                             setCurrentTab('installed');
-                            handleThemeUpload({api, file, setThemes});
+                            confirmModal?.remove();
                         }
                     });
                 } else {
@@ -176,6 +187,7 @@ const ChangeThemeModal = NiceModal.create(() => {
     const [currentTab, setCurrentTab] = useState('official');
     const [selectedTheme, setSelectedTheme] = useState<OfficialTheme|null>(null);
     const [previewMode, setPreviewMode] = useState('desktop');
+    const [isInstalling, setInstalling] = useState(false);
 
     const modal = useModal();
     const {themes, setThemes} = useThemes();
@@ -190,16 +202,50 @@ const ChangeThemeModal = NiceModal.create(() => {
     if (selectedTheme) {
         installedTheme = themes.find(theme => theme.name.toLowerCase() === selectedTheme!.name.toLowerCase());
         onInstall = async () => {
+            setInstalling(true);
             const data = await api.themes.install(selectedTheme.ref);
+            setInstalling(false);
+
             const newlyInstalledTheme = data.themes[0];
             setThemes([
                 ...themes.map(theme => ({...theme, active: false})),
                 newlyInstalledTheme
             ]);
-            showToast({
-                message: `Theme installed - ${newlyInstalledTheme.name}`
+
+            let title = 'Success';
+            let prompt = <>
+                <strong>{newlyInstalledTheme.name}</strong> has been successfully installed.
+            </>;
+
+            if (!newlyInstalledTheme.active) {
+                prompt = <>
+                    {prompt}{' '}
+                    Do you want to activate it now?
+                </>;
+            }
+
+            if (newlyInstalledTheme.errors?.length || newlyInstalledTheme.warnings?.length) {
+                const hasErrors = newlyInstalledTheme.errors?.length;
+
+                title = `Installed with ${hasErrors ? 'errors' : 'warnings'}`;
+                prompt = <>
+                    The theme <strong>"{newlyInstalledTheme.name}"</strong> was installed successfully but we detected some {hasErrors ? 'errors' : 'warnings'}.
+                </>;
+
+                if (!newlyInstalledTheme.active) {
+                    prompt = <>
+                        {prompt}
+                        You are still able to activate and use the theme but it is recommended to contact the theme developer fix these {hasErrors ? 'errors' : 'warnings'} before you do so.
+                    </>;
+                }
+            }
+
+            NiceModal.show(ThemeInstalledModal, {
+                title,
+                prompt,
+                installedTheme: newlyInstalledTheme,
+                setThemes
             });
-            setCurrentTab('installed');
         };
     }
 
@@ -217,11 +263,10 @@ const ChangeThemeModal = NiceModal.create(() => {
                 <div className='grow'>
                     {selectedTheme &&
                         <ThemePreview
-                            installButtonLabel={
-                                installedTheme?.active ? 'Activated' : (installedTheme ? 'Installed' : `Install ${selectedTheme?.name}`)
-                            }
+                            installButtonLabel={installedTheme ? `Update ${selectedTheme?.name}` : `Install ${selectedTheme?.name}`}
+                            installedTheme={installedTheme}
+                            isInstalling={isInstalling}
                             selectedTheme={selectedTheme}
-                            themeInstalled={Boolean(installedTheme)}
                             onBack={() => {
                                 setSelectedTheme(null);
                             }}

diff --git a/...nts/settings/site/ThemeInstalledModal.tsx → ...ttings/site/theme/ThemeInstalledModal.tsx b/...nts/settings/site/ThemeInstalledModal.tsx → ...ttings/site/theme/ThemeInstalledModal.tsx
@@ -1,12 +1,13 @@
-import Button from '../../../admin-x-ds/global/Button';
-import Heading from '../../../admin-x-ds/global/Heading';
-import List from '../../../admin-x-ds/global/List';
-import ListItem from '../../../admin-x-ds/global/ListItem';
+import Button from '../../../../admin-x-ds/global/Button';
+import Heading from '../../../../admin-x-ds/global/Heading';
+import List from '../../../../admin-x-ds/global/List';
+import ListItem from '../../../../admin-x-ds/global/ListItem';
 import NiceModal from '@ebay/nice-modal-react';
 import React, {ReactNode, useState} from 'react';
-import {ConfirmationModalContent} from '../../../admin-x-ds/global/modal/ConfirmationModal';
-import {InstalledTheme, Theme, ThemeProblem} from '../../../types/api';
-import {useApi} from '../../providers/ServiceProvider';
+import {ConfirmationModalContent} from '../../../../admin-x-ds/global/modal/ConfirmationModal';
+import {InstalledTheme, Theme, ThemeProblem} from '../../../../types/api';
+import {showToast} from '../../../../admin-x-ds/global/Toast';
+import {useApi} from '../../../providers/ServiceProvider';
 
 const ThemeProblemView = ({problem}:{problem: ThemeProblem}) => {
     const [isExpanded, setExpanded] = useState(false);
@@ -59,33 +60,47 @@ const ThemeInstalledModal: React.FC<{
         </div>;
     }
 
+    let okLabel = `Activate${installedTheme.errors?.length ? ' with errors' : ''}`;
+
+    if (installedTheme.active) {
+        okLabel = 'OK';
+    }
+
     return <ConfirmationModalContent
         cancelLabel='Close'
         okColor='black'
-        okLabel={`Activate${installedTheme.errors?.length ? ' with errors' : ''}`}
+        okLabel={okLabel}
         okRunningLabel='Activating...'
         prompt={<>
             {prompt}
+
             {errorPrompt}
             {warningPrompt}
         </>}
         title={title}
         onOk={async (activateModal) => {
-            const resData = await api.themes.activate(installedTheme.name);
-            const updatedTheme = resData.themes[0];
+            if (!installedTheme.active) {
+                const resData = await api.themes.activate(installedTheme.name);
+                const updatedTheme = resData.themes[0];
+
+                setThemes((_themes) => {
+                    const updatedThemes: Theme[] = _themes.map((t) => {
+                        if (t.name === updatedTheme.name) {
+                            return updatedTheme;
+                        }
+                        return {
+                            ...t,
+                            active: false
+                        };
+                    });
+                    return updatedThemes;
+                });
 
-            setThemes((_themes) => {
-                const updatedThemes: Theme[] = _themes.map((t) => {
-                    if (t.name === updatedTheme.name) {
-                        return updatedTheme;
-                    }
-                    return {
-                        ...t,
-                        active: false
-                    };
+                showToast({
+                    type: 'success',
+                    message: `${updatedTheme.name} is now your active theme.`
                 });
-                return updatedThemes;
-            });
+            }
             activateModal?.remove();
         }}
     />;

diff --git a/apps/admin-x-settings/src/components/settings/site/theme/ThemePreview.tsx b/apps/admin-x-settings/src/components/settings/site/theme/ThemePreview.tsx
@@ -1,21 +1,26 @@
 import Breadcrumbs from '../../../../admin-x-ds/global/Breadcrumbs';
 import Button from '../../../../admin-x-ds/global/Button';
 import ButtonGroup from '../../../../admin-x-ds/global/ButtonGroup';
+import ConfirmationModal from '../../../../admin-x-ds/global/modal/ConfirmationModal';
 import MobileChrome from '../../../../admin-x-ds/global/chrome/MobileChrome';
+import NiceModal from '@ebay/nice-modal-react';
 import PageHeader from '../../../../admin-x-ds/global/layout/PageHeader';
 import React, {useState} from 'react';
 import {OfficialTheme} from '../../../../models/themes';
+import {Theme} from '../../../../types/api';
 
 const ThemePreview: React.FC<{
     selectedTheme?: OfficialTheme;
     onBack: () => void;
-    themeInstalled?: boolean;
+    isInstalling?: boolean;
+    installedTheme?: Theme;
     installButtonLabel?: string;
-    onInstall?: () => void;
+    onInstall?: () => void | Promise<void>;
 }> = ({
     selectedTheme,
     onBack,
-    themeInstalled,
+    isInstalling,
+    installedTheme,
     installButtonLabel,
     onInstall
 }) => {
@@ -25,6 +30,29 @@ const ThemePreview: React.FC<{
         return null;
     }
 
+    const handleInstall = () => {
+        if (installedTheme) {
+            NiceModal.show(ConfirmationModal, {
+                title: 'Overwrite theme',
+                prompt: (
+                    <>
+                        This will overwrite your existing version of {selectedTheme.name}{installedTheme?.active ? ', which is your active theme' : ''}. All custom changes will be lost.
+                    </>
+                ),
+                okLabel: 'Overwrite',
+                okRunningLabel: 'Installing...',
+                cancelLabel: 'Cancel',
+                okColor: 'red',
+                onOk: async (confirmModal) => {
+                    await onInstall?.();
+                    confirmModal?.remove();
+                }
+            });
+        } else {
+            onInstall?.();
+        }
+    };
+
     const left =
         <div className='flex items-center gap-2'>
             <Breadcrumbs
@@ -63,9 +91,9 @@ const ThemePreview: React.FC<{
             />
             <Button
                 color='green'
-                disabled={themeInstalled}
-                label={installButtonLabel}
-                onClick={onInstall}
+                disabled={isInstalling}
+                label={isInstalling ? 'Installing...' : installButtonLabel}
+                onClick={handleInstall}
             />
         </div>;
 
@@ -87,4 +115,4 @@ const ThemePreview: React.FC<{
     );
 };
 
-export default ThemePreview;
+export default ThemePreview;
diff --git a/apps/admin-x-settings/test/e2e/site/theme.test.ts b/apps/admin-x-settings/test/e2e/site/theme.test.ts
@@ -12,6 +12,14 @@ test.describe('Theme settings', async () => {
                         active: false,
                         templates: []
                     }]
+                },
+                activate: {
+                    themes: [{
+                        name: 'headline',
+                        package: {},
+                        active: true,
+                        templates: []
+                    }]
                 }
             }
         }});
@@ -28,33 +36,23 @@ test.describe('Theme settings', async () => {
 
         await modal.getByRole('button', {name: /Casper/}).click();
 
-        await expect(modal.getByRole('button', {name: 'Installed'})).toBeVisible();
-        await expect(modal.getByRole('button', {name: 'Installed'})).toBeDisabled();
+        await expect(modal.getByRole('button', {name: 'Update Casper'})).toBeVisible();
 
         await expect(page.locator('iframe[title="Theme preview"]')).toHaveAttribute('src', 'https://demo.ghost.io/');
 
         await modal.getByRole('button', {name: 'Official themes'}).click();
 
-        // The "edition" theme is activated in fixtures
-
-        await modal.getByRole('button', {name: /Edition/}).click();
-
-        await expect(modal.getByRole('button', {name: 'Activated'})).toBeVisible();
-        await expect(modal.getByRole('button', {name: 'Activated'})).toBeDisabled();
-
-        await expect(page.locator('iframe[title="Theme preview"]')).toHaveAttribute('src', 'https://edition.ghost.io/');
-
-        await modal.getByRole('button', {name: 'Official themes'}).click();
-
         // Try installing another theme
 
         await modal.getByRole('button', {name: /Headline/}).click();
 
         await modal.getByRole('button', {name: 'Install Headline'}).click();
 
-        await expect(modal.getByRole('button', {name: 'Installed'})).toBeVisible();
-        await expect(modal.getByRole('button', {name: 'Installed'})).toBeDisabled();
-        await expect(page.getByTestId('toast')).toHaveText(/Theme installed - headline/);
+        await expect(page.getByTestId('confirmation-modal')).toHaveText(/successfully installed/);
+
+        await page.getByRole('button', {name: 'Activate'}).click();
+
+        await expect(page.getByTestId('toast')).toHaveText(/headline is now your active theme/);
 
         expect(lastApiRequests.themes.install.url).toMatch(/\?source=github&ref=TryGhost%2FHeadline/);
     });