Skip to content
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
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,15 @@ export const MainPageView = ({
flexDirection="column"
gap="spacingXl"
style={{ maxWidth: '900px', margin: `${tokens.spacingL} auto` }}>
<Heading marginBottom="none">Drive Integration</Heading>
<OAuthConnector
isOAuthConnected={isOAuthConnected}
isOAuthBusy={isOAuthBusy}
onConnect={onConnectGoogleDrive}
onDisconnect={onDisconnectGoogleDrive}
/>
<Flex justifyContent="space-between" alignItems="center">
<Heading marginBottom="none">Drive Integration</Heading>
<OAuthConnector
isOAuthConnected={isOAuthConnected}
isOAuthBusy={isOAuthBusy}
onConnect={onConnectGoogleDrive}
onDisconnect={onDisconnectGoogleDrive}
/>
</Flex>
<Card padding="large">
<Flex
flexDirection="column"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,23 @@ export const ConfirmCancelModal = ({
<Modal isShown={isOpen} onClose={onCancel} size="medium" shouldCloseOnOverlayClick={false}>
{() => (
<>
<Modal.Header title="You're about to lose your progress" onClose={onCancel} />
<Modal.Header title="Delete this job?" onClose={onCancel} />
<Modal.Content>
<Paragraph>No entries will be created and you&apos;ll need to start over.</Paragraph>
<Paragraph>
This will permanently delete the job. No entries will be created and you&apos;ll need
to start over.
</Paragraph>
</Modal.Content>
<Modal.Controls>
<Button onClick={onCancel} variant="secondary" isDisabled={isConfirming}>
Keep creating
Keep review open
</Button>
<Button
onClick={onConfirm}
variant="primary"
variant="negative"
isLoading={isConfirming}
isDisabled={isConfirming}>
Cancel without creating
Delete
</Button>
</Modal.Controls>
</>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,25 +61,3 @@ export const modeToggleButtonActive = css({
color: tokens.gray900,
},
});

export const cancelReviewButton = css({
'&, &&': {
backgroundColor: tokens.colorWhite,
border: `1px solid ${tokens.gray300}`,
borderRadius: tokens.borderRadiusMedium,
boxShadow: 'none',
color: tokens.gray900,
fontWeight: tokens.fontWeightDemiBold,
},

'&:hover, &&:hover': {
backgroundColor: tokens.gray100,
borderColor: tokens.gray400,
color: tokens.gray900,
},

'&:focus-visible, &&:focus-visible': {
boxShadow: `0 0 0 3px ${tokens.blue200}`,
outline: 'none',
},
});

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Even though this is a small UI change, it would be nice to include a screenshot or video showing how it looks. That way, we can make sure it's working correctly and that the change makes sense from a UX perspective.

Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { useCallback, useEffect, useMemo, useState } from 'react';
import { Button, Flex, Heading, Layout } from '@contentful/f36-components';
import { EyeIcon, PencilSimpleIcon } from '@contentful/f36-icons';
import { Button, Flex, Heading, IconButton, Layout, Menu } from '@contentful/f36-components';
import { DotsThreeIcon, EyeIcon, PencilSimpleIcon } from '@contentful/f36-icons';
import tokens from '@contentful/f36-tokens';
import { PageAppSDK } from '@contentful/app-sdk';
import { cx } from '@emotion/css';
Expand All @@ -22,7 +22,6 @@ import { SummaryModal } from '../modals/SummaryModal';
import OverviewSection from '../overview/OverviewSection';
import { MappingView } from './mapping/MappingView';
import {
cancelReviewButton,
modeToggleButton,
modeToggleButtonActive,
modeToggleWrapper,
Expand Down Expand Up @@ -193,14 +192,9 @@ export const ReviewPage = ({
void handleCreateEntries();
}, [hasCreatedEntries, handleCreateEntries]);

const handleCancelOrExitReview = useCallback(() => {
if (hasCreatedEntries) {
onExitReview();
return;
}

const handleDeleteJob = useCallback(() => {
setIsConfirmCancelModalOpen(true);
}, [hasCreatedEntries, onExitReview]);
}, []);

const handleSummaryDone = useCallback(() => {
setIsSummaryModalOpen(false);
Expand Down Expand Up @@ -230,30 +224,48 @@ export const ReviewPage = ({
<Heading marginBottom="none">{title}</Heading>
<Flex className={reviewHeaderActions}>
<div className={modeToggleWrapper} role="group" aria-label="Review mode">
<button
type="button"
<Button
variant="transparent"
size="small"
className={cx(modeToggleButton, reviewMode === 'view' && modeToggleButtonActive)}
onClick={() => handleReviewModeChange('view')}
aria-pressed={reviewMode === 'view'}>
<EyeIcon size="small" />
aria-pressed={reviewMode === 'view'}
startIcon={<EyeIcon />}>
View only
</button>
<button
type="button"
</Button>
<Button
variant="transparent"
size="small"
className={cx(modeToggleButton, reviewMode === 'edit' && modeToggleButtonActive)}
onClick={() => handleReviewModeChange('edit')}
aria-pressed={reviewMode === 'edit'}>
<PencilSimpleIcon size="small" />
aria-pressed={reviewMode === 'edit'}
startIcon={<PencilSimpleIcon />}>
Edit mode
</button>
</Button>
</div>
{!hasCreatedEntries && (
<Menu>
<Menu.Trigger>
<IconButton
variant="secondary"
size="small"
aria-label="More actions"
icon={<DotsThreeIcon />}
/>
</Menu.Trigger>
<Menu.List>
<Menu.Item onClick={handleDeleteJob} style={{ color: 'red' }}>
Delete
</Menu.Item>
</Menu.List>
</Menu>
)}
<Button
variant="secondary"
size="small"
className={cancelReviewButton}
onClick={handleCancelOrExitReview}
aria-label={hasCreatedEntries ? 'Exit review' : 'Cancel review'}>
{hasCreatedEntries ? 'Exit' : 'Cancel'}
onClick={onExitReview}
aria-label="Close review">
Close
</Button>
</Flex>
</Flex>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ export function RunsPage({
});

return (
<Box padding="spacingL" style={{ maxWidth: '1200px' }}>
<Box padding="spacingL" style={{ maxWidth: '1200px', margin: '0 auto' }}>
{/* Page header */}
<Flex justifyContent="space-between" alignItems="flex-start" marginBottom="spacingM">
<Box>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,9 +184,7 @@ describe('ModalOrchestrator', () => {
fireEvent.click(screen.getByRole('button', { name: 'Cancel' }));

await waitFor(() => {
expect(
screen.getByRole('heading', { name: "You're about to lose your progress" })
).toBeTruthy();
expect(screen.getByRole('heading', { name: 'Delete this job?' })).toBeTruthy();
});
});

Expand All @@ -205,18 +203,14 @@ describe('ModalOrchestrator', () => {

fireEvent.click(screen.getByRole('button', { name: 'Cancel' }));
await waitFor(() => {
expect(
screen.getByRole('heading', { name: "You're about to lose your progress" })
).toBeTruthy();
expect(screen.getByRole('heading', { name: 'Delete this job?' })).toBeTruthy();
});

fireEvent.click(screen.getByRole('button', { name: 'Cancel without creating' }));
fireEvent.click(screen.getByRole('button', { name: 'Delete' }));

await waitFor(() => {
expect(screen.queryByRole('heading', { name: 'Select content type(s)' })).toBeNull();
expect(
screen.queryByRole('heading', { name: "You're about to lose your progress" })
).toBeNull();
expect(screen.queryByRole('heading', { name: 'Delete this job?' })).toBeNull();
});
});

Expand Down Expand Up @@ -253,9 +247,7 @@ describe('ModalOrchestrator', () => {

await waitFor(() => {
expect(screen.queryByRole('button', { name: 'Pick document' })).toBeNull();
expect(
screen.queryByRole('heading', { name: "You're about to lose your progress" })
).toBeNull();
expect(screen.queryByRole('heading', { name: 'Delete this job?' })).toBeNull();
});
});

Expand All @@ -272,12 +264,10 @@ describe('ModalOrchestrator', () => {

fireEvent.click(screen.getByRole('button', { name: 'Cancel' }));
await waitFor(() => {
expect(
screen.getByRole('heading', { name: "You're about to lose your progress" })
).toBeTruthy();
expect(screen.getByRole('heading', { name: 'Delete this job?' })).toBeTruthy();
});

fireEvent.click(screen.getByRole('button', { name: 'Cancel without creating' }));
fireEvent.click(screen.getByRole('button', { name: 'Delete' }));

await waitFor(() => {
expect(mockAddRun).not.toHaveBeenCalled();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,39 +24,39 @@ describe('ConfirmCancelModal', () => {
render(<ConfirmCancelModal isOpen={true} onConfirm={onConfirm} onCancel={onCancel} />);

await waitFor(() => {
expect(screen.getByRole('heading', { name: 'Delete this job?' })).toBeTruthy();
expect(
screen.getByRole('heading', { name: "You're about to lose your progress" })
).toBeTruthy();
expect(
screen.getByText("No entries will be created and you'll need to start over.")
screen.getByText(
"This will permanently delete the job. No entries will be created and you'll need to start over."
)
).toBeTruthy();
});
});

it('renders Keep creating and Cancel without creating buttons', async () => {
it('renders Keep review open and Delete buttons', async () => {
render(<ConfirmCancelModal isOpen={true} onConfirm={onConfirm} onCancel={onCancel} />);

await waitFor(() => {
expect(screen.getByRole('button', { name: 'Keep creating' })).toBeTruthy();
expect(screen.getByRole('button', { name: 'Cancel without creating' })).toBeTruthy();
expect(screen.getByRole('button', { name: 'Keep review open' })).toBeTruthy();
expect(screen.getByRole('button', { name: 'Delete' })).toBeTruthy();
});
});

it('calls onConfirm when Cancel without creating is clicked', async () => {
it('calls onConfirm when Delete is clicked', async () => {
render(<ConfirmCancelModal isOpen={true} onConfirm={onConfirm} onCancel={onCancel} />);

fireEvent.click(screen.getByRole('button', { name: 'Cancel without creating' }));
fireEvent.click(screen.getByRole('button', { name: 'Delete' }));

await waitFor(() => {
expect(onConfirm).toHaveBeenCalledTimes(1);
expect(onCancel).not.toHaveBeenCalled();
});
});

it('calls onCancel when Keep creating is clicked', async () => {
it('calls onCancel when Keep review open is clicked', async () => {
render(<ConfirmCancelModal isOpen={true} onConfirm={onConfirm} onCancel={onCancel} />);

fireEvent.click(screen.getByRole('button', { name: 'Keep creating' }));
fireEvent.click(screen.getByRole('button', { name: 'Keep review open' }));

await waitFor(() => {
expect(onCancel).toHaveBeenCalledTimes(1);
Expand All @@ -68,9 +68,7 @@ describe('ConfirmCancelModal', () => {
render(<ConfirmCancelModal isOpen={false} onConfirm={onConfirm} onCancel={onCancel} />);

await waitFor(() => {
expect(
screen.queryByRole('heading', { name: "You're about to lose your progress" })
).toBeNull();
expect(screen.queryByRole('heading', { name: 'Delete this job?' })).toBeNull();
});
});

Expand All @@ -85,16 +83,13 @@ describe('ConfirmCancelModal', () => {
);

await waitFor(() => {
expect(screen.getByRole('button', { name: 'Keep creating' })).toHaveProperty(
expect(screen.getByRole('button', { name: 'Keep review open' })).toHaveProperty(
'disabled',
true
);
// When isLoading, F36 injects a spinner so the accessible name gains "Loading…".
// Query by partial text match to stay resilient to the spinner label.
expect(screen.getByRole('button', { name: /Cancel without creating/ })).toHaveProperty(
'disabled',
true
);
expect(screen.getByRole('button', { name: /Delete/ })).toHaveProperty('disabled', true);
});
});

Expand All @@ -108,10 +103,10 @@ describe('ConfirmCancelModal', () => {
/>
);

await waitFor(() => screen.getByRole('button', { name: 'Keep creating' }));
await waitFor(() => screen.getByRole('button', { name: 'Keep review open' }));

fireEvent.click(screen.getByRole('button', { name: /Cancel without creating/ }));
fireEvent.click(screen.getByRole('button', { name: 'Keep creating' }));
fireEvent.click(screen.getByRole('button', { name: /Delete/ }));
fireEvent.click(screen.getByRole('button', { name: 'Keep review open' }));

expect(onConfirm).not.toHaveBeenCalled();
expect(onCancel).not.toHaveBeenCalled();
Expand Down
Loading