Refactor foundation: split monoliths into sections, shared hooks/utilities, and docs alignment#10
Refactor foundation: split monoliths into sections, shared hooks/utilities, and docs alignment#10
Conversation
There was a problem hiding this comment.
Pull request overview
Refactors large, page-local logic into shared utilities/hooks and decomposes several screens into smaller, reusable section components, while updating docs to match the new organization.
Changes:
- Extracted shared pure utilities (schedule, analytics, marker math, zoom/pan clamping) and reusable hooks (schedule generation, team form, zoom/pan, call-tracking state).
- Decomposed dispatch call-tracking UI and event/venue pages into smaller components (sections/widgets/controls).
- Updated architecture/components documentation to reflect the new structure.
Reviewed changes
Copilot reviewed 33 out of 33 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| src/lib/zoomPanUtils.ts | New shared zoom/pan clamp helpers. |
| src/lib/uploadUtils.ts | New shared Firebase Storage upload-with-retry helper. |
| src/lib/scheduleUtils.ts | New shared schedule time parsing/formatting + posting-time generation. |
| src/lib/markerUtils.ts | New shared marker hit-testing and pixel→percent conversion utilities. |
| src/lib/liteEventAdapters.ts | New adapters for converting lite drafts ↔ full Event shape. |
| src/lib/analyticsUtils.ts | New extracted analytics helpers for summary charts (window/series/team pie). |
| src/hooks/useZoomPan.ts | New reusable zoom/pan hook using shared clamp utils. |
| src/hooks/useTeamForm.ts | New reusable hook for team modal/form state. |
| src/hooks/useScheduleGeneration.ts | New reusable hook for generating posting times from inputs. |
| src/hooks/useCallTrackingState.ts | New reusable hook for call-tracking notes/log/pending UI state. |
| src/components/venue-management/PendingMarkerDialog.tsx | Extracted pending marker naming UI into a component. |
| src/components/venue-management/MarkerPlacementInstruction.tsx | Extracted marker placement instruction overlay. |
| src/components/venue-management/MarkerModeToggleButton.tsx | Extracted “add markers” mode toggle button. |
| src/components/venue-management/LayerControlBar.tsx | Extracted bottom layer/map control bar for venue editor. |
| src/components/venue-management/EquipmentManagementSection.tsx | Extracted equipment CRUD UI for venue editor. |
| src/components/ui/portal-dropdown.tsx | New shared portal-based dropdown anchored to an element. |
| src/components/ui/map-zoom-controls.tsx | New shared zoom/reset button cluster for map viewers. |
| src/components/ui/map-pan-surface.tsx | New shared pan/wheel event surface wrapper. |
| src/components/event-create/TeamStaffingSection.tsx | Extracted team staffing tab section from event create page. |
| src/components/event-create/SupervisorStaffingSection.tsx | Extracted supervisor staffing tab section from event create page. |
| src/components/event-create/PostsEquipmentSection.tsx | Extracted posts + equipment selection sections for event create. |
| src/components/event-create/PostingScheduleSection.tsx | Extracted posting schedule UI for event create. |
| src/components/event-create/MetadataSection.tsx | Extracted event metadata (name/date) section. |
| src/components/dispatch/teamwidget.tsx | Extracted TeamWidget wrapper for normal/condensed team cards. |
| src/components/dispatch/calltrackingdetails.tsx | Extracted expanded call details (notes/log) row UI. |
| src/components/dispatch/calltracking.tsx | Updated call tracking table to use extracted hook + details component. |
| src/app/lite/create/page.tsx | Replaced page-local schedule/team-form logic with shared utils/hooks. |
| src/app/(main)/venues/management/page.client.tsx | Updated venue management page to use extracted sections + shared map utils/hooks. |
| src/app/(main)/events/[eventId]/summary/page.tsx | Replaced page-local analytics helpers with shared analytics utils. |
| src/app/(main)/events/[eventId]/dispatch/page.tsx | Updated dispatch page to use extracted lite adapters + TeamWidget; removed inlined helpers. |
| src/app/(main)/events/[eventId]/create/page.tsx | Decomposed event create page into section components and shared map controls/hooks. |
| docs/COMPONENTS.md | Documentation updates for new component organization and shared UI primitives. |
| docs/ARCHITECTURE.md | Documentation updates to reflect new folders and decomposition guidance. |
src/lib/zoomPanUtils.ts
Outdated
| const { newX, newY, imgWidth, imgHeight, containerWidth, containerHeight, scale } = params; | ||
|
|
||
| const maxX = Math.max(0, (imgWidth - containerWidth) / scale); | ||
| const maxY = Math.max(0, (imgHeight - containerHeight) / scale); | ||
|
|
||
| return { | ||
| x: Math.min(0, Math.max(-maxX, newX)), | ||
| y: Math.min(0, Math.max(-maxY, newY)), |
There was a problem hiding this comment.
clampPanPosition computes maxX/maxY as (imgWidth - containerWidth) / scale, but callers pass imgWidth = offsetWidth * scale and then apply transform: scale(scale) translate(position/scale) (so position is in screen px). Dividing by scale makes the allowed pan range too small at higher zoom (you can’t pan to the image edges). Consider clamping against the scaled overflow directly: maxX = Math.max(0, imgWidth - containerWidth) (and same for Y).
| const isRetryable = | ||
| code === 'storage/retry-limit-exceeded' || | ||
| code === 'storage/unknown' || | ||
| code === 'storage/canceled' || | ||
| code === 'storage/quota-exceeded' || | ||
| code === 'storage/unauthenticated'; | ||
|
|
||
| if (attempt < maxRetries - 1 && isRetryable) { | ||
| const wait = baseDelay * Math.pow(2, attempt); | ||
| await new Promise((res) => setTimeout(res, wait)); | ||
| continue; | ||
| } | ||
|
|
||
| throw new Error('Upload failed'); | ||
| } |
There was a problem hiding this comment.
uploadWithRetry retries on error codes like storage/canceled, storage/unauthenticated, and storage/quota-exceeded, which are typically not transient and can add unnecessary delay before surfacing the real failure. Also, rethrowing new Error('Upload failed') drops the underlying Firebase error/code; consider rethrowing the original error or including the code/message in the thrown error so callers can display/diagnose it.
| <tr> | ||
| <td | ||
| colSpan={7} | ||
| className="p-2 border-b border-surface-liner" | ||
| onClick={onClose} | ||
| > |
There was a problem hiding this comment.
This details row previously inherited the call row styling via getCallRowClass(call), but the extracted component hardcodes the <td> className and no longer applies any status-based row class. If row coloring is intentional for expanded rows, pass a rowClassName/callRowClass prop (or similar) and apply it to the <td>/<tr> to preserve the existing visuals.
| <Button isIconOnly size="sm" variant="flat" onPress={onZoomIn} className={buttonClassName}> | ||
| <ZoomIn className="h-4 w-4" /> | ||
| </Button> | ||
| <Button isIconOnly size="sm" variant="flat" onPress={onZoomOut} className={buttonClassName}> | ||
| <ZoomOut className="h-4 w-4" /> | ||
| </Button> | ||
| </ButtonGroup> | ||
| <Button size="sm" variant="flat" onPress={onReset} className={resetButtonClassName}> |
There was a problem hiding this comment.
MapZoomControls renders icon-only buttons (ZoomIn/ZoomOut and the reset icon) without accessible labels. Add aria-label (or title) to each control so screen readers can announce their purpose; this is especially important since the reset button has no visible text.
| <Button isIconOnly size="sm" variant="flat" onPress={onZoomIn} className={buttonClassName}> | |
| <ZoomIn className="h-4 w-4" /> | |
| </Button> | |
| <Button isIconOnly size="sm" variant="flat" onPress={onZoomOut} className={buttonClassName}> | |
| <ZoomOut className="h-4 w-4" /> | |
| </Button> | |
| </ButtonGroup> | |
| <Button size="sm" variant="flat" onPress={onReset} className={resetButtonClassName}> | |
| <Button | |
| isIconOnly | |
| size="sm" | |
| variant="flat" | |
| onPress={onZoomIn} | |
| className={buttonClassName} | |
| aria-label="Zoom in" | |
| title="Zoom in" | |
| > | |
| <ZoomIn className="h-4 w-4" /> | |
| </Button> | |
| <Button | |
| isIconOnly | |
| size="sm" | |
| variant="flat" | |
| onPress={onZoomOut} | |
| className={buttonClassName} | |
| aria-label="Zoom out" | |
| title="Zoom out" | |
| > | |
| <ZoomOut className="h-4 w-4" /> | |
| </Button> | |
| </ButtonGroup> | |
| <Button | |
| size="sm" | |
| variant="flat" | |
| onPress={onReset} | |
| className={resetButtonClassName} | |
| aria-label="Reset zoom" | |
| title="Reset zoom" | |
| > |
| import React from 'react'; | ||
| import { Button, ButtonGroup } from '@heroui/react'; | ||
| import { RotateCcw, ZoomIn, ZoomOut } from 'lucide-react'; |
There was a problem hiding this comment.
Most components that render HeroUI primitives in this repo are explicitly marked as client components (e.g. src/components/dispatch/teamcard.tsx:2). Consider adding 'use client' at the top of this shared UI component so it’s consistently treated as client-side when reused across the app.
| <Button | ||
| isIconOnly | ||
| size="sm" | ||
| variant="flat" | ||
| isDisabled={currentLayer <= 0} | ||
| onPress={onPreviousLayer} | ||
| > | ||
| <ChevronLeft className="h-4 w-4" /> | ||
| </Button> | ||
| <span className="text-xs text-surface-light min-w-[100px] text-center">{currentLayerName || 'Layer'}</span> | ||
| <Button | ||
| isIconOnly | ||
| size="sm" | ||
| variant="flat" | ||
| isDisabled={totalLayers <= 1 || currentLayer >= totalLayers - 1} | ||
| onPress={onNextLayer} | ||
| > | ||
| <ChevronRight className="h-4 w-4" /> | ||
| </Button> | ||
| <Button | ||
| isIconOnly | ||
| size="sm" | ||
| variant="flat" | ||
| color="danger" | ||
| onPress={onDeleteLayer} | ||
| > | ||
| <Trash2 className="h-4 w-4" /> | ||
| </Button> | ||
| <Button | ||
| isIconOnly | ||
| size="sm" | ||
| variant="flat" | ||
| onPress={onAddLayer} | ||
| > | ||
| <Plus className="h-4 w-4" /> | ||
| </Button> |
There was a problem hiding this comment.
Several icon-only buttons here (prev/next/delete/add) don’t provide an aria-label, which makes the layer controls hard to use with screen readers. Add labels/titles for each icon-only action (e.g. “Previous layer”, “Delete layer”, etc.).
| <ScrollShadow className="space-y-2 pr-2 max-h-[calc(100vh-430px)] scrollbar-hide"> | ||
| {equipment.map((item, idx) => ( | ||
| <Card | ||
| key={idx} |
There was a problem hiding this comment.
The equipment list uses the array index as the React key (key={idx}), which can cause incorrect item reuse when removing/editing entries (especially with the in-place edit UI). Since Equipment includes an id, prefer using item.id as the key to keep identity stable.
| key={idx} | |
| key={item.id} |
| import React from 'react'; | ||
| import { Textarea } from '@heroui/react'; | ||
|
|
There was a problem hiding this comment.
This component imports HeroUI (@heroui/react) and is used in interactive dispatch UI, but it isn’t marked with 'use client'. Most interactive components in this repo include the directive (e.g. src/components/dispatch/teamcard.tsx:2); consider adding it here for consistency and to avoid accidental server-component usage issues.
| import React from 'react'; | ||
| import { DateValue } from '@internationalized/date'; | ||
| import { DatePicker, Input } from '@heroui/react'; | ||
| import type { Event } from '@/app/types'; |
There was a problem hiding this comment.
This section component uses HeroUI components and event handlers but doesn’t include a 'use client' directive. To match established patterns (e.g. src/components/modals/event/addteammodal.tsx:2), consider adding 'use client' at the top of these extracted section components.
| import React from 'react'; | ||
| import { Button, Card, Input, ScrollShadow } from '@heroui/react'; | ||
| import { Edit2, Plus, Trash2 } from 'lucide-react'; | ||
| import type { Equipment } from '@/app/types'; |
There was a problem hiding this comment.
This extracted section uses HeroUI components and interactive callbacks, but it isn’t marked with 'use client'. For consistency with the rest of the codebase’s component conventions, consider adding 'use client' at the top of the file.
Summary
Extracts shared pure utilities for scheduling, analytics, markers, and zoom math. Extracts reusable hooks for schedule generation, team form handling, and zoom/pan. Decomposes dispatch internals into focused modules (adapters, widgets, call tracking state/details). Decomposes event create and venue management pages into focused section components. Adds shared map controls to reduce repeated zoom/pan UI logic. Fixes create-page hasVenue typing issue. Updates architecture/component docs to reflect the new organization. Reduces monolithic file complexity and improves maintainability.
Checklist
Related issues
Closes #9