-
Notifications
You must be signed in to change notification settings - Fork 345
feat(content-uploader): Implement cancel all confirmation modal #4579
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
base: implement-cancelled-state-uploads-manager
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| import * as React from 'react'; | ||
| import { useIntl } from 'react-intl'; | ||
| import { AlertModal } from '@box/blueprint-web'; | ||
| import messages from './messages'; | ||
|
|
||
| export interface CancelAllUploadsModalProps { | ||
| isOpen: boolean; | ||
| onConfirm: () => void; | ||
| onDismiss: () => void; | ||
| } | ||
|
|
||
| export function CancelAllUploadsModal({ isOpen, onConfirm, onDismiss }: CancelAllUploadsModalProps) { | ||
| const { formatMessage } = useIntl(); | ||
|
|
||
| const handleOpenChange = (open: boolean) => { | ||
| if (!open) { | ||
| onDismiss(); | ||
| } | ||
| }; | ||
|
|
||
| return ( | ||
| <AlertModal | ||
| open={isOpen} | ||
| onOpenChange={handleOpenChange} | ||
| heading={formatMessage(messages.cancelAllUploadsModalHeading)} | ||
| textContent={formatMessage(messages.cancelAllUploadsModalContent)} | ||
| closeButtonAriaLabel={formatMessage(messages.cancelAllUploadsCloseLabel)} | ||
| > | ||
| <AlertModal.SecondaryButton onClick={onDismiss}> | ||
| {formatMessage(messages.cancelAllUploadsKeepButton)} | ||
| </AlertModal.SecondaryButton> | ||
| <AlertModal.PrimaryButton variant="destructive" onClick={onConfirm}> | ||
| {formatMessage(messages.cancelAllUploadsConfirmButton)} | ||
| </AlertModal.PrimaryButton> | ||
| </AlertModal> | ||
| ); | ||
| } | ||
|
|
||
| export default CancelAllUploadsModal; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,7 @@ import DroppableContent from './DroppableContent'; | |
| import Footer from './Footer'; | ||
| import UploadsManager from './UploadsManager'; | ||
| import { getUploadItemKey, mapToModernizedUploadItems } from './utils/mapToModernizedUploadItem'; | ||
| import CancelAllUploadsModal from './CancelAllUploadsModal'; | ||
| import API from '../../api'; | ||
| import Browser from '../../utils/Browser'; | ||
| import Internationalize from '../common/Internationalize'; | ||
|
|
@@ -113,6 +114,7 @@ export interface ContentUploaderProps { | |
|
|
||
| type State = { | ||
| errorCode?: string; | ||
| isCancelAllModalOpen: boolean; | ||
| isUploadsManagerExpanded: boolean; | ||
| itemIds: Object; | ||
| items: UploadItem[]; | ||
|
|
@@ -191,6 +193,7 @@ class ContentUploader extends Component<ContentUploaderProps, State> { | |
| items: [], | ||
| errorCode: '', | ||
| itemIds: {}, | ||
| isCancelAllModalOpen: false, | ||
| isUploadsManagerExpanded: false, | ||
| }; | ||
| this.id = uniqueid('bcu_'); | ||
|
|
@@ -977,19 +980,31 @@ class ContentUploader extends Component<ContentUploaderProps, State> { | |
| */ | ||
| updateViewAndCollection(items: UploadItem[], callback?: () => void) { | ||
| const { | ||
| enableModernizedUploads, | ||
| isPartialUploadEnabled, | ||
| isResumableUploadsEnabled, | ||
| onComplete, | ||
| useUploadsManager, | ||
| }: ContentUploaderProps = this.props; | ||
| const someUploadIsInProgress = items.some(uploadItem => uploadItem.status !== STATUS_COMPLETE); | ||
| // When the modernized flow is on, canceled items are kept in the list | ||
| // but treated as terminal so they do not block completion logic. The | ||
| // legacy view/state machine sees canceled items as non-COMPLETE which | ||
| // would otherwise stall progress and emit a stale success notification. | ||
| const isTerminalForModernized = (uploadItem: UploadItem) => | ||
| enableModernizedUploads && uploadItem.status === STATUS_CANCELED; | ||
| const someUploadIsInProgress = items.some( | ||
| uploadItem => uploadItem.status !== STATUS_COMPLETE && !isTerminalForModernized(uploadItem), | ||
| ); | ||
| const someUploadHasFailed = items.some(uploadItem => uploadItem.status === STATUS_ERROR); | ||
| const allItemsArePending = !items.some(uploadItem => uploadItem.status !== STATUS_PENDING); | ||
| const noFileIsPendingOrInProgress = items.every( | ||
| uploadItem => uploadItem.status !== STATUS_PENDING && uploadItem.status !== STATUS_IN_PROGRESS, | ||
| ); | ||
| const areAllItemsFinished = items.every( | ||
| uploadItem => uploadItem.status === STATUS_COMPLETE || uploadItem.status === STATUS_ERROR, | ||
| uploadItem => | ||
| uploadItem.status === STATUS_COMPLETE || | ||
| uploadItem.status === STATUS_ERROR || | ||
| isTerminalForModernized(uploadItem), | ||
| ); | ||
| const uploadItemsStatus = isResumableUploadsEnabled ? areAllItemsFinished : noFileIsPendingOrInProgress; | ||
|
|
||
|
|
@@ -1023,7 +1038,13 @@ class ContentUploader extends Component<ContentUploaderProps, State> { | |
| if (this.isAutoExpanded) { | ||
| this.resetUploadManagerExpandState(); | ||
| } // Else manually expanded so don't close | ||
| onComplete(items); | ||
| // In the modernized flow, suppress the completion notification | ||
| // when no item actually finished successfully (e.g. user canceled | ||
| // every upload). This prevents a misleading "upload complete" toast. | ||
| const hasCompletedItem = items.some(uploadItem => uploadItem.status === STATUS_COMPLETE); | ||
| if (!enableModernizedUploads || hasCompletedItem) { | ||
| onComplete(items); | ||
| } | ||
| } | ||
|
|
||
| const state: Partial<State> = { | ||
|
|
@@ -1181,6 +1202,24 @@ class ContentUploader extends Component<ContentUploaderProps, State> { | |
| onClickCancel(item); | ||
| }; | ||
|
|
||
| /** | ||
| * Open the Cancel All confirmation modal. Wired as the onCancelAll prop | ||
| * passed to the modernized uploads manager so the action requires explicit | ||
| * confirmation before destroying in-progress uploads. | ||
| */ | ||
| handleCancelAllRequest = () => { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [nit] This function name is quite misleading because it's not actually handling any (normally network) request. Can we change this to ex. |
||
| this.setState({ isCancelAllModalOpen: true }); | ||
| }; | ||
|
|
||
| handleCancelAllDismiss = () => { | ||
| this.setState({ isCancelAllModalOpen: false }); | ||
| }; | ||
|
|
||
| handleCancelAllConfirm = () => { | ||
| this.setState({ isCancelAllModalOpen: false }); | ||
| this.handleUploadsManagerCancelAll(); | ||
| }; | ||
|
|
||
| /** | ||
| * Cancel every pending or in-progress upload at once. Items keep their row | ||
| * in the list with the canceled status. Only used by the modernized flow. | ||
|
|
@@ -1409,7 +1448,7 @@ class ContentUploader extends Component<ContentUploaderProps, State> { | |
| theme, | ||
| useUploadsManager, | ||
| }: ContentUploaderProps = this.props; | ||
| const { view, items, errorCode, isUploadsManagerExpanded }: State = this.state; | ||
| const { view, items, errorCode, isCancelAllModalOpen, isUploadsManagerExpanded }: State = this.state; | ||
| const isEmpty = items.length === 0; | ||
| const isVisible = !isEmpty || !!isDraggingItemsToUploadsManager; | ||
|
|
||
|
|
@@ -1434,9 +1473,14 @@ class ContentUploader extends Component<ContentUploaderProps, State> { | |
| onItemCancel={this.handleUploadsManagerItemCancel} | ||
| onItemRetry={this.handleUploadsManagerItemRetry} | ||
| onItemRemove={this.handleUploadsManagerItemRemove} | ||
| onCancelAll={this.handleUploadsManagerCancelAll} | ||
| onCancelAll={this.handleCancelAllRequest} | ||
| onRetryAll={this.handleUploadsManagerRetryAll} | ||
| /> | ||
| <CancelAllUploadsModal | ||
| isOpen={isCancelAllModalOpen} | ||
| onConfirm={this.handleCancelAllConfirm} | ||
| onDismiss={this.handleCancelAllDismiss} | ||
| /> | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| import * as React from 'react'; | ||
| import { render, screen, userEvent } from '../../../test-utils/testing-library'; | ||
| import CancelAllUploadsModal, { type CancelAllUploadsModalProps } from '../CancelAllUploadsModal'; | ||
|
|
||
| const renderModal = (props: Partial<CancelAllUploadsModalProps> = {}) => { | ||
| const defaultProps: CancelAllUploadsModalProps = { | ||
| isOpen: true, | ||
| onConfirm: jest.fn(), | ||
| onDismiss: jest.fn(), | ||
| ...props, | ||
| }; | ||
| render(<CancelAllUploadsModal {...defaultProps} />); | ||
| return defaultProps; | ||
| }; | ||
|
|
||
| describe('elements/content-uploader/CancelAllUploadsModal', () => { | ||
| test('renders heading, body, and both action buttons when open', async () => { | ||
| renderModal(); | ||
| expect(await screen.findByRole('alertdialog')).toBeInTheDocument(); | ||
| expect(screen.getByText('Cancel all uploads?')).toBeInTheDocument(); | ||
| expect(screen.getByText(/Files that are still uploading will be canceled/)).toBeInTheDocument(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [nit] Is there a reason why this can not be expected with the literal string and has to be regex? |
||
| expect(screen.getByRole('button', { name: 'Cancel All' })).toBeInTheDocument(); | ||
| expect(screen.getByRole('button', { name: 'Keep Uploading' })).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| test('calls onConfirm when Cancel All is clicked', async () => { | ||
| const props = renderModal(); | ||
| const user = userEvent(); | ||
| const button = await screen.findByRole('button', { name: 'Cancel All' }); | ||
| await user.click(button); | ||
| expect(props.onConfirm).toHaveBeenCalledTimes(1); | ||
| expect(props.onDismiss).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| test('calls onDismiss when Keep Uploading is clicked', async () => { | ||
| const props = renderModal(); | ||
| const user = userEvent(); | ||
| const button = await screen.findByRole('button', { name: 'Keep Uploading' }); | ||
| await user.click(button); | ||
| expect(props.onDismiss).toHaveBeenCalledTimes(1); | ||
| expect(props.onConfirm).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| test('does not render dialog when isOpen is false', () => { | ||
| renderModal({ isOpen: false }); | ||
| expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument(); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| import { defineMessages } from 'react-intl'; | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [question] What's the intended scope of this file?
|
||
| const messages = defineMessages({ | ||
| cancelAllUploadsModalHeading: { | ||
| id: 'be.contentUploader.cancelAllUploadsModalHeading', | ||
| defaultMessage: 'Cancel all uploads?', | ||
| description: 'Heading for the cancel all uploads confirmation modal', | ||
| }, | ||
| cancelAllUploadsModalContent: { | ||
| id: 'be.contentUploader.cancelAllUploadsModalContent', | ||
| defaultMessage: 'Files that are still uploading will be canceled. Completed uploads will not be affected.', | ||
| description: 'Body content for the cancel all uploads confirmation modal', | ||
| }, | ||
| cancelAllUploadsConfirmButton: { | ||
| id: 'be.contentUploader.cancelAllUploadsConfirmButton', | ||
| defaultMessage: 'Cancel All', | ||
| description: 'Confirm button for the cancel all uploads modal', | ||
| }, | ||
| cancelAllUploadsKeepButton: { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The name feels slightly unclear what it's trying to do. How about |
||
| id: 'be.contentUploader.cancelAllUploadsKeepButton', | ||
| defaultMessage: 'Keep Uploading', | ||
| description: 'Dismiss button for the cancel all uploads modal', | ||
| }, | ||
| cancelAllUploadsCloseLabel: { | ||
| id: 'be.contentUploader.cancelAllUploadsCloseLabel', | ||
| defaultMessage: 'Close cancel uploads dialog', | ||
| description: 'Aria label for the close button on the cancel all uploads modal', | ||
| }, | ||
| }); | ||
|
|
||
| export default messages; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nit] imports should follow alphabetical order