Skip to content
Closed
Show file tree
Hide file tree
Changes from 3 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
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: changed

Connection UI: Migrate the disconnect, manage-connection and owner-disconnect modals from the @wordpress/components Modal to the @wordpress/ui Dialog.
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import jetpackAnalytics from '@automattic/jetpack-analytics';
import restApi from '@automattic/jetpack-api';
import { jetpackConfigHas, jetpackConfigGet } from '@automattic/jetpack-config';
import { Modal } from '@wordpress/components';
import { __ } from '@wordpress/i18n';
import { useMemo, useEffect, useCallback, useState } from 'react';
import useRestApiInit from '../../hooks/use-rest-api-init';
import ConnectionDialog from '../shared/connection-dialog';
import './style.scss';
import StepDisconnect from './steps/step-disconnect';
import StepDisconnectConfirm from './steps/step-disconnect-confirm';
Expand Down Expand Up @@ -270,7 +270,6 @@ const DisconnectDialog = ( {
const handleDisconnect = useCallback(
( e?: MouseEvent< HTMLElement > ) => {
e && e.preventDefault();

setDisconnectError( false );
setIsDisconnecting( true );

Expand Down Expand Up @@ -425,27 +424,17 @@ const DisconnectDialog = ( {
};

return (
<>
{ isOpen && (
<Modal
title=""
contentLabel={ title }
aria={ {
labelledby: 'jp-connection__disconnect-dialog__heading',
} }
onRequestClose={ onClose }
shouldCloseOnClickOutside={ false }
shouldCloseOnEsc={ false }
isDismissible={ false }
className={
'jp-connection__disconnect-dialog' +
( isDisconnected ? ' jp-connection__disconnect-dialog__success' : '' )
}
>
{ getCurrentStep() }
</Modal>
) }
</>
<ConnectionDialog
isOpen={ isOpen }
onClose={ onClose }
hasOwnTitle
className={
'jp-connection__disconnect-dialog' +
( isDisconnected ? ' jp-connection__disconnect-dialog__success' : '' )
}
>
{ getCurrentStep() }
</ConnectionDialog>
);
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { DecorativeCard } from '@automattic/jetpack-components';
import { createInterpolateElement } from '@wordpress/element';
import { __ } from '@wordpress/i18n';
import { Button } from '@wordpress/ui';
import { ConnectionDialogTitle } from '../../shared/connection-dialog';
import disconnectImage from '../images/disconnect-confirm.jpg';
import type { MouseEvent } from 'react';

Expand Down Expand Up @@ -31,14 +32,14 @@ const StepDisconnectConfirm = ( {
<DecorativeCard icon="unlink" imageUrl={ disconnectImage } />

<div className="jp-connection__disconnect-dialog__step-copy jp-connection__disconnect-dialog__step-copy--narrow">
<h1>
<ConnectionDialogTitle>
{ createInterpolateElement(
__( 'Jetpack has been <br/>successfully disconnected.', 'jetpack-connection-js' ),
{
br: <br />,
}
) }
</h1>
</ConnectionDialogTitle>

{ canProvideFeedback && (
<>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { __ } from '@wordpress/i18n';
import { Fragment, useCallback, useEffect } from 'react';
import ConnectedPlugins from '../../connected-plugins';
import { ConnectionDialogTitle } from '../../shared/connection-dialog';
import DisconnectActionFooter from '../../shared/disconnect-action-footer';
import type { StepDisconnectProps } from './types';
import type { MouseEvent } from 'react';
Expand Down Expand Up @@ -120,7 +121,7 @@ const StepDisconnect = ( {
return (
<Fragment>
<div className="jp-connection__disconnect-dialog__content">
<h1 id="jp-connection__disconnect-dialog__heading">{ title }</h1>
<ConnectionDialogTitle>{ title }</ConnectionDialogTitle>
<ConnectedPlugins
connectedPlugins={ connectedPlugins }
disconnectingPlugin={ disconnectingPlugin }
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { __ } from '@wordpress/i18n';
import '../../disconnect-survey/_jp-connect_disconnect-survey-card.scss';
import DisconnectSurvey from '../../disconnect-survey';
import { ConnectionDialogTitle } from '../../shared/connection-dialog';
import type { MouseEvent } from 'react';

interface StepSurveyProps {
Expand All @@ -21,9 +22,11 @@ interface StepSurveyProps {
const StepSurvey = ( { onExit, onFeedBackProvided, isSubmittingFeedback }: StepSurveyProps ) => {
return (
<div className="jp-connection__disconnect-dialog__content">
<h1>{ __( 'Before you go, help us improve Jetpack', 'jetpack-connection-js' ) }</h1>
<ConnectionDialogTitle>
{ __( 'Before you go, help us improve Jetpack', 'jetpack-connection-js' ) }
</ConnectionDialogTitle>
<p className="jp-connection__disconnect-dialog__large-text">
{ __( 'Let us know what didnt work for you', 'jetpack-connection-js' ) }
{ __( "Let us know what didn't work for you", 'jetpack-connection-js' ) }
</p>
<DisconnectSurvey
onSubmit={ onFeedBackProvided }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { DecorativeCard } from '@automattic/jetpack-components';
import { createInterpolateElement } from '@wordpress/element';
import { __ } from '@wordpress/i18n';
import { Button, Stack } from '@wordpress/ui';
import { ConnectionDialogTitle } from '../../shared/connection-dialog';
import disconnectImage from '../images/disconnect-thanks.jpg';
import type { MouseEvent } from 'react';

Expand All @@ -27,7 +28,9 @@ const StepThankYou = ( { onExit }: StepThankYouProps ) => {
align="center"
gap="md"
>
<h1>{ __( 'Thank you!', 'jetpack-connection-js' ) }</h1>
<ConnectionDialogTitle>
{ __( 'Thank you!', 'jetpack-connection-js' ) }
</ConnectionDialogTitle>
<p className="jp-connection__disconnect-dialog__large-text">
{ createInterpolateElement(
__(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,29 +1,55 @@
@use "@automattic/jetpack-base-styles/style";

.jp-connection__disconnect-dialog {
--viewport-inset: 3rem;

display: flex;
flex-direction: column;
padding: 0;
overflow-y: auto;

h1 {
margin-top: 0;
--_gcd-heading-font-size: var(--font-title-small);
--_gcd-heading-font-weight: 600;
--_gcd-heading-margin: 0 0 0.67em;
Comment on lines +12 to +14

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

These attributes were meant to protect any CSS reset or other h1 styles from leaking to the component; not as much as a way to customize the component.

Are the style overrieds (here and elsewhere in this file) to the core component really needed? Ideally we would just use the components as-is. Worth checking with Jetpack design team if that would work!

Long term these styles become pain to maintain since they're assuming the internal DOM structure doesn't change, and there aren't any other conflicting styles introduced in component later on.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm looking into creating a lint rule to avoid using the internal gcd tokens: WordPress/gutenberg#80952


margin: 0 0 0.67em;
line-height: 1.2;
font-size: var(--font-title-small);
font-weight: 600;
}

h2 {
--_gcd-heading-font-size: var(--font-title-small);
--_gcd-heading-font-weight: 400;

margin: 0;
line-height: 1.2;
font-size: var(--font-title-small);
font-weight: 400;
}

p {
--_gcd-p-font-size: var(--font-body);

margin-top: 0;
font-size: var(--font-body);
}

// `Text`'s own stylesheet sets `margin: 0` (layered), which for a `<p>`
// falls back to that default anyway, but for a bare `Text` (renders as a
// `<span>`, e.g. `OwnerDisconnectDialog`'s copy) leaves no gap before the
// next element, since a `<span>` has no UA margin of its own to restore.
// Same story for `line-height`: wp-admin's global `common.css` sets a bare
// `p { line-height: 1.5 }` that the raw `<p>` steps pick up for free, but a
// `<span>` never matches that selector, so it's restored explicitly here.
&__large-text,
p.jp-connection__disconnect-dialog__large-text {
--_gcd-p-font-size: 1.25rem;

margin: 0 0 1em;
font-size: 1.25rem;
line-height: 1.5;
}

&__link,
Expand Down Expand Up @@ -54,25 +80,6 @@
border-radius: 4px;
}

.components-modal {

&__content {
padding: 0;
display: flex;
flex-direction: column;
flex-grow: 1;
margin: 0;

&::before {
display: none;
}
}

&__header {
display: none;
}
}

// The help message grows to fill the row; the button pair keeps its natural
// width beside it and wraps below once the row runs out of room. The
// flex-basis biases toward wrapping early, while `min-width: 0` lets the
Expand Down Expand Up @@ -115,6 +122,23 @@
background: var(--jp-red) !important;
}

// Moved here from the owner dialog's stylesheet, which no longer renders
// these buttons — `DisconnectActionFooter` supplies its own footer actions.
// The @wordpress/ui Button emits no `.components-button` class, so the
// shared reset that wp-admin would otherwise apply is restored on the
// class itself.
Comment thread
coder-karen marked this conversation as resolved.
&__btn-dismiss,
&__btn-disconnect {
border: 0;
border-radius: 4px;
color: var(--wp-components-color-accent-inverted, #fff);
font-size: var(--font-body-small);
height: 40px;
text-decoration: none;
text-shadow: none;
white-space: nowrap;
}
Comment on lines +127 to +142

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Similar to above question, could we simply use the button component as-is without overriding its styles entirely? Otherwise, there's little point in using a shared component in the first place.


&__btn-back-to-wp {
background: var(--jp-black) !important;
}
Expand Down Expand Up @@ -181,8 +205,7 @@

@media (min-width: 600px) {

.jp-connection__disconnect-dialog,
.jp-connection__disconnect-dialog.components-modal__frame {
.jp-connection__disconnect-dialog {
width: 100%;
max-width: calc(100% - 32px);
}
Expand All @@ -202,8 +225,7 @@

@media (min-width: 960px) {

.jp-connection__disconnect-dialog,
.jp-connection__disconnect-dialog.components-modal__frame {
.jp-connection__disconnect-dialog {
width: 1200px;
height: 900px;
display: flex;
Expand All @@ -213,11 +235,15 @@
.jp-connection__disconnect-dialog {

h1 {
--_gcd-heading-font-size: var(--font-title-large);

font-size: var(--font-title-large);
}

&__large-text,
p.jp-connection__disconnect-dialog__large-text {
--_gcd-p-font-size: 1.5rem;

font-size: 1.5rem;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ describe( 'DisconnectDialog', () => {
} );

describe( 'Initially', () => {
it( 'renders the Modal', () => {
it( 'renders the Dialog', () => {
render( <DisconnectDialog { ...testProps } /> );
expect(
screen.getByRole( 'dialog', { name: 'Are you sure you want to disconnect?' } )
Expand All @@ -47,10 +47,12 @@ describe( 'DisconnectDialog', () => {

it( 'renders the "StepDisconnect" step', () => {
render( <DisconnectDialog { ...testProps } /> );
// Each step renders its heading as the dialog's `Dialog.Title`, so the
// accessible name tracks the current step's own visible <h1>.
expect(
within(
screen.getByRole( 'dialog', { name: 'Are you sure you want to disconnect?' } )
).getByRole( 'heading' )
).getByRole( 'heading', { level: 1 } )
).toHaveTextContent( 'Are you sure you want to disconnect?' );
} );
} );
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,17 @@
import { jest } from '@jest/globals';
import { render, screen } from '@testing-library/react';
import { render as rtlRender, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { Dialog } from '@wordpress/ui';
import StepDisconnectConfirm from '../steps/step-disconnect-confirm';
import type { ReactElement, ReactNode } from 'react';

// The step renders its heading as a `Dialog.Title`, which needs a Dialog
// context. In the app that's always `ConnectionDialog`; in isolation, supply a
// minimal one so the heading can register.
const DialogWrapper = ( { children }: { children: ReactNode } ) => (
<Dialog.Root open>{ children }</Dialog.Root>
);
const render = ( ui: ReactElement ) => rtlRender( ui, { wrapper: DialogWrapper } );

describe( 'StepDisconnectConfirm', () => {
const testProps = {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,17 @@
import { jest } from '@jest/globals';
import { render, screen } from '@testing-library/react';
import { render as rtlRender, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { Dialog } from '@wordpress/ui';
import StepDisconnect from '../steps/step-disconnect';
import type { ReactElement, ReactNode } from 'react';

// The step renders its heading as a `Dialog.Title`, which needs a Dialog
// context. In the app that's always `ConnectionDialog`; in isolation, supply a
// minimal one so the heading can register.
const DialogWrapper = ( { children }: { children: ReactNode } ) => (
<Dialog.Root open>{ children }</Dialog.Root>
);
const render = ( ui: ReactElement ) => rtlRender( ui, { wrapper: DialogWrapper } );

describe( 'StepDisconnect', () => {
const testProps = {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,17 @@
import { jest } from '@jest/globals';
import { render, screen } from '@testing-library/react';
import { render as rtlRender, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { Dialog } from '@wordpress/ui';
import StepThankYou from '../steps/step-thank-you';
import type { ReactElement, ReactNode } from 'react';

// The step renders its heading as a `Dialog.Title`, which needs a Dialog
// context. In the app that's always `ConnectionDialog`; in isolation, supply a
// minimal one so the heading can register.
const DialogWrapper = ( { children }: { children: ReactNode } ) => (
<Dialog.Root open>{ children }</Dialog.Root>
);
const render = ( ui: ReactElement ) => rtlRender( ui, { wrapper: DialogWrapper } );

describe( 'StepThankYou', () => {
const testProps = {
Expand Down
Loading
Loading