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

chore(content-explorer): migrate renameDialog #3666

Merged
merged 5 commits into from
Sep 25, 2024
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
6 changes: 6 additions & 0 deletions i18n/en-US.properties
Original file line number Diff line number Diff line change
Expand Up @@ -592,10 +592,16 @@ be.renameDialogErrorInUse = An item with the same name already exists.
be.renameDialogErrorInvalid = This name is invalid.
# Error text for rename dialog when name is too long
be.renameDialogErrorTooLong = This name is too long.
# Header for rename file dialog
be.renameDialogFileHeader = Rename File
# Header for rename folder dialog
be.renameDialogFolderHeader = Rename Folder
# Label for rename dialog
be.renameDialogLabel = Rename
# Text for rename dialog
be.renameDialogText = Please enter a new name for {name}:
# Header for rename web link dialog
be.renameDialogWebLinkHeader = Rename Link
# Label for resume action for a single file.
be.resume = Resume
# Label for resume action for multiple files.
Expand Down
14 changes: 14 additions & 0 deletions scripts/jest/jest-setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,20 @@
import '@testing-library/jest-dom';
import util from 'util';

Object.defineProperty(window, 'matchMedia', {
writable: true,
value: jest.fn().mockImplementation(query => ({
matches: false,
media: query,
onchange: null,
addListener: jest.fn(), // Deprecated
removeListener: jest.fn(), // Deprecated
addEventListener: jest.fn(),
removeEventListener: jest.fn(),
dispatchEvent: jest.fn(),
})),
});

Copy link
Contributor Author

@tjiang-box tjiang-box Sep 23, 2024

Choose a reason for hiding this comment

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

resolve issue TypeError: window.matchMedia is not a function when running tests
https://jestjs.io/docs/manual-mocks#mocking-methods-which-are-not-implemented-in-jsdom

Copy link
Contributor

Choose a reason for hiding this comment

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

if i recall this occurs when using fireEvent, can you try using userEvent for tests?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think he has to use fireEvent.change, I didnt see a way around it when I was doing something similar in the other PR, for everything else he could probably swap out with userEvent

Copy link
Contributor Author

@tjiang-box tjiang-box Sep 24, 2024

Choose a reason for hiding this comment

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

I swap out all fireEvent with userEvents now but still face this error. The matchMedia function might be wrapped internally somewhere.

@greg-in-a-box for fireEvent.change(), we could use userEvent.clear(textInput) + userEvent.type(textInput, 'xxx') to replace.

global.setImmediate = cb => {
setTimeout(cb, 0);
};
Expand Down
15 changes: 15 additions & 0 deletions src/elements/common/messages.js
Copy link
Contributor

Choose a reason for hiding this comment

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

can renameDialogText be removed now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good question! renameDialogText is still used inside of the renameDialog.js.flow file, so simply removing this message will cause flow check error. I believe those .flow file will be removed eventually after we finish migration? If so, should I leave it here since I believe we will also remove things used in this file such as messages or styles when we do it.

Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,21 @@ const messages = defineMessages({
description: 'Error text for rename dialog when name is too long',
defaultMessage: 'This name is too long.',
},
renameDialogFileHeader: {
id: 'be.renameDialogFileHeader',
description: 'Header for rename file dialog',
defaultMessage: 'Rename File',
},
renameDialogFolderHeader: {
id: 'be.renameDialogFolderHeader',
description: 'Header for rename folder dialog',
defaultMessage: 'Rename Folder',
},
renameDialogWebLinkHeader: {
id: 'be.renameDialogWebLinkHeader',
description: 'Header for rename web link dialog',
defaultMessage: 'Rename Link',
},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are 4 types of BoxItem in total ("folder", "file", "hubs", "web_link").

type ItemType = typeof ITEM_TYPE_FOLDER | typeof ITEM_TYPE_FILE | typeof ITEM_TYPE_HUBS | typeof ITEM_TYPE_WEBLINK;

const ITEM_TYPE_FILE: 'file' = 'file';
const ITEM_TYPE_FOLDER: 'folder' = 'folder';
const ITEM_TYPE_HUBS: 'hubs' = 'hubs';
const ITEM_TYPE_WEBLINK: 'web_link' = 'web_link';

Copy link
Contributor

Choose a reason for hiding this comment

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

are all these flows possible within ContentExplorer? could you check the behavior of this vs EUA?

createDialogLabel: {
id: 'be.createDialogLabel',
description: 'Label for create folder dialog',
Expand Down
1 change: 0 additions & 1 deletion src/elements/content-explorer/ContentExplorer.js
Original file line number Diff line number Diff line change
Expand Up @@ -1783,7 +1783,6 @@ class ContentExplorer extends Component<Props, State> {
isLoading={isLoading}
errorCode={errorCode}
parentElement={this.rootElement}
appElement={this.appElement}
/>
) : null}
{canShare && selected && !!this.appElement ? (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import {
import type { BoxItem } from '../../common/types/core';

type Props = {
appElement: HTMLElement,
errorCode: string,
intl: IntlShape,
isLoading: boolean,
Expand Down Expand Up @@ -102,7 +101,6 @@ const RenameDialog = ({

return (
<Modal
appElement={appElement}
className={CLASS_MODAL_CONTENT}
contentLabel={intl.formatMessage(messages.renameDialogLabel)}
isOpen={isOpen}
Expand Down
6 changes: 6 additions & 0 deletions src/elements/content-explorer/RenameDialog.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
.bce-RenameDialog {
// override the textinput width inside _forms.scss
input[type='text'] {
width: 100%;
}
}
130 changes: 130 additions & 0 deletions src/elements/content-explorer/RenameDialog.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
import * as React from 'react';
import { useIntl } from 'react-intl';
import { Modal, TextInput } from '@box/blueprint-web';
import type { BoxItem } from '../../common/types/core';

import {
ERROR_CODE_ITEM_NAME_IN_USE,
ERROR_CODE_ITEM_NAME_TOO_LONG,
TYPE_FILE,
TYPE_FOLDER,
TYPE_WEBLINK,
} from '../../constants';

import messages from '../common/messages';

import './RenameDialog.scss';

export interface RenameDialogProps {
errorCode: string;
isLoading: boolean;
isOpen: boolean;
item: BoxItem;
onCancel: () => void;
onRename: (nameWithoutExt: string, extension: string) => void;
parentElement: HTMLElement;
}

const RenameDialog = ({ errorCode, isLoading, isOpen, item, onCancel, onRename, parentElement }: RenameDialogProps) => {
const { formatMessage } = useIntl();

let textInput = null;
let error;

const { name = '', extension, type } = item;
const ext = extension ? `.${extension}` : '';
const nameWithoutExt = extension ? name.replace(ext, '') : name;

const headerMessages = {
[TYPE_FILE]: messages.renameDialogFileHeader,
[TYPE_FOLDER]: messages.renameDialogFolderHeader,
[TYPE_WEBLINK]: messages.renameDialogWebLinkHeader,
};

/**
* Appends the extension and calls rename function
*/
const rename = () => {
if (textInput && textInput.value) {
if (textInput.value === nameWithoutExt) {
onCancel();
} else {
onRename(textInput.value, ext);
}
}
};

/**
* Grabs reference to the input element
*/
const ref = input => {
textInput = input;
if (textInput instanceof HTMLInputElement) {
textInput.focus();
textInput.select();
}
};

/**
* Handles enter key down
*/
const onKeyDown = ({ key }) => {
switch (key) {
case 'Enter':
rename();
break;
default:
break;
}
};

switch (errorCode) {
case ERROR_CODE_ITEM_NAME_IN_USE:
error = messages.renameDialogErrorInUse;
break;
case ERROR_CODE_ITEM_NAME_TOO_LONG:
error = messages.renameDialogErrorTooLong;
break;
default:
error = errorCode ? messages.renameDialogErrorInvalid : null;
break;
}

return (
<Modal onOpenChange={onCancel} open={isOpen}>
tjuanitas marked this conversation as resolved.
Show resolved Hide resolved
<Modal.Content
aria-label={formatMessage(messages.renameDialogLabel)}
className="bce-RenameDialog"
container={parentElement}
size="small"
>
<Modal.Header>{formatMessage(headerMessages[type])}</Modal.Header>
<Modal.Body>
<TextInput
defaultValue={nameWithoutExt}
error={error && formatMessage(error)}
label={formatMessage(messages.name)}
onKeyDown={onKeyDown}
ref={ref}
required
/>
</Modal.Body>
<Modal.Footer>
<Modal.Footer.SecondaryButton disabled={isLoading} onClick={onCancel}>
{formatMessage(messages.cancel)}
</Modal.Footer.SecondaryButton>
<Modal.Footer.PrimaryButton
loading={isLoading}
loadingAriaLabel={formatMessage(messages.loading)}
onClick={rename}
>
{formatMessage(messages.rename)}
</Modal.Footer.PrimaryButton>
</Modal.Footer>
<Modal.Close aria-label={formatMessage(messages.close)} />
</Modal.Content>
</Modal>
);
};

export default RenameDialog;
115 changes: 115 additions & 0 deletions src/elements/content-explorer/__tests__/RenameDialog.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
import * as React from 'react';
import userEvent from '@testing-library/user-event';
import { render, screen } from '../../../test-utils/testing-library';

import RenameDialog, { RenameDialogProps } from '../RenameDialog';

describe('elements/content-explorer/RenameDialog', () => {
const mockItem = {
id: '123456',
name: 'mockFile',
extension: 'txt',
type: 'file',
};

const defaultProps = {
errorCode: '',
isLoading: false,
isOpen: false,
item: mockItem,
onCancel: jest.fn(),
onRename: jest.fn(),
parentElement: document.body,
};

const renderComponent = (props: Partial<RenameDialogProps>) =>
render(<RenameDialog {...defaultProps} {...props} />);

test('render rename dialog correctly when it is open and not loading', () => {
renderComponent({ isOpen: true, item: mockItem });

expect(screen.getByText('Rename File')).toBeInTheDocument();
expect(screen.getByRole('button', { name: 'Rename' })).toBeInTheDocument();
expect(screen.getByRole('button', { name: 'Cancel' })).toBeInTheDocument();
expect(screen.getByRole('button', { name: 'Close' })).toBeInTheDocument();

const textInput = screen.getByLabelText('Name');
expect(textInput).toBeInTheDocument();
expect(textInput).toHaveValue('mockFile');
});

test('render dialog footer correctly when it is open and loading', () => {
renderComponent({ isOpen: true, isLoading: true });

const cancelButton = screen.getByRole('button', { name: 'Cancel' });
expect(cancelButton).toBeInTheDocument();
expect(cancelButton).toBeDisabled();

const loadingIndicator = screen.getByRole('status', { name: 'Loading' });
expect(loadingIndicator).toBeInTheDocument();

const renameButton = screen.queryByRole('button', { name: 'Rename' });
expect(renameButton).not.toBeInTheDocument();
});

test('call onRename with input value and extension when rename button is clicked', async () => {
const mockRenameFunction = jest.fn();
renderComponent({ isOpen: true, item: mockItem, onRename: mockRenameFunction });

const textInput = screen.getByLabelText('Name');
await userEvent.clear(textInput);
await userEvent.type(textInput, 'newFileName');
await userEvent.click(screen.getByRole('button', { name: 'Rename' }));

expect(mockRenameFunction).toHaveBeenCalledTimes(1);
expect(mockRenameFunction).toHaveBeenCalledWith('newFileName', '.txt');
});

test('call onRename with input value and extension when enter key is pressed', async () => {
const mockRenameFunction = jest.fn();
renderComponent({ isOpen: true, item: mockItem, onRename: mockRenameFunction });

const textInput = screen.getByLabelText('Name');
await userEvent.clear(textInput);
await userEvent.type(textInput, 'newFileName');
await userEvent.keyboard('{Enter}');

expect(mockRenameFunction).toHaveBeenCalledTimes(1);
expect(mockRenameFunction).toHaveBeenCalledWith('newFileName', '.txt');
});

test('call onCancel when cancel button is clicked', async () => {
const mockCancelFunction = jest.fn();
renderComponent({ isOpen: true, onCancel: mockCancelFunction });

await userEvent.click(screen.getByRole('button', { name: 'Cancel' }));
expect(mockCancelFunction).toHaveBeenCalledTimes(1);
});

test('call onCancel when close icon is clicked', async () => {
const mockCancelFunction = jest.fn();
renderComponent({ isOpen: true, onCancel: mockCancelFunction });

await userEvent.click(screen.getByRole('button', { name: 'Close' }));
expect(mockCancelFunction).toHaveBeenCalledTimes(1);
});

test('call onCancel when text value is not changed and rename button is clicked', async () => {
const mockCancelFunction = jest.fn();
renderComponent({ isOpen: true, onCancel: mockCancelFunction });

await userEvent.click(screen.getByRole('button', { name: 'Rename' }));
expect(mockCancelFunction).toHaveBeenCalledTimes(1);
});

test.each`
errorCode | expectedError
${'item_name_in_use'} | ${'An item with the same name already exists.'}
${'item_name_too_long'} | ${'This name is too long.'}
${'default'} | ${'This name is invalid.'}
`('render correct error message based on errorCode', ({ errorCode, expectedError }) => {
renderComponent({ isOpen: true, errorCode });

expect(screen.getByText(expectedError)).toBeInTheDocument();
});
});
51 changes: 51 additions & 0 deletions src/elements/content-explorer/stories/RenameDialog.stories.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import * as React from 'react';
import { useArgs } from '@storybook/preview-api';

import { Button } from '@box/blueprint-web';
import { addRootElement } from '../../../utils/storybook';

import RenameDialog from '../RenameDialog';

export const renameDialog = {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
render: (args: any) => {
// eslint-disable-next-line react-hooks/rules-of-hooks
const [, setArgs] = useArgs();

const handleOpenModal = () => setArgs({ isOpen: true });

const handleCloseModal = () => {
setArgs({ isOpen: false });
};

const { rootElement } = addRootElement();

return (
<div>
<RenameDialog
item={{
id: '123456',
name: 'mockItem',
type: 'file',
}}
onCancel={handleCloseModal}
parentElement={rootElement}
{...args}
/>

<Button onClick={handleOpenModal} variant="primary">
Launch RenameDialog
</Button>
</div>
);
},
};

export default {
title: 'Elements/ContentExplorer',
component: RenameDialog,
args: {
isLoading: false,
isOpen: false,
},
};
Loading
Loading