From 9d9385465d01a9ba3d60e7ac194b1ab7e9d9ccd1 Mon Sep 17 00:00:00 2001 From: JakubKida Date: Thu, 19 Sep 2024 14:09:39 +0200 Subject: [PATCH 1/8] fix(metadata-sidebar): Fix selecting and cancelling templates --- .../MetadataInstanceEditor.tsx | 3 ++ .../MetadataSidebarRedesign.tsx | 40 ++++++++++++++----- 2 files changed, 33 insertions(+), 10 deletions(-) diff --git a/src/elements/content-sidebar/MetadataInstanceEditor.tsx b/src/elements/content-sidebar/MetadataInstanceEditor.tsx index c5884fdade..d03839e06e 100644 --- a/src/elements/content-sidebar/MetadataInstanceEditor.tsx +++ b/src/elements/content-sidebar/MetadataInstanceEditor.tsx @@ -16,6 +16,7 @@ export interface MetadataInstanceEditorProps { template: MetadataTemplateInstance; onSubmit: (values: FormValues, operations: JSONPatchOperations) => Promise; setIsUnsavedChangesModalOpen: (isUnsavedChangesModalOpen: boolean) => void; + onUnsavedChangesModalCancel: () => void; } const MetadataInstanceEditor: React.FC = ({ @@ -27,6 +28,7 @@ const MetadataInstanceEditor: React.FC = ({ setIsUnsavedChangesModalOpen, template, onCancel, + onUnsavedChangesModalCancel, }) => { const handleCancel = () => { onCancel(); @@ -43,6 +45,7 @@ const MetadataInstanceEditor: React.FC = ({ onSubmit={onSubmit} setIsUnsavedChangesModalOpen={setIsUnsavedChangesModalOpen} onDelete={onDelete} + onUnsavedChangesModalCancel={onUnsavedChangesModalCancel} /> ); diff --git a/src/elements/content-sidebar/MetadataSidebarRedesign.tsx b/src/elements/content-sidebar/MetadataSidebarRedesign.tsx index b60b27000c..98c81c8473 100644 --- a/src/elements/content-sidebar/MetadataSidebarRedesign.tsx +++ b/src/elements/content-sidebar/MetadataSidebarRedesign.tsx @@ -90,19 +90,40 @@ function MetadataSidebarRedesign({ const [isDeleteButtonDisabled, setIsDeleteButtonDisabled] = React.useState(false); const [selectedTemplates, setSelectedTemplates] = React.useState>(templateInstances); + const [pendingTemplateToEdit, setPendingTemplateToEdit] = React.useState(null); React.useEffect(() => { setSelectedTemplates(templateInstances); }, [templateInstances]); - const handleUnsavedChanges = () => { - setIsUnsavedChangesModalOpen(true); + const handleTemplateSelect = (selectedTemplate: MetadataTemplate) => { + if (editingTemplate) { + setPendingTemplateToEdit(convertTemplateToTemplateInstance(file, selectedTemplate)); + setIsUnsavedChangesModalOpen(true); + } else { + setSelectedTemplates([...selectedTemplates, selectedTemplate]); + setEditingTemplate(convertTemplateToTemplateInstance(file, selectedTemplate)); + setIsDeleteButtonDisabled(true); + } }; - const handleTemplateSelect = (selectedTemplate: MetadataTemplate) => { - setSelectedTemplates([...selectedTemplates, selectedTemplate]); - setEditingTemplate(convertTemplateToTemplateInstance(file, selectedTemplate)); - setIsDeleteButtonDisabled(true); + const handleCancel = () => { + setEditingTemplate(null); + setSelectedTemplates(templateInstances); + }; + + const handleCancelUnsavedChanges = () => { + // check if user tried to edit another template before unsaved changes modal + if (pendingTemplateToEdit) { + setEditingTemplate(pendingTemplateToEdit); + setSelectedTemplates([...templateInstances, pendingTemplateToEdit]); + setIsDeleteButtonDisabled(true); + + setPendingTemplateToEdit(null); + setIsUnsavedChangesModalOpen(false); + } else { + handleCancel(); + } }; const handleDeleteInstance = (metadataInstance: MetadataTemplateInstance) => { @@ -128,9 +149,7 @@ function MetadataSidebarRedesign({ { - editingTemplate ? handleUnsavedChanges() : handleTemplateSelect(selectedTemplate); - }} + onSelect={handleTemplateSelect} /> ); @@ -166,7 +185,8 @@ function MetadataSidebarRedesign({ isBoxAiSuggestionsEnabled={isBoxAiSuggestionsEnabled} isDeleteButtonDisabled={isDeleteButtonDisabled} isUnsavedChangesModalOpen={isUnsavedChangesModalOpen} - onCancel={() => setEditingTemplate(null)} + onCancel={handleCancel} + onUnsavedChangesModalCancel={handleCancelUnsavedChanges} onSubmit={handleSubmit} onDelete={handleDeleteInstance} template={editingTemplate} From 5c7e3e9de4848eb7cd5a4aa43f6da9faa9f0eb44 Mon Sep 17 00:00:00 2001 From: JakubKida Date: Thu, 19 Sep 2024 14:23:27 +0200 Subject: [PATCH 2/8] fix(metadata-sidebar): Add new MetadataInstanceEditor props in tests --- .../content-sidebar/__tests__/MetadataInstanceEditor.test.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/elements/content-sidebar/__tests__/MetadataInstanceEditor.test.tsx b/src/elements/content-sidebar/__tests__/MetadataInstanceEditor.test.tsx index 3d56958ba1..31237bd43c 100644 --- a/src/elements/content-sidebar/__tests__/MetadataInstanceEditor.test.tsx +++ b/src/elements/content-sidebar/__tests__/MetadataInstanceEditor.test.tsx @@ -33,6 +33,7 @@ describe('MetadataInstanceEditor', () => { onDelete: jest.fn(), onSubmit: jest.fn(), setIsUnsavedChangesModalOpen: jest.fn(), + onUnsavedChangesModalCancel: jest.fn(), }; test('should render MetadataInstanceForm with correct props', () => { From d7f55b38fddcf21954774ef5e387e23869b2dfbc Mon Sep 17 00:00:00 2001 From: Jakub Kida Date: Mon, 23 Sep 2024 14:08:19 +0200 Subject: [PATCH 3/8] feat(metadata-sidebar): Add storybook test for cancelling template edit --- ...MetadataSidebarRedesign-visual.stories.tsx | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/elements/content-sidebar/stories/tests/MetadataSidebarRedesign-visual.stories.tsx b/src/elements/content-sidebar/stories/tests/MetadataSidebarRedesign-visual.stories.tsx index a9c699137b..843fc13836 100644 --- a/src/elements/content-sidebar/stories/tests/MetadataSidebarRedesign-visual.stories.tsx +++ b/src/elements/content-sidebar/stories/tests/MetadataSidebarRedesign-visual.stories.tsx @@ -248,3 +248,31 @@ export const DeleteButtonIsEnabledWhenEditingMetadataTemplateInstance: StoryObj< expect(deleteButton).toBeEnabled(); }, }; + +export const MetadataInstanceEditorAddTemplateAgainAfterCancel: StoryObj = { + args: { + fileId: '416047501580', + metadataSidebarProps: defaultMetadataSidebarProps, + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + + const addTemplateButton = await canvas.findByRole('button', { name: 'Add template' }, { timeout: 5000 }); + + await userEvent.click(addTemplateButton); + + const templateMetadataOption = canvas.getByRole('option', { name: 'My Template' }); + expect(templateMetadataOption).not.toHaveStyle('pointer-events: none;'); + await userEvent.click(templateMetadataOption); + + // Check if currently open template is disabled in dropdown + await userEvent.click(addTemplateButton); + expect(templateMetadataOption).toHaveStyle('pointer-events: none;'); + + // Check if template available again after cancelling + const cancelButton = await canvas.findByRole('button', { name: 'Cancel' }); + await userEvent.click(cancelButton); + await userEvent.click(addTemplateButton); + expect(templateMetadataOption).not.toHaveStyle('pointer-events: none;'); + }, +}; From 1bf201be6358da93e334226594056a9d9349dd1d Mon Sep 17 00:00:00 2001 From: Jakub Kida Date: Mon, 23 Sep 2024 15:23:24 +0200 Subject: [PATCH 4/8] feat(metadata-sidebar): add tests for onCancel and onUnsavedChangesModalCancel action --- .../__tests__/MetadataInstanceEditor.test.tsx | 65 +++++++++++++++++-- 1 file changed, 61 insertions(+), 4 deletions(-) diff --git a/src/elements/content-sidebar/__tests__/MetadataInstanceEditor.test.tsx b/src/elements/content-sidebar/__tests__/MetadataInstanceEditor.test.tsx index 31237bd43c..bbebbb1433 100644 --- a/src/elements/content-sidebar/__tests__/MetadataInstanceEditor.test.tsx +++ b/src/elements/content-sidebar/__tests__/MetadataInstanceEditor.test.tsx @@ -1,8 +1,12 @@ import React from 'react'; import { type MetadataTemplateInstance } from '@box/metadata-editor'; -import { screen, render } from '../../../test-utils/testing-library'; +import { screen, render, fireEvent, act } from '../../../test-utils/testing-library'; import MetadataInstanceEditor, { MetadataInstanceEditorProps } from '../MetadataInstanceEditor'; +const mockOnCancel = jest.fn(); +const mockOnUnsavedChangesModalCancel = jest.fn(); +const mockSetIsUnsavedChangesModalOpen = jest.fn(); + describe('MetadataInstanceEditor', () => { const mockCustomMetadataTemplate: MetadataTemplateInstance = { id: 'template-id', @@ -19,6 +23,25 @@ describe('MetadataInstanceEditor', () => { displayName: 'Template Name', canEdit: true, }; + + const mockCustomMetadataTemplateWithField: MetadataTemplateInstance = { + id: 'template-id', + fields: [ + { + id: '1', + type: 'string', + key: 'signature', + hidden: false, + displayName: 'Signature', + }, + ], + scope: 'global', + templateKey: 'customTemplate', + type: 'template-id', + hidden: false, + canEdit: true, + }; + const mockMetadataTemplateInstance: MetadataTemplateInstance = { ...mockCustomMetadataTemplate, displayName: 'Template Name', @@ -29,11 +52,11 @@ describe('MetadataInstanceEditor', () => { isDeleteButtonDisabled: false, isUnsavedChangesModalOpen: false, template: mockMetadataTemplate, - onCancel: jest.fn(), + onCancel: mockOnCancel, onDelete: jest.fn(), onSubmit: jest.fn(), - setIsUnsavedChangesModalOpen: jest.fn(), - onUnsavedChangesModalCancel: jest.fn(), + setIsUnsavedChangesModalOpen: mockSetIsUnsavedChangesModalOpen, + onUnsavedChangesModalCancel: mockOnUnsavedChangesModalCancel, }; test('should render MetadataInstanceForm with correct props', () => { @@ -87,4 +110,38 @@ describe('MetadataInstanceEditor', () => { const deleteButton = screen.getByRole('button', { name: 'Delete' }); expect(deleteButton).toBeEnabled(); }); + + test('Should call onCancel when canceling editing', () => { + const props: MetadataInstanceEditorProps = { ...defaultProps, template: mockCustomMetadataTemplate }; + const { getByRole } = render(); + + act(() => { + fireEvent.click(getByRole('button', { name: 'Cancel' })); + }); + + expect(mockOnCancel).toHaveBeenCalled(); + }); + + test('Should call onUnsavedChangesModalCancel instead onCancel when canceling through UnsavedChangesModal', () => { + const props: MetadataInstanceEditorProps = { ...defaultProps, template: mockCustomMetadataTemplateWithField }; + const { getByRole, rerender } = render(); + const input = getByRole('textbox'); + + act(() => { + fireEvent.change(input, { target: { value: 'Lorem ipsum dolor.' } }); + fireEvent.click(getByRole('button', { name: 'Cancel' })); + }); + + expect(mockOnCancel).not.toHaveBeenCalled(); + expect(mockSetIsUnsavedChangesModalOpen).toHaveBeenCalledWith(true); + + rerender(); + const unsavedChangesModal = screen.getByText('Unsaved Changes'); + expect(unsavedChangesModal).toBeInTheDocument(); + + act(() => { + fireEvent.click(getByRole('button', { name: 'Cancel' })); + }); + expect(mockOnUnsavedChangesModalCancel).toHaveBeenCalled(); + }); }); From af35fd238083749b5ad817969e97f289968697e2 Mon Sep 17 00:00:00 2001 From: Jakub Kida Date: Mon, 23 Sep 2024 16:55:34 +0200 Subject: [PATCH 5/8] fix(metadata-editor): fix failing tests --- .../__tests__/MetadataInstanceEditor.test.tsx | 1 + .../tests/MetadataSidebarRedesign-visual.stories.tsx | 6 ++++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/elements/content-sidebar/__tests__/MetadataInstanceEditor.test.tsx b/src/elements/content-sidebar/__tests__/MetadataInstanceEditor.test.tsx index bbebbb1433..0cebde2fd6 100644 --- a/src/elements/content-sidebar/__tests__/MetadataInstanceEditor.test.tsx +++ b/src/elements/content-sidebar/__tests__/MetadataInstanceEditor.test.tsx @@ -137,6 +137,7 @@ describe('MetadataInstanceEditor', () => { rerender(); const unsavedChangesModal = screen.getByText('Unsaved Changes'); + expect(unsavedChangesModal).toBeInTheDocument(); act(() => { diff --git a/src/elements/content-sidebar/stories/tests/MetadataSidebarRedesign-visual.stories.tsx b/src/elements/content-sidebar/stories/tests/MetadataSidebarRedesign-visual.stories.tsx index 843fc13836..701dd625ed 100644 --- a/src/elements/content-sidebar/stories/tests/MetadataSidebarRedesign-visual.stories.tsx +++ b/src/elements/content-sidebar/stories/tests/MetadataSidebarRedesign-visual.stories.tsx @@ -267,12 +267,14 @@ export const MetadataInstanceEditorAddTemplateAgainAfterCancel: StoryObj Date: Tue, 24 Sep 2024 16:03:34 +0200 Subject: [PATCH 6/8] fix(metadata-sidebar): Fix attribute checks in tests --- .../tests/MetadataSidebarRedesign-visual.stories.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/elements/content-sidebar/stories/tests/MetadataSidebarRedesign-visual.stories.tsx b/src/elements/content-sidebar/stories/tests/MetadataSidebarRedesign-visual.stories.tsx index 701dd625ed..abec549672 100644 --- a/src/elements/content-sidebar/stories/tests/MetadataSidebarRedesign-visual.stories.tsx +++ b/src/elements/content-sidebar/stories/tests/MetadataSidebarRedesign-visual.stories.tsx @@ -262,19 +262,19 @@ export const MetadataInstanceEditorAddTemplateAgainAfterCancel: StoryObj Date: Wed, 25 Sep 2024 12:38:42 +0200 Subject: [PATCH 7/8] fix(metadata-editor): Fix test warnings --- .../__tests__/MetadataInstanceEditor.test.tsx | 75 ++++++++++--------- 1 file changed, 41 insertions(+), 34 deletions(-) diff --git a/src/elements/content-sidebar/__tests__/MetadataInstanceEditor.test.tsx b/src/elements/content-sidebar/__tests__/MetadataInstanceEditor.test.tsx index 0cebde2fd6..bdb2a128af 100644 --- a/src/elements/content-sidebar/__tests__/MetadataInstanceEditor.test.tsx +++ b/src/elements/content-sidebar/__tests__/MetadataInstanceEditor.test.tsx @@ -1,6 +1,7 @@ import React from 'react'; import { type MetadataTemplateInstance } from '@box/metadata-editor'; -import { screen, render, fireEvent, act } from '../../../test-utils/testing-library'; +import userEvent from '@testing-library/user-event'; +import { screen, render } from '../../../test-utils/testing-library'; import MetadataInstanceEditor, { MetadataInstanceEditorProps } from '../MetadataInstanceEditor'; const mockOnCancel = jest.fn(); @@ -59,6 +60,21 @@ describe('MetadataInstanceEditor', () => { onUnsavedChangesModalCancel: mockOnUnsavedChangesModalCancel, }; + // Mock window.matchMedia to simulate media query behavior for tests + // in which UnsavedChangesModal component relies on it. + const mockMatchMedia = () => + Object.defineProperty(window, 'matchMedia', { + writable: true, + value: jest.fn().mockImplementation(query => ({ + matches: false, + media: query, + onchange: null, + addEventListener: jest.fn(), + removeEventListener: jest.fn(), + dispatchEvent: jest.fn(), + })), + }); + test('should render MetadataInstanceForm with correct props', () => { render(); @@ -74,25 +90,13 @@ describe('MetadataInstanceEditor', () => { expect(templateHeader).toBeInTheDocument(); }); - test('should render UnsavedChangesModal if isUnsavedChangesModalOpen is true', () => { - // Mock window.matchMedia to simulate media query behavior for this test, - // as the UnsavedChangesModal component relies on it. - Object.defineProperty(window, 'matchMedia', { - writable: true, - value: jest.fn().mockImplementation(query => ({ - matches: false, - media: query, - onchange: null, - addEventListener: jest.fn(), - removeEventListener: jest.fn(), - dispatchEvent: jest.fn(), - })), - }); + test('should render UnsavedChangesModal if isUnsavedChangesModalOpen is true', async () => { + mockMatchMedia(); const props = { ...defaultProps, isUnsavedChangesModalOpen: true }; - render(); + const { findByText } = render(); - const unsavedChangesModal = screen.getByText('Unsaved Changes'); + const unsavedChangesModal = await findByText('Unsaved Changes'); expect(unsavedChangesModal).toBeInTheDocument(); }); @@ -111,38 +115,41 @@ describe('MetadataInstanceEditor', () => { expect(deleteButton).toBeEnabled(); }); - test('Should call onCancel when canceling editing', () => { + test('Should call onCancel when canceling editing', async () => { const props: MetadataInstanceEditorProps = { ...defaultProps, template: mockCustomMetadataTemplate }; - const { getByRole } = render(); + const { findByRole } = render(); + const cancelButton = await findByRole('button', { name: 'Cancel' }); - act(() => { - fireEvent.click(getByRole('button', { name: 'Cancel' })); - }); + await userEvent.click(cancelButton); expect(mockOnCancel).toHaveBeenCalled(); }); - test('Should call onUnsavedChangesModalCancel instead onCancel when canceling through UnsavedChangesModal', () => { - const props: MetadataInstanceEditorProps = { ...defaultProps, template: mockCustomMetadataTemplateWithField }; - const { getByRole, rerender } = render(); - const input = getByRole('textbox'); + test('Should call onUnsavedChangesModalCancel instead onCancel when canceling through UnsavedChangesModal', async () => { + mockMatchMedia(); - act(() => { - fireEvent.change(input, { target: { value: 'Lorem ipsum dolor.' } }); - fireEvent.click(getByRole('button', { name: 'Cancel' })); - }); + const props: MetadataInstanceEditorProps = { + ...defaultProps, + template: mockCustomMetadataTemplateWithField, + }; + const { rerender, findByRole, findByText } = render(); + const input = await findByRole('textbox'); + const cancelButton = await findByRole('button', { name: 'Cancel' }); + + await userEvent.type(input, 'Lorem ipsum dolor.'); + await userEvent.click(cancelButton); expect(mockOnCancel).not.toHaveBeenCalled(); expect(mockSetIsUnsavedChangesModalOpen).toHaveBeenCalledWith(true); rerender(); - const unsavedChangesModal = screen.getByText('Unsaved Changes'); + const unsavedChangesModal = await findByText('Unsaved Changes'); expect(unsavedChangesModal).toBeInTheDocument(); + const unsavedChangesModalCancelButton = await findByRole('button', { name: 'Cancel' }); + + await userEvent.click(unsavedChangesModalCancelButton); - act(() => { - fireEvent.click(getByRole('button', { name: 'Cancel' })); - }); expect(mockOnUnsavedChangesModalCancel).toHaveBeenCalled(); }); }); From 62d915efb459727b08ad747d4631ccc38092bc35 Mon Sep 17 00:00:00 2001 From: Jakub Kida Date: Thu, 26 Sep 2024 10:29:48 +0200 Subject: [PATCH 8/8] fix(metadata-editor): Remove unnecessary mock --- .../__tests__/MetadataInstanceEditor.test.tsx | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/src/elements/content-sidebar/__tests__/MetadataInstanceEditor.test.tsx b/src/elements/content-sidebar/__tests__/MetadataInstanceEditor.test.tsx index bdb2a128af..a16ee03ba6 100644 --- a/src/elements/content-sidebar/__tests__/MetadataInstanceEditor.test.tsx +++ b/src/elements/content-sidebar/__tests__/MetadataInstanceEditor.test.tsx @@ -60,21 +60,6 @@ describe('MetadataInstanceEditor', () => { onUnsavedChangesModalCancel: mockOnUnsavedChangesModalCancel, }; - // Mock window.matchMedia to simulate media query behavior for tests - // in which UnsavedChangesModal component relies on it. - const mockMatchMedia = () => - Object.defineProperty(window, 'matchMedia', { - writable: true, - value: jest.fn().mockImplementation(query => ({ - matches: false, - media: query, - onchange: null, - addEventListener: jest.fn(), - removeEventListener: jest.fn(), - dispatchEvent: jest.fn(), - })), - }); - test('should render MetadataInstanceForm with correct props', () => { render(); @@ -91,8 +76,6 @@ describe('MetadataInstanceEditor', () => { }); test('should render UnsavedChangesModal if isUnsavedChangesModalOpen is true', async () => { - mockMatchMedia(); - const props = { ...defaultProps, isUnsavedChangesModalOpen: true }; const { findByText } = render(); @@ -126,8 +109,6 @@ describe('MetadataInstanceEditor', () => { }); test('Should call onUnsavedChangesModalCancel instead onCancel when canceling through UnsavedChangesModal', async () => { - mockMatchMedia(); - const props: MetadataInstanceEditorProps = { ...defaultProps, template: mockCustomMetadataTemplateWithField,