mukeshs/twenty_crm
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Fix modal click outside (#12120)

Browse Source 
Fixes https://github.com/twentyhq/twenty/issues/12111

The bug occurred because in
https://github.com/twentyhq/twenty/pull/12062, I changed the click
outside mode of the modal from compare pixels to compare html ref. This
happens because the modal is in a portal, so the `compareHTMLRef`
doesn't work.

A bug already existed before but since the mode was compare pixel, it
only happened when a dropdown was overflowing from the modal:


https://github.com/user-attachments/assets/e34bfaca-dd21-46e5-a532-a66ba494889d

I commented the tests `CancelButtonClick`, and `ConfirmButtonClick`
because they don't work with compare pixel mode (the `userEvent.click()`
creates a `MouseEvent` with `clientX`=0 and `clientY`=0 so it triggers
the click outside listener even when the story tiggers a click on an
element inside a modal)

We should find a way to make the ClickOutsideMode `compareHTMLRef` work
with portals. I believe the `comparePixels` mode was used as a hacky way
to get around this problem (hacky because of the existing bug above).
This commit is contained in:
Raphaël Bosi
2025-05-19 15:04:04 +02:00
committed by GitHub
parent efc43208d3
commit b70119dbe6
2 changed files with 41 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
packages/twenty-front/src/modules/ui/layout/modal/components/ModalHotkeysAndClickOutsideEffect.tsx
View File

@ -47,7 +47,7 @@ export const ModalHotkeysAndClickOutsideEffect = ({
onClose();
}
},
mode: ClickOutsideMode.compareHTMLRef,
mode: ClickOutsideMode.comparePixels,
});
return null;

80
packages/twenty-front/src/modules/ui/layout/modal/components/__stories__/ConfirmationModal.stories.tsx
View File

@ -130,51 +130,51 @@ export const ConfirmWithEnterKey: Story = {
},
};
export const CancelButtonClick: Story = {
args: {
modalId: 'confirmation-modal',
title: 'Cancel Button Test',
subtitle: 'Clicking the cancel button should close the modal',
confirmButtonText: 'Confirm',
onClose: closeMock,
},
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);
// export const CancelButtonClick: Story = {
// args: {
// modalId: 'confirmation-modal',
// title: 'Cancel Button Test',
// subtitle: 'Clicking the cancel button should close the modal',
// confirmButtonText: 'Confirm',
// onClose: closeMock,
// },
// play: async ({ canvasElement }) => {
// const canvas = within(canvasElement);
await canvas.findByText('Cancel Button Test');
// await canvas.findByText('Cancel Button Test');
const cancelButton = await canvas.findByRole('button', {
name: /Cancel/,
});
await userEvent.click(cancelButton);
// const cancelButton = await canvas.findByRole('button', {
// name: /Cancel/,
// });
// await userEvent.click(cancelButton);
await waitFor(() => {
expect(closeMock).toHaveBeenCalledTimes(1);
});
},
};
// await waitFor(() => {
// expect(closeMock).toHaveBeenCalledTimes(1);
// });
// },
// };
export const ConfirmButtonClick: Story = {
args: {
modalId: 'confirmation-modal',
title: 'Confirm Button Test',
subtitle: 'Clicking the confirm button should trigger the confirm action',
confirmButtonText: 'Confirm',
onConfirmClick: confirmMock,
},
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);
// export const ConfirmButtonClick: Story = {
// args: {
// modalId: 'confirmation-modal',
// title: 'Confirm Button Test',
// subtitle: 'Clicking the confirm button should trigger the confirm action',
// confirmButtonText: 'Confirm',
// onConfirmClick: confirmMock,
// },
// play: async ({ canvasElement }) => {
// const canvas = within(canvasElement);
await canvas.findByText('Confirm Button Test');
// await canvas.findByText('Confirm Button Test');
const confirmButton = await canvas.findByRole('button', {
name: /Confirm/,
});
// const confirmButton = await canvas.findByRole('button', {
// name: /Confirm/,
// });
await userEvent.click(confirmButton);
// await userEvent.click(confirmButton);
await waitFor(() => {
expect(confirmMock).toHaveBeenCalledTimes(1);
});
},
};
// await waitFor(() => {
// expect(confirmMock).toHaveBeenCalledTimes(1);
// });
// },
// };