Skip to content

Plan editor UX: time picker, day-pager polish, day-range mismatch banner#31

Open
infinitea1 wants to merge 5 commits into
samrford:masterfrom
infinitea1:plan-scratchpad-copy
Open

Plan editor UX: time picker, day-pager polish, day-range mismatch banner#31
infinitea1 wants to merge 5 commits into
samrford:masterfrom
infinitea1:plan-scratchpad-copy

Conversation

@infinitea1
Copy link
Copy Markdown
Contributor

Summary

Stacked on top of # (fix-tilt-cross-platform). When that
merges, GitHub will auto-retarget this to master.

Four small, independent changes to the plan editor — split into one commit
each so anything can be reverted on its own:

  1. feat(plan): clarify scratchpad drop-zone copy — replaces "Drop items
    here to unassign them" with clearer wording about removing items from a day.
    "Unassign" was jargon that didn't explain the actual interaction.
  2. feat(plan-item): two-column time picker with native fallback — new
    TimePicker component with hour + minute (5-min) columns in a portal
    popover. The trigger keeps a native <input type="time"> so typing still
    works and Firefox users (whose native input has no clock UI) finally have
    a clickable picker.
  3. feat(plan-itinerary): improve day-pager UX — adds "Day X of N" count,
    restyled circular chevrons, and lifts the day header out of each carousel
    card into a single shared bar above the scrollable container
    . Most
    opinionated change in this PR — the chevrons now stay in a fixed screen
    position regardless of scroll, so rapid clicks always register.
    Also replaces the 800ms-timeout flag-flip in the scroll-sync effect with
    a precise scrollend listener (with a 1500ms timer fallback).
  4. feat(plan-itinerary): surface day-range mismatch banner — when the
    plan's start/end dates don't match its existing days (e.g. user edited
    dates after generating), an always-on banner offers "Add N days" and
    "Remove N (X items → scratchpad)". Items on removed days move to the
    scratchpad automatically thanks to the existing ON DELETE SET NULL.

Test plan

  • Open a plan in Firefox and edit an activity's start time — verify the
    picker popover opens, both columns scroll, typing into the input still
    works.
  • Open a plan with multiple days — confirm the header reads "Day X of N",
    chevrons cycle correctly, disabled state on Day 1 / Day N is obvious.
  • Spam-click the next-day chevron — should slide smoothly all the way to
    the last day with no wobble or stuck-chevron state.
  • Edit a plan's date range to be longer than its existing days → banner
    appears with "Add N days" → click adds only the missing dates.
  • Edit a plan's date range to be shorter, with items on the now-orphan
    days → banner says "Remove N (X items → scratchpad)" → click removes
    the days and items appear in the scratchpad.

infinitea1 and others added 5 commits May 2, 2026 22:32
The current serve_cmd uses bash-style env-var prefix and line continuations
(KEY="value" \). That works on macOS where Tilt runs commands through bash,
but on Windows Tilt invokes serve_cmd via `cmd /S /C`, which doesn't parse
bash continuations or env prefixes — so `tilt up` can't boot the backend on
a Windows machine.

Switch to Tilt's serve_env={...} dict (platform-agnostic env wiring) and
swap the pre-built bin/server invocation for `go run ./cmd/server` (no
OS-specific binary suffix). Same env vars, same behaviour on macOS — also
works on Windows now.

Also gitignore CLAUDE.md / CLAUDE.local.md / .claude/ for personal
agent-tooling files.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace "Drop items here to unassign them" with clearer wording about
removing items from a day, since "unassign" is jargon that doesn't
explain the actual interaction to users.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace the bare native time input on plan items with a custom
TimePicker that opens a portal-based popover with separate hour
and minute (5-min granularity) columns. The trigger keeps a
native time input so typing still works and Firefox users — who
get no clock UI from the native input — finally have a clickable
picker.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Add "Day X of N" count to the day header so users see how many days
  exist at a glance.
- Replace the prev/next chevron buttons with circular tinted controls
  whose disabled state reads as inactive (transparent bg, 30% opacity).
- Lift the day header out of the per-card carousel into a single shared
  bar above the scrollable container. The chevrons now stay in a fixed
  screen position regardless of scroll progress, so rapid clicks always
  register and the carousel slides cleanly underneath.
- Replace the 800ms-timeout flag-flip in the scroll-sync effect with a
  precise scrollend listener (with a 1500ms safety-net timeout for
  browsers that don't support scrollend). Eliminates a race where the
  onScroll listener could mid-animation set selectedDayId to a different
  day, causing chevrons to feel stuck.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When the existing plan-days don't match the plan's start/end dates
(e.g. user edited the dates after generating days), show an always-on
banner above the itinerary with two actions:

- "Add N days" — POSTs only the missing dates to /v1/plans/:id/days,
  leaving any existing days untouched.
- "Remove N (X items → scratchpad)" — DELETEs the orphan days. Items
  on those days move to the scratchpad automatically thanks to the
  ON DELETE SET NULL on plan_items.plan_day_id; the button label tells
  the user how many items will move.

The empty-state "+ Generate Days from Dates" button now routes through
the same handler (handleAddMissingDays), which generalises cleanly
since "all dates missing" is just one case of the partial mismatch.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Would be good to use this new component inside DateTimePicker (replacing the time part with this - that we we have the same timepicker component across the app

for (const date of days) {
const newDay = await apiFetch<PlanDay>(`/v1/plans/${id}/days`, {
for (const key of missingDateKeys) {
await apiFetch<PlanDay>(`/v1/plans/${id}/days`, {
Copy link
Copy Markdown
Owner

@samrford samrford May 4, 2026

Choose a reason for hiding this comment

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

I think we should use Promise.allSettled here so all requests fire in parallel rather than sequentially, and you can report what actually happened.

I think we should add a little helper for retrying in api.ts so we don't just fail outright if there's an error in any of the requests:

// in lib/api.ts (next to apiFetch):
 export async function withRetry<T>(
   fn: () => Promise<T>,
   { retryCount = 3, baseDelayMs = 300 } = {},
 ): Promise<T> {
   let lastErr: unknown;
   for (let i = 0; i <= retryCount; i++) {
     try {
       return await fn();
     } catch (e) {
       lastErr = e;
       // Don't retry 4xx — they won't get better on retry.
       if (e instanceof ApiError && e.status >= 400 && e.status < 500) throw e;
       if (i < retryCount) {
         await new Promise((r) => setTimeout(r, baseDelayMs * 2 ** i));
       }
     }
   }
   throw lastErr;
 }

Then the handler in this file becomes:

const results = await Promise.allSettled(
  missingDateKeys.map((key) =>
    withRetry(() =>
      apiFetch<PlanDay>(`/v1/plans/${id}/days`, {
        method: "POST",
        body: JSON.stringify({ date: `${key}T00:00:00Z` }),
      }),
    ),
  ),
);
queryClient.invalidateQueries({ queryKey: planKeys.detail(id) });

const ok = results.filter((r) => r.status === "fulfilled").length;
const failed = results.length - ok;
if (failed === 0) {
  toast.success(`Added ${ok} day${ok === 1 ? "" : "s"}`);
} else if (ok === 0) {
  toast.error("Failed to add missing days");
} else {
  toast.error(`Added ${ok} of ${results.length} days — ${failed} failed`);
}

Same shape applies to handleDeleteOrphanDays below

) : (
<div className="flex flex-col items-center justify-center h-full py-4 text-center pointer-events-none">
<p className="text-gray-500 dark:text-gray-400 text-sm">{isOwner ? "Drop items here to unassign them, or add new ideas!" : "No unassigned items."}</p>
<p className="text-gray-500 dark:text-gray-400 text-sm">{isOwner ? "Drag items here to remove them from a day, or add new ideas!" : "No unassigned items."}</p>
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Suggested change
<p className="text-gray-500 dark:text-gray-400 text-sm">{isOwner ? "Drag items here to remove them from a day, or add new ideas!" : "No unassigned items."}</p>
<p className="text-gray-500 dark:text-gray-400 text-sm">{isOwner ? "Drag items here to unassign them, or add new ideas by clicking the button above!" : "No unassigned items."}</p>

I like the word unassign here :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants