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

fix: prevent opening form popup through detail popup when it is disabled #1252

Merged
merged 2 commits into from
Aug 16, 2022
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
59 changes: 55 additions & 4 deletions apps/calendar/src/components/popup/eventDetailPopup.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@ import { initCalendarStore, StoreProvider, useDispatch } from '@src/contexts/cal
import { EventBusProvider } from '@src/contexts/eventBus';
import { FloatingLayerProvider } from '@src/contexts/floatingLayer';
import EventModel from '@src/model/eventModel';
import { render, screen } from '@src/test/utils';
import { render, screen, userEvent } from '@src/test/utils';
import TZDate from '@src/time/date';
import { EventBusImpl } from '@src/utils/eventBus';

import type { PropsWithChildren } from '@t/components/common';
import type { Options } from '@t/options';

describe('event detail popup', () => {
const mockCalendarId = 'calendarId';
Expand Down Expand Up @@ -48,7 +49,7 @@ describe('event detail popup', () => {
return <FloatingLayerProvider>{children}</FloatingLayerProvider>;
};

beforeEach(() => {
function setup(options: Options = {}) {
const eventBus = new EventBusImpl();
const store = initCalendarStore({
calendars: [
Expand All @@ -57,9 +58,14 @@ describe('event detail popup', () => {
name: mockCalendarName,
},
],
...options,
});

render(
// Spy should be set before rendering
const showFormPopupSpy = jest.fn();
store.getState().dispatch.popup.showFormPopup = showFormPopupSpy;

const renderResult = render(
<EventBusProvider value={eventBus}>
<StoreProvider store={store}>
<Wrapper>
Expand All @@ -68,23 +74,33 @@ describe('event detail popup', () => {
</StoreProvider>
</EventBusProvider>
);
});

return {
eventBus,
store,
renderResult,
showFormPopupSpy,
};
}

it('should display location when `event.location` is exists', () => {
setup();
const { location } = event;
const locationText = screen.getByText(location).textContent;

expect(locationText).toBe(location);
});

it('should display recurrence rule when `event.recurrenceRule` is exists', () => {
setup();
const { recurrenceRule } = event;
const recurrenceRuleText = screen.getByText(recurrenceRule).textContent;

expect(recurrenceRuleText).toBe(recurrenceRule);
});

it('should display attendees when `event.attendees` is exists', () => {
setup();
const { attendees } = event;
const text = attendees.join(', ');
const attendeesText = screen.getByText(text).textContent;
Expand All @@ -93,30 +109,65 @@ describe('event detail popup', () => {
});

it('should display state when `event.state` is exists', () => {
setup();
const { state } = event;
const stateText = screen.getByText(state).textContent;

expect(stateText).toBe(state);
});

it('should display calendar name when `event.calendarId` and corresponding calendar is exists', () => {
setup();
const calendarName = screen.getByText(mockCalendarName);

expect(calendarName).toBeInTheDocument();
});

it('should display body when `event.body` is exists', () => {
setup();
const { body } = event;
const bodyText = screen.getByText(body).textContent;

expect(bodyText).toBe(body);
});

it('should display edit and delete buttons when event is not read only', () => {
setup();
const editButton = screen.getByText('Edit');
const deleteButton = screen.getByText('Delete');

expect(editButton).not.toBeNull();
expect(deleteButton).not.toBeNull();
});

it('should open the form popup when edit button is clicked, while the `useFormPopup` option is enabled', async () => {
// Given
const { showFormPopupSpy } = setup({ useFormPopup: true });
const user = userEvent.setup();
const editButton = screen.getByText('Edit');

// When
await user.click(editButton);

// Then
expect(showFormPopupSpy).toHaveBeenCalledWith(expect.objectContaining({ event }));
});

it('should only fire the `beforeUpdateEvent` event when edit button is clicked, while the `useFormPopup` option is disabled', async () => {
// Given
const { eventBus } = setup({ useFormPopup: false });
const mockEventHandler = jest.fn();
eventBus.on('beforeUpdateEvent', mockEventHandler);

const user = userEvent.setup();
const editButton = screen.getByText('Edit');

// When
await user.click(editButton);

// Then
expect(mockEventHandler).toHaveBeenCalledWith(
expect.objectContaining({ event: event.toEventObject() })
);
});
});
33 changes: 20 additions & 13 deletions apps/calendar/src/components/popup/eventDetailPopup.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { useLayoutContainer } from '@src/contexts/layoutContainer';
import { cls } from '@src/helpers/css';
import { isLeftOutOfLayout, isTopOutOfLayout } from '@src/helpers/popup';
import { useCalendarColor } from '@src/hooks/calendar/useCalendarColor';
import { optionsSelector } from '@src/selectors';
import { eventDetailPopupParamSelector } from '@src/selectors/popup';
import TZDate from '@src/time/date';
import { isNil } from '@src/utils/type';
Expand Down Expand Up @@ -66,6 +67,7 @@ function calculatePopupArrowPosition(eventRect: Rect, layoutRect: Rect, popupRec
}

export function EventDetailPopup() {
const { useFormPopup } = useStore(optionsSelector);
const popupParams = useStore(eventDetailPopupParamSelector);
const { event, eventRect } = popupParams ?? {};

Expand Down Expand Up @@ -128,19 +130,24 @@ export function EventDetailPopup() {
left: eventRect.left + eventRect.width / 2,
};

const onClickEditButton = () =>
showFormPopup({
isCreationPopup: false,
event,
title,
location,
start,
end,
isAllday,
isPrivate,
eventState: state,
popupArrowPointPosition,
});
const onClickEditButton = () => {
if (useFormPopup) {
showFormPopup({
isCreationPopup: false,
event,
title,
location,
start,
end,
isAllday,
isPrivate,
eventState: state,
popupArrowPointPosition,
});
} else {
eventBus.fire('beforeUpdateEvent', { event: event.toEventObject(), changes: {} });
Copy link
Contributor

Choose a reason for hiding this comment

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

beforeUpdateEvent μ»€μŠ€ν…€μ΄λ²€νŠΈλŠ” λ­”κ°€ 이벀트λ₯Ό μ‹€μ œλ‘œ μˆ˜μ •ν•  λ•Œ μΌμ–΄λ‚˜μ•Όν•  것 같은데 Edit λ²„νŠΌμ„ λˆ„λ₯΄λ©΄ changes 없이 μ΄λ²€νŠΈκ°€ λ°œμƒν•˜λŠ”κ²Œ 쑰금 μ–΄μƒ‰ν•˜κ²Œ λŠκ»΄μ§‘λ‹ˆλ‹€. μ–΄λ–»κ²Œ μƒκ°ν•˜μ‹œλ‚˜μš”?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

일단 μ§€κΈˆ μ½”λ“œλŠ” v1 λ™μž‘κ³Ό λ™μΌν•œ μƒνƒœμž…λ‹ˆλ‹€.
μ œμ•ˆν•œλŒ€λ‘œ μˆ˜μ •ν•˜λ €λ©΄ μ €λŠ” μΈμŠ€ν„΄μŠ€ μ΄λ²€νŠΈκ°€ μƒˆλ‘œ μΆ”κ°€λ˜μ–΄μ•Όν•œλ‹€κ³  λ΄μ„œ, μ΅œλŒ€ν•œ 변경을 적게 λ§Œλ“€μ—ˆμ–΄μš”.

changes 도 μ˜΅μ…”λ„λ‘œ λ§Œλ“€κΉŒ ν–ˆλŠ”λ° 그러면 μ‹ κ²½μ¨μ•Όν• κ²Œ λ„ˆλ¬΄ λ§Žμ•„μ„œ κ·Έλƒ₯ 빈 객체λ₯Ό μ „λ‹¬ν•˜λŠ”κ±°λ‘œ λ°”κΏ¨μŠ΅λ‹ˆλ‹€.

Copy link
Contributor

Choose a reason for hiding this comment

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

λ„΅ μ•Œκ² μŠ΅λ‹ˆλ‹€!

}
};

const onClickDeleteButton = () => {
eventBus.fire('beforeDeleteEvent', event.toEventObject());
Expand Down
3 changes: 2 additions & 1 deletion docs/en/apis/calendar.md
Original file line number Diff line number Diff line change
Expand Up @@ -809,8 +809,9 @@ The `event` property is information of the existing event, and the `changes` pro

In the following cases, `beforeUpdateEvent` is executed.

- `When useFormPopup` and `useDetailPopup` of the calendar instance options are both `true` and the β€˜Update’ button is pressed after modifying the event through the event details popup
- When `useFormPopup` and `useDetailPopup` of the calendar instance options are both `true` and the β€˜Update’ button is pressed after modifying the event through the event details popup
- ![Event execution through popup](../../assets/before-update-event-1.gif)
- When the `useDetailPopup` option is `true` and the `useFormPopup` is `false`, the 'Edit' button inside the event details popup is pressed.
- When moving or resizing events by drag and drop, while `isReadOnly` is not `true` among calendar instance options and also `isReadOnly` is not `true` in the properties of individual events.
- ![Event execution via drag and drop](../../assets/before-update-event-2.gif)

Expand Down
3 changes: 2 additions & 1 deletion docs/ko/apis/calendar.md
Original file line number Diff line number Diff line change
Expand Up @@ -815,8 +815,9 @@ interface UpdatedEventInfo {

λ‹€μŒμ˜ 경우 `beforeUpdateEvent` κ°€ μ‹€ν–‰λœλ‹€.

- μΊ˜λ¦°λ” μΈμŠ€ν„΄μŠ€ μ˜΅μ…˜ 쀑 `useFormPopup` 와 `useDetailPopup` κ°€ λͺ¨λ‘ `true` 이고, 이벀트 상세 νŒμ—…μ„ 톡해 이벀트λ₯Ό μˆ˜μ • ν›„ Update λ²„νŠΌμ„ λˆ„λ₯Ό λ•Œ
- μΊ˜λ¦°λ” μΈμŠ€ν„΄μŠ€ μ˜΅μ…˜ 쀑 `useFormPopup` 와 `useDetailPopup` κ°€ λͺ¨λ‘ `true` 이고, 이벀트 상세 νŒμ—…μ—μ„œ Edit λ²„νŠΌμ„ λˆ„λ₯Έ ν›„ ν‘œμ‹œλ˜λŠ” 이벀트 폼 νŒμ—…μ—μ„œ Update λ²„νŠΌμ„ λˆ„λ₯Ό λ•Œ
- ![νŒμ—…μ„ ν†΅ν•œ 이벀트 μ‹€ν–‰](../../assets/before-update-event-1.gif)
- μΊ˜λ¦°λ”μ˜ μΈμŠ€ν„΄μŠ€ μ˜΅μ…˜ 쀑 `useDetailPopup` 이 `true` 이고, `useFormPopup` 이 `false` 일 λ•Œ, 이벀트 상세 νŒμ—…μ—μ„œ Edit λ²„νŠΌμ„ λˆ„λ₯Έ 경우
- μΊ˜λ¦°λ” μΈμŠ€ν„΄μŠ€ μ˜΅μ…˜ 쀑 `isReadOnly` κ°€ `true` κ°€ μ•„λ‹ˆλ©°, κ°œλ³„ 이벀트의 속성에 `isReadOnly` κ°€ `true` κ°€ μ•„λ‹Œ μƒνƒœμ—μ„œ λ“œλž˜κ·Έ μ•€ λ“œλžμœΌλ‘œ 일정을 μ΄λ™ν•˜κ±°λ‚˜ 리사이징할 λ•Œ
- ![λ“œλž˜κ·Έ μ•€ λ“œλžμ„ ν†΅ν•œ 이벀트 μ‹€ν–‰](../../assets/before-update-event-2.gif)

Expand Down