Skip to content

Commit

Permalink
Fix/connection status inconsistencies (#40632)
Browse files Browse the repository at this point in the history
* Update connection status card to align with connection banner

* unify the way we connect sites on My Jetpack

* Unify user connection flows in My Jetpack

* changelog

* Fix tests

* Add comment

* Scroll to top all the time when connecting site

* Add slight delay to scroll to top
  • Loading branch information
CodeyGuyDylan authored Dec 19, 2024
1 parent d2add81 commit bfbeaac
Show file tree
Hide file tree
Showing 21 changed files with 305 additions and 189 deletions.
88 changes: 36 additions & 52 deletions projects/packages/my-jetpack/_inc/admin.jsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
/**
* External dependencies
*/
import { ThemeProvider } from '@automattic/jetpack-components';
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
import { createRoot } from '@wordpress/element';
import { useEffect } from 'react';
import { HashRouter, Navigate, Routes, Route, useLocation } from 'react-router-dom';
Expand Down Expand Up @@ -32,10 +30,9 @@ import {
import JetpackAiProductPage from './components/product-interstitial/jetpack-ai/product-page';
import RedeemTokenScreen from './components/redeem-token-screen';
import { MyJetpackRoutes } from './constants';
import NoticeContextProvider from './context/notices/noticeContext';
import ValueStoreContextProvider from './context/value-store/valueStoreContext';
import { getMyJetpackWindowInitialState } from './data/utils/get-my-jetpack-window-state';
import './style.module.scss';
import Providers from './providers';

/**
* Component to scroll window to top on route change.
Expand All @@ -50,57 +47,44 @@ function ScrollToTop() {
}

const MyJetpack = () => {
const queryClient = new QueryClient();
const { loadAddLicenseScreen } = getMyJetpackWindowInitialState();

return (
<ThemeProvider>
<NoticeContextProvider>
<ValueStoreContextProvider>
<QueryClientProvider client={ queryClient }>
<HashRouter>
<ScrollToTop />
<Routes>
<Route path={ MyJetpackRoutes.Home } element={ <MyJetpackScreen /> } />
<Route path={ MyJetpackRoutes.Connection } element={ <ConnectionScreen /> } />
<Route path={ MyJetpackRoutes.AddAkismet } element={ <AntiSpamInterstitial /> } />
{ /* Redirect the old route for Anti Spam */ }
<Route
path={ MyJetpackRoutes.AddAntiSpam }
element={ <Navigate replace to={ MyJetpackRoutes.AddAkismet } /> }
/>
<Route path={ MyJetpackRoutes.AddBackup } element={ <BackupInterstitial /> } />
<Route path={ MyJetpackRoutes.AddBoost } element={ <BoostInterstitial /> } />
<Route path={ MyJetpackRoutes.AddCRM } element={ <CRMInterstitial /> } />
<Route
path={ MyJetpackRoutes.AddJetpackAI }
element={ <JetpackAiInterstitial /> }
/>
<Route path={ MyJetpackRoutes.AddExtras } element={ <ExtrasInterstitial /> } />
<Route path={ MyJetpackRoutes.AddProtect } element={ <ProtectInterstitial /> } />
<Route path={ MyJetpackRoutes.AddScan } element={ <ScanInterstitial /> } />
<Route path={ MyJetpackRoutes.AddSocial } element={ <SocialInterstitial /> } />
<Route path={ MyJetpackRoutes.AddSearch } element={ <SearchInterstitial /> } />
<Route
path={ MyJetpackRoutes.AddVideoPress }
element={ <VideoPressInterstitial /> }
/>
<Route path={ MyJetpackRoutes.AddStats } element={ <StatsInterstitial /> } />
{ loadAddLicenseScreen && (
<Route path={ MyJetpackRoutes.AddLicense } element={ <AddLicenseScreen /> } />
) }
<Route path={ MyJetpackRoutes.RedeemToken } element={ <RedeemTokenScreen /> } />
<Route path={ MyJetpackRoutes.RedeemToken } element={ <RedeemTokenScreen /> } />
<Route path={ MyJetpackRoutes.JetpackAi } element={ <JetpackAiProductPage /> } />
<Route path={ MyJetpackRoutes.AddSecurity } element={ <SecurityInterstitial /> } />
<Route path={ MyJetpackRoutes.AddGrowth } element={ <GrowthInterstitial /> } />
<Route path={ MyJetpackRoutes.AddComplete } element={ <CompleteInterstitial /> } />
</Routes>
</HashRouter>
</QueryClientProvider>
</ValueStoreContextProvider>
</NoticeContextProvider>
</ThemeProvider>
<Providers>
<HashRouter>
<ScrollToTop />
<Routes>
<Route path={ MyJetpackRoutes.Home } element={ <MyJetpackScreen /> } />
<Route path={ MyJetpackRoutes.Connection } element={ <ConnectionScreen /> } />
<Route path={ MyJetpackRoutes.AddAkismet } element={ <AntiSpamInterstitial /> } />
{ /* Redirect the old route for Anti Spam */ }
<Route
path={ MyJetpackRoutes.AddAntiSpam }
element={ <Navigate replace to={ MyJetpackRoutes.AddAkismet } /> }
/>
<Route path={ MyJetpackRoutes.AddBackup } element={ <BackupInterstitial /> } />
<Route path={ MyJetpackRoutes.AddBoost } element={ <BoostInterstitial /> } />
<Route path={ MyJetpackRoutes.AddCRM } element={ <CRMInterstitial /> } />
<Route path={ MyJetpackRoutes.AddJetpackAI } element={ <JetpackAiInterstitial /> } />
<Route path={ MyJetpackRoutes.AddExtras } element={ <ExtrasInterstitial /> } />
<Route path={ MyJetpackRoutes.AddProtect } element={ <ProtectInterstitial /> } />
<Route path={ MyJetpackRoutes.AddScan } element={ <ScanInterstitial /> } />
<Route path={ MyJetpackRoutes.AddSocial } element={ <SocialInterstitial /> } />
<Route path={ MyJetpackRoutes.AddSearch } element={ <SearchInterstitial /> } />
<Route path={ MyJetpackRoutes.AddVideoPress } element={ <VideoPressInterstitial /> } />
<Route path={ MyJetpackRoutes.AddStats } element={ <StatsInterstitial /> } />
{ loadAddLicenseScreen && (
<Route path={ MyJetpackRoutes.AddLicense } element={ <AddLicenseScreen /> } />
) }
<Route path={ MyJetpackRoutes.RedeemToken } element={ <RedeemTokenScreen /> } />
<Route path={ MyJetpackRoutes.RedeemToken } element={ <RedeemTokenScreen /> } />
<Route path={ MyJetpackRoutes.JetpackAi } element={ <JetpackAiProductPage /> } />
<Route path={ MyJetpackRoutes.AddSecurity } element={ <SecurityInterstitial /> } />
<Route path={ MyJetpackRoutes.AddGrowth } element={ <GrowthInterstitial /> } />
<Route path={ MyJetpackRoutes.AddComplete } element={ <CompleteInterstitial /> } />
</Routes>
</HashRouter>
</Providers>
);
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ const ConnectedProductCard: FC< ConnectedProductCardProps > = ( {
manageUrl,
} = detail;

const navigateToConnectionPage = useMyJetpackNavigate( MyJetpackRoutes.Connection );
const navigateToConnectionPage = useMyJetpackNavigate( MyJetpackRoutes.ConnectionSkipPricing );

/*
* Redirect only if connected
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Container, Col, AdminPage } from '@automattic/jetpack-components';
import { __ } from '@wordpress/i18n';
import { useSearchParams } from 'react-router-dom';
import useMyJetpackConnection from '../../hooks/use-my-jetpack-connection';
import useMyJetpackReturnToPage from '../../hooks/use-my-jetpack-return-to-page';
import CloseLink from '../close-link';
Expand All @@ -9,6 +10,8 @@ import styles from './styles.module.scss';
import type { FC } from 'react';

const ConnectionScreen: FC = () => {
const [ searchParams ] = useSearchParams();
const shouldSkipPricing = searchParams.get( 'skip_pricing' ) === 'true';
const returnToPage = useMyJetpackReturnToPage();
const { apiRoot, apiNonce, registrationNonce } = useMyJetpackConnection();

Expand All @@ -28,6 +31,7 @@ const ConnectionScreen: FC = () => {
apiRoot={ apiRoot }
apiNonce={ apiNonce }
registrationNonce={ registrationNonce }
skipPricingPage={ shouldSkipPricing }
footer={ <ConnectionScreenFooter /> }
/>
</Col>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { useAllProducts } from '../../data/products/use-product';
import { getMyJetpackWindowInitialState } from '../../data/utils/get-my-jetpack-window-state';
import getProductSlugsThatRequireUserConnection from '../../data/utils/get-product-slugs-that-require-user-connection';
import useAnalytics from '../../hooks/use-analytics';
import useConnectSite from '../../hooks/use-connect-site';
import useMyJetpackConnection from '../../hooks/use-my-jetpack-connection';
import cloud from './cloud.svg';
import emptyAvatar from './empty-avatar.svg';
Expand All @@ -33,6 +34,11 @@ const ConnectionListItem: ConnectionListItemType = ( {
let icon = check;
let statusStyles = '';

if ( status === 'info' ) {
icon = null;
statusStyles = '';
}

if ( status === 'success' ) {
icon = check;
statusStyles = styles.success;
Expand All @@ -50,13 +56,13 @@ const ConnectionListItem: ConnectionListItemType = ( {

if ( status === 'unlock' ) {
icon = lockOutline;
statusStyles = styles.unlock;
statusStyles = '';
}

return (
<div className={ styles[ 'list-item' ] }>
<Text className={ clsx( styles[ 'list-item-text' ], statusStyles ) }>
<Icon icon={ icon } />
{ icon && <Icon icon={ icon } /> }
{ text }
</Text>
{ actionText && status !== 'success' && (
Expand All @@ -77,9 +83,17 @@ const ConnectionItemButton: ConnectionItemButtonType = ( { actionText, onClick }
const getSiteConnectionLineData: getSiteConnectionLineDataType = ( {
isRegistered,
hasSiteConnectionBrokenModules,
handleConnectUser,
handleConnectSite,
siteIsRegistering,
openManageSiteConnectionDialog,
} ) => {
if ( siteIsRegistering ) {
return {
text: __( 'Connecting your site…', 'jetpack-my-jetpack' ),
status: 'info',
};
}

if ( isRegistered ) {
return {
onClick: openManageSiteConnectionDialog,
Expand All @@ -91,15 +105,15 @@ const getSiteConnectionLineData: getSiteConnectionLineDataType = ( {

if ( hasSiteConnectionBrokenModules ) {
return {
onClick: handleConnectUser,
onClick: handleConnectSite,
text: __( 'Missing site connection to enable some features.', 'jetpack-my-jetpack' ),
actionText: __( 'Connect', 'jetpack-my-jetpack' ),
status: 'error',
};
}

return {
onClick: handleConnectUser,
onClick: handleConnectSite,
text: __( 'Start with Jetpack.', 'jetpack-my-jetpack' ),
actionText: __( 'Connect your site with one click', 'jetpack-my-jetpack' ),
status: 'warning',
Expand Down Expand Up @@ -194,13 +208,16 @@ const ConnectionStatusCard: ConnectionStatusCardType = ( {
const { isRegistered, isUserConnected, userConnectionData } = useMyJetpackConnection( {
redirectUri,
} );

const { siteIsRegistering } = useMyJetpackConnection( {
skipUserConnection: true,
redirectUri,
} );
const { lifecycleStats } = getMyJetpackWindowInitialState();
const { recordEvent } = useAnalytics();
const [ isManageConnectionDialogOpen, setIsManageConnectionDialogOpen ] = useState( false );
const { setConnectionStatus, setUserIsConnecting } = useDispatch( CONNECTION_STORE_ID );
const connectUserFn = onConnectUser || setUserIsConnecting;
const avatar = userConnectionData.currentUser?.wpcomUser?.avatar;
const { lifecycleStats } = getMyJetpackWindowInitialState();
const { brokenModules } = lifecycleStats || {};
const products = useAllProducts();
const hasProductsThatRequireUserConnection =
Expand Down Expand Up @@ -272,6 +289,13 @@ const ConnectionStatusCard: ConnectionStatusCardType = ( {
[ connectUserFn, recordEvent, tracksEventData ]
);

const { connectSite: handleConnectSite } = useConnectSite( {
tracksInfo: {
event: 'jetpack_myjetpack_connection_connect_site',
properties: tracksEventData,
},
} );

const getConnectionLineStyles = () => {
if ( isRegistered ) {
return '';
Expand All @@ -283,7 +307,8 @@ const ConnectionStatusCard: ConnectionStatusCardType = ( {
const siteConnectionLineData = getSiteConnectionLineData( {
isRegistered,
hasSiteConnectionBrokenModules,
handleConnectUser,
handleConnectSite,
siteIsRegistering,
openManageSiteConnectionDialog,
} );

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import '@testing-library/jest-dom';
import { CONNECTION_STORE_ID } from '@automattic/jetpack-connection';
import { render, renderHook, screen } from '@testing-library/react';
import { useSelect } from '@wordpress/data';
import Providers from '../../../providers';
import ConnectionStatusCard from '../index';
import type { StateProducts, MyJetpackInitialState } from '../../../data/types';

Expand Down Expand Up @@ -58,7 +59,9 @@ const setConnectionStore = ( {
hasConnectedOwner = false,
} = {} ) => {
let storeSelect;
renderHook( () => useSelect( select => ( storeSelect = select( CONNECTION_STORE_ID ) ), [] ) );
renderHook( () => useSelect( select => ( storeSelect = select( CONNECTION_STORE_ID ) ), [] ), {
wrapper: Providers,
} );
jest
.spyOn( storeSelect, 'getConnectionStatus' )
.mockReset()
Expand All @@ -80,7 +83,11 @@ describe( 'ConnectionStatusCard', () => {

describe( 'When the site is not registered and has no broken modules', () => {
const setup = () => {
return render( <ConnectionStatusCard { ...testProps } /> );
return render(
<Providers>
<ConnectionStatusCard { ...testProps } />
</Providers>
);
};

it( 'renders the correct copy for the site connection line item', () => {
Expand All @@ -103,7 +110,11 @@ describe( 'ConnectionStatusCard', () => {
window.myJetpackInitialState.lifecycleStats.brokenModules.needs_site_connection = [
'anti-spam',
];
return render( <ConnectionStatusCard { ...testProps } /> );
return render(
<Providers>
<ConnectionStatusCard { ...testProps } />
</Providers>
);
};

it( 'renders the correct copy for the site connection line item', () => {
Expand All @@ -125,7 +136,11 @@ describe( 'ConnectionStatusCard', () => {
describe( 'There are no products that require user connection', () => {
const setup = () => {
setConnectionStore( { isRegistered: true } );
return render( <ConnectionStatusCard { ...testProps } /> );
return render(
<Providers>
<ConnectionStatusCard { ...testProps } />
</Providers>
);
};

it( 'renders the correct site connection line item', () => {
Expand All @@ -145,7 +160,11 @@ describe( 'ConnectionStatusCard', () => {
const setup = () => {
setConnectionStore( { isRegistered: true } );
window.myJetpackInitialState.products.items[ 'anti-spam' ].requires_user_connection = true;
return render( <ConnectionStatusCard { ...testProps } /> );
return render(
<Providers>
<ConnectionStatusCard { ...testProps } />
</Providers>
);
};

it( 'renders the correct site connection line item', () => {
Expand All @@ -168,7 +187,11 @@ describe( 'ConnectionStatusCard', () => {
window.myJetpackInitialState.lifecycleStats.brokenModules.needs_user_connection = [
'anti-spam',
];
return render( <ConnectionStatusCard { ...testProps } /> );
return render(
<Providers>
<ConnectionStatusCard { ...testProps } />
</Providers>
);
};

it( 'renders the correct site connection line item', () => {
Expand All @@ -189,7 +212,11 @@ describe( 'ConnectionStatusCard', () => {
describe( 'When the user has connected their WordPress.com account', () => {
const setup = () => {
setConnectionStore( { isRegistered: true, isUserConnected: true, hasConnectedOwner: true } );
return render( <ConnectionStatusCard { ...testProps } /> );
return render(
<Providers>
<ConnectionStatusCard { ...testProps } />
</Providers>
);
};

it( 'renders the correct site connection line item', () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type { FC, MouseEvent } from 'react';

type StatusType = 'warning' | 'error' | 'unlock' | 'success';
type StatusType = 'warning' | 'error' | 'unlock' | 'success' | 'info';

interface ConnectionListItemProps {
text: string;
Expand All @@ -19,7 +19,8 @@ export type ConnectionItemButtonType = FC< {
interface getSiteConnectionLineDataProps {
isRegistered: boolean;
hasSiteConnectionBrokenModules: boolean;
handleConnectUser: ( e: MouseEvent< HTMLButtonElement > ) => void;
siteIsRegistering: boolean;
handleConnectSite: ( e: MouseEvent< HTMLButtonElement > ) => void;
openManageSiteConnectionDialog: ( e: MouseEvent ) => void;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import ConnectionStatusCard from '../connection-status-card';
*/
export default function ConnectionsSection() {
const { apiRoot, apiNonce, topJetpackMenuItemUrl, connectedPlugins } = useMyJetpackConnection();
const navigate = useMyJetpackNavigate( MyJetpackRoutes.Connection );
const navigate = useMyJetpackNavigate( MyJetpackRoutes.ConnectionSkipPricing );
const products = useAllProducts();
const onDisconnected = () => document?.location?.reload( true ); // TODO: replace with a better experience.
const productsThatRequireUserConnection = getProductSlugsThatRequireUserConnection( products );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ const PlanSectionFooter: FC< PlanSectionHeaderAndFooterProps > = ( { numberOfPur
recordEvent( 'jetpack_myjetpack_plans_purchase_click' );
}, [ recordEvent ] );

const navigateToConnectionPage = useMyJetpackNavigate( MyJetpackRoutes.Connection );
const navigateToConnectionPage = useMyJetpackNavigate( MyJetpackRoutes.ConnectionSkipPricing );
const activateLicenseClickHandler = useCallback( () => {
recordEvent( 'jetpack_myjetpack_activate_license_click' );
if ( ! isUserConnected ) {
Expand Down
Loading

0 comments on commit bfbeaac

Please sign in to comment.