-
Notifications
You must be signed in to change notification settings - Fork 305
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
Changes from all commits
534c3ba
97c1867
a295502
b1ebf48
2fdf109
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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. can 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. good question! |
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -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', | ||||||||||||
}, | ||||||||||||
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. There are 4 types of BoxItem in total ( box-ui-elements/src/common/types/core.js Line 86 in f6abd4e
box-ui-elements/src/common/constants.js Lines 2 to 5 in f6abd4e
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. 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', | ||||||||||||
|
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%; | ||
} | ||
} |
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; |
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(); | ||
}); | ||
}); |
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, | ||
}, | ||
}; |
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.
resolve issue
TypeError: window.matchMedia is not a function
when running testshttps://jestjs.io/docs/manual-mocks#mocking-methods-which-are-not-implemented-in-jsdom
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.
if i recall this occurs when using
fireEvent
, can you try usinguserEvent
for tests?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.
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 withuserEvent
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.
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 useuserEvent.clear(textInput)
+userEvent.type(textInput, 'xxx')
to replace.