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

Revert "Change/at a glance unify connection ctas" #39584

Merged
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
11 changes: 10 additions & 1 deletion projects/plugins/jetpack/_inc/client/at-a-glance/connections.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ export class DashConnections extends Component {
<Gridicon icon="globe" size={ 64 } />
) }
<div className="jp-connection-settings__text">
{ __( 'Your site is connected to WordPress.com.', 'jetpack' ) }
{ this.props.isConnectionOwner && (
<span className="jp-connection-settings__is-owner">
{ __( 'You are the Jetpack owner.', 'jetpack' ) }
Expand Down Expand Up @@ -101,6 +102,7 @@ export class DashConnections extends Component {
const maybeShowLinkUnlinkBtn = this.props.isConnectionOwner ? null : (
<ConnectButton asLink connectUser={ true } from="connection-settings" />
);

let cardContent = '';

if ( this.props.isOfflineMode ) {
Expand Down Expand Up @@ -129,7 +131,14 @@ export class DashConnections extends Component {
}

if ( ! this.props.isLinked ) {
cardContent = <div className="jp-connection-settings__info">{ maybeShowLinkUnlinkBtn }</div>;
cardContent = (
<div>
<div className="jp-connection-settings__info">
{ __( 'Get the most out of Jetpack.', 'jetpack' ) }
</div>
<div className="jp-connection-settings__actions">{ maybeShowLinkUnlinkBtn }</div>
</div>
);
} else if ( this.props.isFetchingUserData ) {
cardContent = __( 'Loading…', 'jetpack' );
} else if ( ! this.props.wpComConnectedUser?.email ) {
Expand Down
34 changes: 15 additions & 19 deletions projects/plugins/jetpack/_inc/client/at-a-glance/monitor.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,9 @@ import { createInterpolateElement } from '@wordpress/element';
import { __ } from '@wordpress/i18n';
import Button from 'components/button';
import DashItem from 'components/dash-item';
import JetpackBanner from 'components/jetpack-banner';
import analytics from 'lib/analytics';
import PropTypes from 'prop-types';
import { Component } from 'react';
import React, { Component } from 'react';
import { connect } from 'react-redux';
import { isOfflineMode, hasConnectedOwner, connectUser } from 'state/connection';
import { isModuleAvailable } from 'state/modules';
Expand Down Expand Up @@ -79,29 +78,26 @@ class DashMonitor extends Component {
support={ support }
className="jp-dash-item__is-inactive"
noToggle={ ! this.props.hasConnectedOwner }
overrideContent={
( ! this.props.hasConnectedOwner && ! this.props.isOfflineMode && (
<JetpackBanner
title={ __(
'Connect your WordPress.com account to enable alerts if your site goes down.',
'jetpack'
) }
noIcon
callToAction={ __( 'Connect', 'jetpack' ) }
onClick={ this.props.connectUser }
eventFeature="monitor"
path="dashboard"
eventProps={ { type: 'connect' } }
/>
) ) ||
null
}
>
<p className="jp-dash-item__description">
{ this.props.isOfflineMode
? __( 'Unavailable in Offline Mode.', 'jetpack' )
: activateMessage }
</p>

{ ! this.props.isOfflineMode && ! this.props.hasConnectedOwner && (
<p className="jp-dash-item__description jp-dash-item__connect">
{ createInterpolateElement(
__(
'<Button>Connect your WordPress.com</Button> account to use this feature.',
'jetpack'
),
{
Button: <Button className="jp-link-button" onClick={ this.connect } />,
}
) }
</p>
) }
</DashItem>
);
}
Expand Down
34 changes: 0 additions & 34 deletions projects/plugins/jetpack/_inc/client/at-a-glance/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -717,37 +717,3 @@ a.jp-dash-item__manage-in-wpcom,
border-top: 1px solid $gray-5;
}
}

.jp-dash-item.jp-connection-type {
border: 1px solid transparent;
border-radius: 4px;

.jp-dash-item__card {
padding: 16px 16px 16px 24px;
}

.jp-dash-item__content {
display: block;
}

.jp-connection-settings__text {
flex-grow: 1;
}

.jp-dash-item__content,
.jp-connection-settings__text {
align-self: center;
}

.dops-banner {
padding: 0;

&__title {
padding: 0 0.5rem 0 0;
}
}

.dops-banner.dops-card {
display: block;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ describe( 'Connections', () => {
it( 'shows a disconnection link', () => {
render( <DashConnections { ...testProps } />, { initialState: buildInitialState() } );
expect(
withinCard( 'Site connection' ).getByRole( 'button', { name: 'Manage' } )
withinCard( 'Site connection' ).getByRole( 'button', { name: 'Manage site connection' } )
).toBeInTheDocument();
} );

Expand Down Expand Up @@ -128,8 +128,8 @@ describe( 'Connections', () => {
initialState: buildInitialState( { userIsLinked: false } ),
} );
expect(
withinCard( 'Account connection' ).getByRole( 'button', {
name: 'Connect',
withinCard( 'Account connection' ).getByRole( 'link', {
name: 'Connect your WordPress.com account',
} )
).toBeInTheDocument();
} );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ export class Banner extends Component {
}
return `/plans/${ siteSlug }`;
}

return href;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import { __ } from '@wordpress/i18n';
import { getFragment } from '@wordpress/url';
import Button from 'components/button';
import QuerySiteBenefits from 'components/data/query-site-benefits';
import JetpackBanner from 'components/jetpack-banner';
import analytics from 'lib/analytics';
import PropTypes from 'prop-types';
import React from 'react';
Expand Down Expand Up @@ -75,10 +74,8 @@ export class ConnectButton extends React.Component {
}

handleOpenModal = e => {
if ( e ) {
e.preventDefault();
}
analytics.tracks.recordJetpackClick( 'manage_site_connection' );
e.preventDefault();
this.toggleVisibility();
};

Expand All @@ -92,9 +89,7 @@ export class ConnectButton extends React.Component {
};

loadConnectionScreen = e => {
if ( e ) {
e.preventDefault();
}
e.preventDefault();
// If the iframe is already loaded or we don't have a connectUrl yet, return.
if ( this.props.isAuthorizing || this.props.fetchingConnectUrl ) {
return;
Expand Down Expand Up @@ -136,19 +131,27 @@ export class ConnectButton extends React.Component {
);
}

return (
<JetpackBanner
title={ __(
'Get the most out of Jetpack by connecting your WordPress.com account',
'jetpack'
) }
noIcon
callToAction={ __( 'Connect', 'jetpack' ) }
onClick={ this.loadConnectionScreen }
eventFeature="connect-account"
path="dashboard"
eventProps={ { type: 'connect' } }
/>
let connectUrl = this.props.connectUrl;
if ( this.props.from ) {
connectUrl += `&from=${ this.props.from }`;
connectUrl += '&additional-user';
}

const buttonProps = {
className: 'is-primary jp-jetpack-connect__button',
href: connectUrl,
disabled: this.props.fetchingConnectUrl || this.props.isAuthorizing,
onClick: this.loadConnectionScreen,
},
connectLegend =
this.props.connectLegend || __( 'Connect your WordPress.com account', 'jetpack' );

return this.props.asLink ? (
<a { ...buttonProps }>{ connectLegend }</a>
) : (
<Button rna={ this.props.rna } compact={ this.props.compact } { ...buttonProps }>
{ connectLegend }
</Button>
);
};

Expand All @@ -159,15 +162,15 @@ export class ConnectButton extends React.Component {

if ( this.props.isSiteConnected ) {
return (
<JetpackBanner
title={ __( 'Your site is connected to WordPress.com.', 'jetpack' ) }
noIcon
callToAction={ this.props.connectLegend || __( 'Manage', 'jetpack' ) }
<a
role="button"
tabIndex="0"
onKeyDown={ onKeyDownCallback( this.handleOpenModal ) }
onClick={ this.handleOpenModal }
eventFeature="manage-site-connection"
path="dashboard"
eventProps={ { type: 'manage' } }
/>
disabled={ this.props.isDisconnecting }
>
{ this.props.connectLegend || __( 'Manage site connection', 'jetpack' ) }
</a>
);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { jest } from '@jest/globals';
import userEvent from '@testing-library/user-event';
import React from 'react';
import { render, screen } from 'test/test-utils';
import { ConnectButton } from '../index';
import { buildInitialState } from './fixtures';

// Mock components that do fetches in the background. We supply needed state directly.
jest.mock( 'components/data/query-site-benefits', () => ( {
Expand All @@ -29,10 +29,26 @@ describe( 'ConnectButton', () => {

describe( 'Initially', () => {
it( 'renders a button to connect or link', () => {
render( <ConnectButton { ...testProps } fetchingConnectUrl={ true } />, {
initialState: buildInitialState(),
} );
expect( screen.getByRole( 'button', { name: 'Connect' } ) ).toBeInTheDocument();
render( <ConnectButton { ...testProps } fetchingConnectUrl={ true } /> );
expect(
screen.getByRole( 'link', { name: 'Connect your WordPress.com account' } )
).toBeInTheDocument();
} );

it( 'disables the button while fetching the connect URL', () => {
render( <ConnectButton { ...testProps } fetchingConnectUrl={ true } /> );
expect( screen.getByRole( 'link', { name: 'Connect your WordPress.com account' } ) )
// eslint-disable-next-line jest-dom/prefer-enabled-disabled -- `.toBeDisabled()` doesn't work on links.
.toHaveAttribute( 'disabled' );
} );
} );

describe( 'When it is used to link a user', () => {
it( 'has a link to jetpack.wordpress.com', () => {
render( <ConnectButton { ...testProps } /> );
expect(
screen.getByRole( 'link', { name: 'Connect your WordPress.com account' } )
).toHaveAttribute( 'href', 'https://jetpack.wordpress.com/jetpack.authorize/1/' );
} );
} );

Expand All @@ -46,15 +62,15 @@ describe( 'ConnectButton', () => {
};

it( 'has a link to jetpack.wordpress.com', () => {
render( <ConnectButton { ...currentTestProps } />, {
initialState: buildInitialState(),
} );
expect( screen.getByRole( 'button', { name: 'Connect' } ) ).toBeInTheDocument();
render( <ConnectButton { ...currentTestProps } /> );
expect(
screen.getByRole( 'link', { name: 'Link your account to WordPress.com' } )
).toHaveAttribute( 'href', 'https://jetpack.wordpress.com/jetpack.authorize/1/' );
} );

it( 'when clicked, loadConnectionScreen() is called once', async () => {
const user = userEvent.setup();
const loadConnectionScreen = jest.fn();
const loadConnectionScreen = jest.fn( e => e.preventDefault() );

class ConnectButtonMock extends ConnectButton {
constructor( props ) {
Expand All @@ -63,10 +79,10 @@ describe( 'ConnectButton', () => {
}
}

render( <ConnectButtonMock { ...currentTestProps } />, {
initialState: buildInitialState(),
} );
await user.click( screen.getByRole( 'button', { name: 'Connect' } ) );
render( <ConnectButtonMock { ...currentTestProps } /> );
await user.click(
screen.getByRole( 'link', { name: 'Link your account to WordPress.com' } )
);
expect( loadConnectionScreen ).toHaveBeenCalledTimes( 1 );
} );
} );
Expand All @@ -80,9 +96,7 @@ describe( 'ConnectButton', () => {
};

it( 'does not link to a URL', () => {
render( <ConnectButton { ...currentTestProps } />, {
initialState: buildInitialState(),
} );
render( <ConnectButton { ...currentTestProps } /> );
expect(
screen.getByRole( 'button', { name: 'Unlink your account from WordPress.com' } )
).not.toHaveAttribute( 'href' );
Expand Down Expand Up @@ -138,17 +152,15 @@ describe( 'ConnectButton', () => {
};

it( 'does not link to a URL', () => {
render( <ConnectButton { ...currentTestProps } />, {
initialState: buildInitialState(),
} );
render( <ConnectButton { ...currentTestProps } /> );
expect(
screen.getByRole( 'button', { name: 'Disconnect your site from WordPress.com' } )
).not.toHaveAttribute( 'href' );
} );

it( 'when clicked, handleOpenModal() is called once', async () => {
const user = userEvent.setup();
const handleOpenModal = jest.fn();
const handleOpenModal = jest.fn( e => e.preventDefault() );

class ConnectButtonMock extends ConnectButton {
constructor( props ) {
Expand All @@ -157,9 +169,7 @@ describe( 'ConnectButton', () => {
}
}

render( <ConnectButtonMock { ...currentTestProps } />, {
initialState: buildInitialState(),
} );
render( <ConnectButtonMock { ...currentTestProps } /> );
await user.click(
screen.getByRole( 'button', { name: 'Disconnect your site from WordPress.com' } )
);
Expand Down

This file was deleted.

Loading
Loading