[STYLE] Create custom scrollbar design#41
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a custom scrollbar theme (light/dark token + global CSS) and significantly expands the Cycles feature across UI and API (CRUD, filtering/events, favorites, and server-derived status).
Changes:
- Add global custom scrollbar styling + tokens, with a main-content override for invisible scrollbars.
- Expand Cycles UI: active-cycle summary/stats, filtering/search via cross-route events, favorites, and create/update/delete modals.
- Backend cycle improvements: accept RFC3339/date-only inputs and derive cycle status from dates on read/write.
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| ui/src/styles/tokens.css | Adds scrollbar CSS variables for light/dark themes. |
| ui/src/index.css | Implements global custom scrollbar styling and updates main-content override. |
| ui/src/services/cycleService.ts | Adds cycles CRUD + listIssueIds; aligns client calls with backend routes. |
| ui/src/pages/ModulesPage.tsx | Minor UI tweak to module title styling. |
| ui/src/pages/CyclesPage.tsx | Major Cycles page redesign: filtering, favorites, stats, menus, modals wiring. |
| ui/src/lib/projectCyclesEvents.ts | Defines window event names for cycles filter/refresh cross-route sync. |
| ui/src/lib/dateOnly.ts | Adds parseISODateForDisplay helper for date-only display semantics. |
| ui/src/hooks/useCycleFavorites.ts | Adds localStorage-backed cycle favorites with a change event. |
| ui/src/components/layout/Sidebar.tsx | Displays favorite cycles in sidebar and reacts to favorites change events. |
| ui/src/components/layout/PageHeader.tsx | Adds cycles filters/search UI and “Create cycle” modal trigger; dispatches filter/refresh events. |
| ui/src/components/layout/AppShell.tsx | Adjusts page padding for cycles route to match layout changes. |
| ui/src/components/UpdateCycleModal.tsx | New modal for editing cycle name/description/date range. |
| ui/src/components/CreateCycleModal.tsx | New modal for creating cycles (including project selection + date range). |
| ui/package.json | Bumps UI package version to 0.5.0. |
| ui/package-lock.json | Lockfile version bump to match package.json. |
| api/internal/service/cycle.go | Adds server-side status computation and applies it on list/get/create/update. |
| api/internal/handler/cycle.go | Extends date parsing to accept RFC3339 timestamps or YYYY-MM-DD strings. |
Files not reviewed (1)
- ui/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const ranges: Array<{ start: number; end: number }> = []; | ||
| for (const p of selectedPresets) { | ||
| if (p === 'custom') { | ||
| if (!customAfter || !customBefore) continue; | ||
| const a = parseISODateLocal(customAfter); | ||
| const b = parseISODateLocal(customBefore); | ||
| const aMs = new Date(a.getFullYear(), a.getMonth(), a.getDate()).getTime(); | ||
| const bMs = new Date(b.getFullYear(), b.getMonth(), b.getDate()).getTime(); | ||
| ranges.push({ start: Math.min(aMs, bMs), end: Math.max(aMs, bMs) }); | ||
| } else { | ||
| const days = presetDays[p]; | ||
| if (days == null) continue; | ||
| ranges.push({ | ||
| start: startOfToday, | ||
| end: startOfToday + days * 24 * 60 * 60 * 1000, | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| if (ranges.length === 0) return false; | ||
| return ranges.some((r) => inRange(date, r.start, r.end)); | ||
| }; |
There was a problem hiding this comment.
When only the custom preset is selected but customAfter/customBefore are still null (e.g., user checks “Custom” and then cancels the date modal), matchesPresetUnion ends up with ranges.length === 0 and returns false, filtering out all cycles. Either treat “custom with no dates yet” as no-op in matchesPresetUnion, or ensure the UI never leaves custom selected without a range.
| if (p.key === 'custom') { | ||
| if (checked) { | ||
| setCyclesSelectedStartDatePresets((prev) => | ||
| prev.filter((k) => k !== 'custom'), | ||
| ); | ||
| setCyclesStartAfter(null); | ||
| setCyclesStartBefore(null); | ||
| } else { | ||
| setCyclesSelectedStartDatePresets((prev) => [...prev, 'custom']); | ||
| setCyclesFiltersDropdownOpen(null); | ||
| setCyclesDateRangeModal('start'); | ||
| } | ||
| return; |
There was a problem hiding this comment.
Selecting the “Custom” start-date preset immediately adds 'custom' to cyclesSelectedStartDatePresets before the date modal is confirmed. If the user cancels the modal, the filter state can remain as ['custom'] with cyclesStartAfter/Before still null, which (with the current filter logic) filters out all cycles. Consider only adding 'custom' after DateRangeModal apply, or removing 'custom' in the modal onClose when no dates were set.
| if (p.key === 'custom') { | ||
| if (checked) { | ||
| setCyclesSelectedDueDatePresets((prev) => | ||
| prev.filter((k) => k !== 'custom'), | ||
| ); | ||
| setCyclesDueAfter(null); | ||
| setCyclesDueBefore(null); | ||
| } else { | ||
| setCyclesSelectedDueDatePresets((prev) => [...prev, 'custom']); | ||
| setCyclesFiltersDropdownOpen(null); | ||
| setCyclesDateRangeModal('due'); | ||
| } | ||
| return; |
There was a problem hiding this comment.
Same issue as the start-date “Custom” preset: 'custom' is added to cyclesSelectedDueDatePresets before the user confirms a range. If the modal is cancelled, the filter can end up with ['custom'] but no cyclesDueAfter/Before, which filters out all cycles. Consider deferring adding 'custom' until apply, or clearing it on modal cancel when no dates are set.
| function formatDateRangeDisplay(start: string | null, end: string | null): string { | ||
| if (!start && !end) return 'Start date → End date'; | ||
| if (start && end) return `${formatISODateDisplay(start)} → ${formatISODateDisplay(end)}`; | ||
| return start | ||
| ? formatISODateDisplay(start) | ||
| : end | ||
| ? formatISODateDisplay(end) | ||
| : 'Start date → End date'; |
There was a problem hiding this comment.
formatISODateDisplay() is used to render cycle start/end ranges here, but cycles come back from the API as RFC3339 timestamps (time.Time JSON). formatISODateDisplay currently falls back to new Date(iso) for non-YYYY-MM-DD strings, which can display the wrong day in non-UTC timezones. Consider switching these labels to use the new parseISODateForDisplay logic (or updating formatISODateDisplay to use it) so cycles created/stored as midnight-UTC don't show off-by-one dates.
| function formatDateRangeDisplay(start: string | null, end: string | null): string { | ||
| if (!start && !end) return 'Start date → End date'; | ||
| if (start && end) return `${formatISODateDisplay(start)} → ${formatISODateDisplay(end)}`; | ||
| return start | ||
| ? formatISODateDisplay(start) | ||
| : end | ||
| ? formatISODateDisplay(end) | ||
| : 'Start date → End date'; |
There was a problem hiding this comment.
Same timezone/off-by-one risk as in the create modal: the date range button uses formatISODateDisplay(start/end) even though cycle.start_date/end_date are RFC3339 timestamps from the API. This can show the previous/next day depending on the user timezone. Prefer using parseISODateForDisplay (or a formatter built on it) for cycle dates.
| import { useEffect, useMemo, useState } from 'react'; | ||
| import { Link, useParams } from 'react-router-dom'; | ||
| import { Avatar } from '../components/ui'; | ||
| import { Avatar, Badge, Button, Modal } from '../components/ui'; | ||
| import { UpdateCycleModal } from '../components/UpdateCycleModal'; | ||
| import { workspaceService } from '../services/workspaceService'; | ||
| import { projectService } from '../services/projectService'; | ||
| import { cycleService } from '../services/cycleService'; | ||
| import type { WorkspaceApiResponse, ProjectApiResponse, CycleApiResponse } from '../api/types'; | ||
| import { issueService } from '../services/issueService'; | ||
| import { stateService } from '../services/stateService'; | ||
| import { labelService } from '../services/labelService'; | ||
| import type { | ||
| WorkspaceApiResponse, | ||
| ProjectApiResponse, | ||
| CycleApiResponse, | ||
| WorkspaceMemberApiResponse, | ||
| IssueApiResponse, | ||
| StateApiResponse, | ||
| LabelApiResponse, | ||
| } from '../api/types'; | ||
| import type { Priority } from '../types'; | ||
| import { | ||
| PROJECT_CYCLES_FILTER_EVENT, | ||
| PROJECT_CYCLES_REFRESH_EVENT, | ||
| } from '../lib/projectCyclesEvents'; | ||
| import { useCycleFavorites } from '../hooks/useCycleFavorites'; | ||
| import { parseISODateForDisplay, parseISODateLocal } from '../lib/dateOnly'; | ||
| import { cn, getImageUrl } from '../lib/utils'; |
There was a problem hiding this comment.
The PR title/description indicates a scrollbar styling change, but this diff also adds/changes substantial cycles functionality (cycles CRUD, filtering UI/events, favorites, backend status computation, version bump). Consider updating the PR title/description (or splitting into smaller PRs) so reviewers can scope and validate the behavioral changes appropriately.
| func computeCycleStatus(start, end *time.Time) string { | ||
| now := time.Now() | ||
| if start == nil && end == nil { | ||
| return "draft" | ||
| } | ||
| if end != nil && end.Before(now) { | ||
| return "completed" | ||
| } | ||
| if start != nil && end != nil { | ||
| if !now.Before(*start) && !now.After(*end) { | ||
| return "current" | ||
| } | ||
| if start.After(now) { | ||
| return "upcoming" | ||
| } | ||
| } | ||
| if start != nil && start.After(now) { | ||
| return "upcoming" | ||
| } | ||
| if start != nil && !start.After(now) && (end == nil || !end.Before(now)) { | ||
| return "current" | ||
| } | ||
| return "upcoming" |
There was a problem hiding this comment.
computeCycleStatus returns upcoming when start == nil and end is in the future (end-only cycles). That makes a cycle with only a due/end date show as not started even though it should be considered current/in progress until the end passes. Consider handling the start == nil && end != nil case explicitly (e.g., treat as current when end is after now).
| // Empty selection means "no filtering" (match Plane behavior). | ||
| if (selectedPresets.length === 0) return true; | ||
| if (!dateIso) return false; | ||
|
|
||
| const date = parseISODateLocal(dateIso); | ||
|
|
||
| const ranges: Array<{ start: number; end: number }> = []; | ||
| for (const p of selectedPresets) { | ||
| if (p === 'custom') { | ||
| if (!customAfter || !customBefore) continue; | ||
| const a = parseISODateLocal(customAfter); | ||
| const b = parseISODateLocal(customBefore); | ||
| const aMs = new Date(a.getFullYear(), a.getMonth(), a.getDate()).getTime(); |
There was a problem hiding this comment.
matchesPresetUnion parses dateIso with parseISODateLocal(dateIso), which falls back to new Date(dateIso) for RFC3339 timestamps. Since the API serializes start_date/end_date as RFC3339, this can introduce timezone off-by-one errors in the date filtering. Prefer extracting the YYYY-MM-DD portion (e.g., via parseISODateForDisplay) before comparing day ranges.
Closes #28
Closes #40