Skip to content

Commit 0c382c2

Browse files
tyler-daneclaude
andcommitted
refactor(web): simplify grid->sidebar drag per PR review
Addresses review feedback that the cross-zone drag was too complex and introduced too much parallel state/logic. Consolidates onto existing primitives instead of a second preview pipeline. - Reuse the native someday reorder pipeline. A grid event dragged over the sidebar is now injected into the someday snapshot (startCalendarSidebarDrag) so the existing previewSomedaySidebarDrop / getSomedayEventsAfterSidebarDrop / isDragging machinery drives the live reorder, blocked state, and drop-zone styling. Removes the parallel setCalendarSidebarDropPreview and getSomedayEventsAfterCalendarDrop. - One drag state. Deletes isCalendarDragActive; the single isDragging flag now covers grid drags too. SomedayEventsContainer keys only on isDragging. - Conversion via a mapper. Replaces the generic assembleSomedayConversionEvent (with inline RRULE regex) with MapEvent.toSomeday (concrete signature) plus a Zod-validated rewriteRecurrenceFreq helper in core. The existing "Move to Sidebar" action shares the same mapper. - Slims the week adapter: onPreviewCalendarToSidebar payload narrows to {category,index}; adds an onCancelInteraction teardown hook. - Removes the keyboard conversion feature (Shift+A/W/M draft conversion) to be reintroduced in a separate PR. The floating overlay is shrunk to a someday-row chip over the sidebar so it no longer covers the list (the visual bug that made the reorder look broken). Verified visually with a new Playwright repro (e2e/someday/drag-grid-event-to-sidebar.spec.ts): rows open a live gap, the overlay is a small chip, and a drop between lists converts to the nearest list. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent a278746 commit 0c382c2

20 files changed

Lines changed: 618 additions & 553 deletions
Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
1+
import { expect, type Page, test } from "@playwright/test";
2+
import {
3+
createEventTitle,
4+
ensureSidebarOpen,
5+
expectSomedayEventVisible,
6+
fillTitleAndSaveEventForm,
7+
openSomedayEventFormWithMouse,
8+
prepareCalendarPage,
9+
} from "../utils/event-test-utils";
10+
11+
test.skip(
12+
({ isMobile }) => isMobile,
13+
"Mouse flows are desktop-only in week view.",
14+
);
15+
16+
const sidebarButton = (page: Page, name: string) =>
17+
page.locator("#sidebar").getByRole("button", { name });
18+
19+
const centerOf = async (
20+
page: Page,
21+
name: string,
22+
scope: "sidebar" | "grid",
23+
) => {
24+
const locator =
25+
scope === "sidebar"
26+
? sidebarButton(page, name)
27+
: page.locator("#mainGrid").getByRole("button", { name });
28+
const box = await locator.boundingBox();
29+
30+
if (!box) {
31+
throw new Error(`Expected ${scope} element "${name}" to have a box.`);
32+
}
33+
34+
return { x: box.x + box.width / 2, y: box.y + box.height / 2, box };
35+
};
36+
37+
const createSomeday = async (
38+
page: Page,
39+
section: "week" | "month",
40+
prefix: string,
41+
) => {
42+
const title = createEventTitle(prefix);
43+
await openSomedayEventFormWithMouse(page, section);
44+
await fillTitleAndSaveEventForm(page, title);
45+
await expectSomedayEventVisible(page, title);
46+
return title;
47+
};
48+
49+
// Creates a TALL timed event via vertical drag-select and waits for its
50+
// optimistic/pending state to settle, so it can be reliably grabbed for a drag
51+
// (a short, freshly-created event lands on the resize handle / refuses to drag).
52+
const createTimed = async (page: Page, prefix: string) => {
53+
const title = createEventTitle(prefix);
54+
const grid = await page.locator("#mainGrid").boundingBox();
55+
56+
if (!grid) throw new Error("Expected the week grid to be visible.");
57+
58+
const gx = grid.x + grid.width * 0.4;
59+
const gy = grid.y + grid.height * 0.25;
60+
61+
await page.mouse.move(gx, gy);
62+
await page.mouse.down();
63+
await page.mouse.move(gx, gy + 160, { steps: 12 });
64+
await page.mouse.up();
65+
await fillTitleAndSaveEventForm(page, title);
66+
67+
await expect(
68+
page.locator("#mainGrid").getByRole("button", { name: title }),
69+
).toBeVisible({ timeout: 8000 });
70+
await page.waitForTimeout(2500);
71+
72+
return title;
73+
};
74+
75+
test("drags a timed grid event into the Someday week list and reorders rows live", async ({
76+
page,
77+
}) => {
78+
await prepareCalendarPage(page);
79+
await ensureSidebarOpen(page);
80+
81+
const weekA = await createSomeday(page, "week", "Week A");
82+
const weekB = await createSomeday(page, "week", "Week B");
83+
const timed = await createTimed(page, "Timed T");
84+
85+
const start = await centerOf(page, timed, "grid");
86+
const a = await centerOf(page, weekA, "sidebar");
87+
const b = await centerOf(page, weekB, "sidebar");
88+
89+
// Aim for the seam between A and B in the week list.
90+
const target = { x: a.x, y: (a.y + b.y) / 2 };
91+
92+
await page.mouse.move(start.x, start.y);
93+
await page.mouse.down();
94+
await page.mouse.move(target.x, target.y, { steps: 16 });
95+
await page.waitForTimeout(250);
96+
97+
await page.screenshot({
98+
path: "test-results/repro/week-list-mid-drag.png",
99+
});
100+
101+
// The dragged event should appear in the sidebar between A and B (the existing
102+
// rows opened a gap). The dragged row drops its `button` role, so match text.
103+
const placeholder = page
104+
.locator("#sidebar")
105+
.getByText(timed, { exact: false });
106+
await expect(placeholder).toBeVisible();
107+
108+
const placeholderBox = await placeholder.boundingBox();
109+
const aBox = await sidebarButton(page, weekA).boundingBox();
110+
const bBox = await sidebarButton(page, weekB).boundingBox();
111+
112+
await page.mouse.up();
113+
114+
expect(placeholderBox).not.toBeNull();
115+
expect(aBox).not.toBeNull();
116+
expect(bBox).not.toBeNull();
117+
118+
if (placeholderBox && aBox && bBox) {
119+
const placeholderMid = placeholderBox.y + placeholderBox.height / 2;
120+
const aMid = aBox.y + aBox.height / 2;
121+
const bMid = bBox.y + bBox.height / 2;
122+
123+
expect(aMid).toBeLessThan(placeholderMid);
124+
expect(placeholderMid).toBeLessThan(bMid);
125+
}
126+
});
127+
128+
test("snaps to the nearest Someday list when dropped between the lists", async ({
129+
page,
130+
}) => {
131+
await prepareCalendarPage(page);
132+
await ensureSidebarOpen(page);
133+
134+
const weekA = await createSomeday(page, "week", "Week A");
135+
const monthM = await createSomeday(page, "month", "Month M");
136+
const timed = await createTimed(page, "Timed T");
137+
138+
const start = await centerOf(page, timed, "grid");
139+
const a = await centerOf(page, weekA, "sidebar");
140+
const m = await centerOf(page, monthM, "sidebar");
141+
142+
// A point in the empty space between the week list (above) and the month
143+
// list (below), biased toward the month list so "nearest" should be Month.
144+
const gapTarget = { x: a.x, y: m.box.y - 8 };
145+
146+
await page.mouse.move(start.x, start.y);
147+
await page.mouse.down();
148+
await page.mouse.move(gapTarget.x, gapTarget.y, { steps: 16 });
149+
await page.waitForTimeout(250);
150+
151+
await page.screenshot({
152+
path: "test-results/repro/between-lists-mid-drag.png",
153+
});
154+
155+
await page.mouse.up();
156+
await page.waitForTimeout(400);
157+
158+
await page.screenshot({
159+
path: "test-results/repro/between-lists-after-drop.png",
160+
});
161+
162+
// It should have converted to a Someday event (nearest = Month), not jumped
163+
// back onto the grid.
164+
await expect(
165+
page.locator("#mainGrid").getByRole("button", { name: timed }),
166+
).toHaveCount(0, { timeout: 8000 });
167+
await expect(sidebarButton(page, timed)).toBeVisible({ timeout: 8000 });
168+
});

packages/core/src/mappers/map.event.test.ts

Lines changed: 71 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { ObjectId } from "bson";
2+
import { Origin, Priorities } from "@core/constants/core.constants";
23
import { MapEvent } from "@core/mappers/map.event";
3-
import { type Schema_Event } from "@core/types/event.types";
4+
import { Categories_Event, type Schema_Event } from "@core/types/event.types";
45
import {
56
createMockBaseEvent,
67
createMockInstance,
@@ -25,3 +26,72 @@ describe("MapEvent.removeProviderData", () => {
2526
expect((result as Schema_Event).recurrence?.eventId).toBeUndefined();
2627
});
2728
});
29+
30+
describe("MapEvent.toSomeday", () => {
31+
const baseEvent: Schema_Event = {
32+
_id: "event-1",
33+
title: "Grid event",
34+
startDate: "2024-03-19T10:00:00.000Z",
35+
endDate: "2024-03-19T11:00:00.000Z",
36+
isAllDay: false,
37+
isSomeday: false,
38+
origin: Origin.COMPASS,
39+
priority: Priorities.WORK,
40+
user: "user-1",
41+
};
42+
43+
it("maps a calendar event into a someday payload with the given dates", () => {
44+
const result = MapEvent.toSomeday(baseEvent, {
45+
category: Categories_Event.SOMEDAY_WEEK,
46+
endDate: "2024-03-23",
47+
order: 2,
48+
startDate: "2024-03-17",
49+
});
50+
51+
expect(result).toEqual(
52+
expect.objectContaining({
53+
_id: "event-1",
54+
endDate: "2024-03-23",
55+
isAllDay: false,
56+
isSomeday: true,
57+
order: 2,
58+
priority: Priorities.WORK,
59+
startDate: "2024-03-17",
60+
title: "Grid event",
61+
}),
62+
);
63+
});
64+
65+
it("rewrites the recurrence FREQ for the destination list", () => {
66+
const result = MapEvent.toSomeday(
67+
{ ...baseEvent, recurrence: { rule: ["RRULE:FREQ=DAILY;COUNT=5"] } },
68+
{
69+
category: Categories_Event.SOMEDAY_MONTH,
70+
endDate: "2024-03-31",
71+
order: 0,
72+
startDate: "2024-03-01",
73+
},
74+
);
75+
76+
expect(result.recurrence?.rule).toEqual(["RRULE:FREQ=MONTHLY;COUNT=5"]);
77+
});
78+
79+
it("defaults missing priority and user", () => {
80+
const result = MapEvent.toSomeday(
81+
{
82+
...baseEvent,
83+
priority: undefined,
84+
user: undefined as unknown as string,
85+
},
86+
{
87+
category: Categories_Event.SOMEDAY_WEEK,
88+
endDate: "2024-03-23",
89+
order: 0,
90+
startDate: "2024-03-17",
91+
},
92+
);
93+
94+
expect(result.priority).toBe(Priorities.UNASSIGNED);
95+
expect(result.user).toBe("");
96+
});
97+
});

packages/core/src/mappers/map.event.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@
33
import mergeWith from "lodash.mergewith";
44
import { Origin, Priorities } from "@core/constants/core.constants";
55
import { BaseError } from "@core/errors/errors.base";
6+
import { rewriteRecurrenceFreq } from "@core/mappers/map.recurrence";
67
import {
78
CalendarProvider,
9+
type Categories_Event,
810
type Event_Core,
911
type Schema_Event,
1012
type Schema_Event_Recur_Base,
@@ -78,6 +80,31 @@ export namespace MapEvent {
7880
return coreEvent;
7981
};
8082

83+
export const toSomeday = (
84+
event: Schema_Event,
85+
{
86+
category,
87+
endDate,
88+
order,
89+
startDate,
90+
}: {
91+
category: Categories_Event.SOMEDAY_WEEK | Categories_Event.SOMEDAY_MONTH;
92+
endDate: string;
93+
order: number;
94+
startDate: string;
95+
},
96+
): Schema_Event => ({
97+
...event,
98+
endDate,
99+
isAllDay: false,
100+
isSomeday: true,
101+
order,
102+
priority: event.priority ?? Priorities.UNASSIGNED,
103+
recurrence: rewriteRecurrenceFreq(event.recurrence, category),
104+
startDate,
105+
user: event.user ?? "",
106+
});
107+
81108
export const toGcal = (
82109
event: Schema_Event,
83110
{ status = "confirmed" }: Pick<gSchema$Event, "status"> = {},
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
import { rewriteRecurrenceFreq } from "@core/mappers/map.recurrence";
2+
import { Categories_Event } from "@core/types/event.types";
3+
4+
describe("rewriteRecurrenceFreq", () => {
5+
it("rewrites FREQ to WEEKLY for the week list", () => {
6+
const result = rewriteRecurrenceFreq(
7+
{ rule: ["RRULE:FREQ=DAILY;COUNT=10;INTERVAL=1"] },
8+
Categories_Event.SOMEDAY_WEEK,
9+
);
10+
11+
expect(result?.rule).toEqual(["RRULE:FREQ=WEEKLY;COUNT=10;INTERVAL=1"]);
12+
});
13+
14+
it("rewrites FREQ to MONTHLY for the month list", () => {
15+
const result = rewriteRecurrenceFreq(
16+
{ rule: ["RRULE:FREQ=WEEKLY;COUNT=10"] },
17+
Categories_Event.SOMEDAY_MONTH,
18+
);
19+
20+
expect(result?.rule).toEqual(["RRULE:FREQ=MONTHLY;COUNT=10"]);
21+
});
22+
23+
it("rewrites FREQ with no trailing tokens", () => {
24+
const result = rewriteRecurrenceFreq(
25+
{ rule: ["RRULE:FREQ=DAILY"] },
26+
Categories_Event.SOMEDAY_WEEK,
27+
);
28+
29+
expect(result?.rule).toEqual(["RRULE:FREQ=WEEKLY"]);
30+
});
31+
32+
it("leaves non-RRULE lines untouched", () => {
33+
const result = rewriteRecurrenceFreq(
34+
{ rule: ["RRULE:FREQ=DAILY", "EXDATE:20240101T000000Z"] },
35+
Categories_Event.SOMEDAY_WEEK,
36+
);
37+
38+
expect(result?.rule).toEqual([
39+
"RRULE:FREQ=WEEKLY",
40+
"EXDATE:20240101T000000Z",
41+
]);
42+
});
43+
44+
it("preserves the recurrence eventId", () => {
45+
const result = rewriteRecurrenceFreq(
46+
{ rule: ["RRULE:FREQ=DAILY"], eventId: "abc" },
47+
Categories_Event.SOMEDAY_WEEK,
48+
);
49+
50+
expect(result?.eventId).toBe("abc");
51+
});
52+
53+
it("returns the recurrence unchanged when there is no rule", () => {
54+
expect(
55+
rewriteRecurrenceFreq(undefined, Categories_Event.SOMEDAY_WEEK),
56+
).toBeUndefined();
57+
expect(
58+
rewriteRecurrenceFreq({ rule: null }, Categories_Event.SOMEDAY_WEEK),
59+
).toEqual({ rule: null });
60+
});
61+
});

0 commit comments

Comments
 (0)