fix(metadata-sidebar): handle onCancel action for editing templates #3669

JakubKida · 2024-09-19T12:46:22Z

Description

fix and refactor cancelling logic for MetadataInstanceEditor
add new additional props for MetadataInstanceEditor: setIsUnsavedChangesModalOpen and onUnsavedChangesModalCancel for handling the UnsavedChangesModal directly from BUIE
fix issues with templates in AddMetadataTemplateDropdown still being disabled after cancelling editing a new template
handle adding a new template when another one is still in edit mode with UnsavedChangesModal, add handleCancelUnsavedChanges in MetadataSidebarRedesign ('Cancel' action in UnsavedChangesModal) for checking if there are pending editors to switch to immediately after cancelling the previous one

Additional Notes:

Temporary conversion between MetadataTemplateInstance and MetadataTemplate was added, which will be handled properly in the future PR

CLAassistant · 2024-09-19T12:46:28Z

All committers have signed the CLA.

jankowiakdawid

Can we add one more story that shows one can add new template, cancel it, and add it again?
that was not working before

and maybe one that showcases current behavior of canceling editing

greg-in-a-box

can we add in some unit test coverage for your changes? i see the mock props in the defaultProps in one of the tests files but i dont see any tests added with this change to do with the onCancel action

src/elements/content-sidebar/stories/tests/MetadataSidebarRedesign-visual.stories.tsx

jankowiakdawid · 2024-09-24T12:39:07Z

src/elements/content-sidebar/MetadataSidebarRedesign.tsx

+        if (editingTemplate) {
+            setPendingTemplateToEdit(selectedTemplate as MetadataTemplateInstance);
+            setIsUnsavedChangesModalOpen(true);
+        } else {
+            setSelectedTemplates([...selectedTemplates, selectedTemplate]);
+            setEditingTemplate(selectedTemplate as MetadataTemplateInstance);


For future, as soon as @karolinaru merges #3663 we should use convertTemplateToTemplateInstance here.

greg-in-a-box · 2024-09-24T16:10:07Z

src/elements/content-sidebar/__tests__/MetadataInstanceEditor.test.tsx

+        const { getByRole } = render(<MetadataInstanceEditor {...props} />);
+
+        act(() => {
+            fireEvent.click(getByRole('button', { name: 'Cancel' }));


Suggested change

fireEvent.click(getByRole('button', { name: 'Cancel' }));

await userEvent.click(getByRole('button', { name: 'Cancel' }));

I dont think we need act if we are using the right combination of functions, if you still get the act error/warning still i wondering its you need to do use findByRole instead of getByRolewhich also might fix that error/warning

I changed that to not use it, is act deprecated or unwanted here though ? I thought it's just recommended to change where it's imported from.

you dont need to use act if you use userEvent, should already being doing the act within the function, if start seeing "maybe you need an act" errors, you could probably switch to userEvent in that case.

src/elements/content-sidebar/__tests__/MetadataInstanceEditor.test.tsx

greg-in-a-box · 2024-09-25T18:38:36Z

src/elements/content-sidebar/__tests__/MetadataInstanceEditor.test.tsx

+    // 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(),
+            })),
+        });
+


this was just added to master, in the global jest setup file,

box-ui-elements/scripts/jest/jest-setup.ts

Line 5 in f5f1bac

Object.defineProperty(window, 'matchMedia', {

rebase and remove this and verify if you still need this

JChan106

lgtm

…dalCancel action

JakubKida requested a review from a team as a code owner September 19, 2024 12:46

JakubKida force-pushed the metadata-cancel-editing branch 2 times, most recently from bab51ea to a365a22 Compare September 20, 2024 09:00

wpiesiak previously approved these changes Sep 20, 2024

View reviewed changes

jankowiakdawid reviewed Sep 20, 2024

View reviewed changes

greg-in-a-box requested changes Sep 20, 2024

View reviewed changes

JakubKida dismissed wpiesiak’s stale review via 5c750ea September 23, 2024 11:20

JakubKida force-pushed the metadata-cancel-editing branch from a365a22 to 5c750ea Compare September 23, 2024 11:20

JakubKida requested review from jankowiakdawid and greg-in-a-box September 23, 2024 13:24

JakubKida force-pushed the metadata-cancel-editing branch 2 times, most recently from aaa74a3 to 09d00c3 Compare September 24, 2024 09:08

JakubKida requested a review from wpiesiak September 24, 2024 09:08

JakubKida force-pushed the metadata-cancel-editing branch from 09d00c3 to 9ae3e54 Compare September 24, 2024 09:08

jankowiakdawid reviewed Sep 24, 2024

View reviewed changes

src/elements/content-sidebar/stories/tests/MetadataSidebarRedesign-visual.stories.tsx Outdated Show resolved Hide resolved

jankowiakdawid reviewed Sep 24, 2024

View reviewed changes

jankowiakdawid previously approved these changes Sep 24, 2024

View reviewed changes

JakubKida dismissed jankowiakdawid’s stale review via edb434d September 24, 2024 14:03

jankowiakdawid previously approved these changes Sep 24, 2024

View reviewed changes

JakubKida force-pushed the metadata-cancel-editing branch 2 times, most recently from 1573571 to acc6392 Compare September 24, 2024 15:27

greg-in-a-box requested changes Sep 24, 2024

View reviewed changes

JakubKida dismissed jankowiakdawid’s stale review via 04e074e September 25, 2024 10:39

JakubKida force-pushed the metadata-cancel-editing branch from 04e074e to 7656f6a Compare September 25, 2024 10:56

greg-in-a-box requested changes Sep 25, 2024

View reviewed changes

JakubKida force-pushed the metadata-cancel-editing branch 2 times, most recently from 7e5810c to 5cb0050 Compare September 26, 2024 13:53

jankowiakdawid approved these changes Sep 26, 2024

View reviewed changes

greg-in-a-box previously approved these changes Sep 26, 2024

View reviewed changes

jankowiakdawid added the ready-to-merge label Sep 26, 2024

jankowiakdawid previously approved these changes Sep 26, 2024

View reviewed changes

jankowiakdawid force-pushed the metadata-cancel-editing branch from 5cb0050 to 860a8bb Compare September 26, 2024 20:35

JakubKida dismissed stale reviews from greg-in-a-box and jankowiakdawid via 76e0493 September 27, 2024 11:21

JakubKida force-pushed the metadata-cancel-editing branch 2 times, most recently from 76e0493 to 694815c Compare September 27, 2024 11:33

JakubKida requested review from jankowiakdawid and greg-in-a-box September 27, 2024 11:47

jankowiakdawid approved these changes Sep 27, 2024

View reviewed changes

greg-in-a-box approved these changes Sep 27, 2024

View reviewed changes

JChan106 approved these changes Sep 30, 2024

View reviewed changes

TylerGauntlett approved these changes Sep 30, 2024

View reviewed changes

JakubKida and others added 8 commits October 1, 2024 09:30

fix(metadata-sidebar): Fix selecting and cancelling templates

9d93854

fix(metadata-sidebar): Add new MetadataInstanceEditor props in tests

5c7e3e9

feat(metadata-sidebar): Add storybook test for cancelling template edit

d7f55b3

feat(metadata-sidebar): add tests for onCancel and onUnsavedChangesMo…

1bf201b

…dalCancel action

fix(metadata-editor): fix failing tests

af35fd2

fix(metadata-sidebar): Fix attribute checks in tests

d2f7c99

fix(metadata-editor): Fix test warnings

698c7bd

fix(metadata-editor): Remove unnecessary mock

62d915e

JakubKida force-pushed the metadata-cancel-editing branch from 694815c to 62d915e Compare October 1, 2024 07:30

wpiesiak approved these changes Oct 1, 2024

View reviewed changes

mergify bot merged commit 6ad083b into box:master Oct 1, 2024
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(metadata-sidebar): handle onCancel action for editing templates #3669

fix(metadata-sidebar): handle onCancel action for editing templates #3669

JakubKida commented Sep 19, 2024 •

edited

Loading

CLAassistant commented Sep 19, 2024 •

edited

Loading

jankowiakdawid left a comment

greg-in-a-box left a comment •

edited

Loading

jankowiakdawid Sep 24, 2024

greg-in-a-box Sep 24, 2024

JakubKida Sep 25, 2024

greg-in-a-box Sep 25, 2024

greg-in-a-box Sep 25, 2024

JChan106 left a comment

	fireEvent.click(getByRole('button', { name: 'Cancel' }));
	await userEvent.click(getByRole('button', { name: 'Cancel' }));

fix(metadata-sidebar): handle onCancel action for editing templates #3669

fix(metadata-sidebar): handle onCancel action for editing templates #3669

Conversation

JakubKida commented Sep 19, 2024 • edited Loading

Description

Additional Notes:

CLAassistant commented Sep 19, 2024 • edited Loading

jankowiakdawid left a comment

Choose a reason for hiding this comment

greg-in-a-box left a comment • edited Loading

Choose a reason for hiding this comment

jankowiakdawid Sep 24, 2024

Choose a reason for hiding this comment

greg-in-a-box Sep 24, 2024

Choose a reason for hiding this comment

JakubKida Sep 25, 2024

Choose a reason for hiding this comment

greg-in-a-box Sep 25, 2024

Choose a reason for hiding this comment

greg-in-a-box Sep 25, 2024

Choose a reason for hiding this comment

JChan106 left a comment

Choose a reason for hiding this comment

JakubKida commented Sep 19, 2024 •

edited

Loading

CLAassistant commented Sep 19, 2024 •

edited

Loading

greg-in-a-box left a comment •

edited

Loading