Fix week navigation using ref to eliminate stale closure entirely

Previous wsArg approach still suffered from useCallback memoisation
interacting with React batching — loadAll could fire with the render-
before-last week start. Switch to weekStartRef (written before every
loadAll call and on every render) so loadAll always reads the live
value regardless of when the memoised function was created.

Also removes unused putWarningThresholds import (added externally).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
jtricerolph 2026-07-22 13:35:34 +00:00
parent f1ba176195
commit fee388db4c
6 changed files with 219 additions and 176 deletions

View file

@ -128,8 +128,11 @@ export function Planner() {
const [staff, setStaff] = useState<StaffMember[]>([])
const [pickup, setPickup] = useState<PickupData>({})
const [generalTasks, setGeneralTasks] = useState<GeneralTask[]>([])
const [tolerance, setTolerance] = useState(30)
const [adjustments, setAdjustments] = useState<Adjustment[]>([])
const [warnOverRed, setWarnOverRed] = useState(4)
const [warnOverAmber, setWarnOverAmber] = useState(1)
const [warnUnderAmber, setWarnUnderAmber] = useState(1)
const [warnUnderRed, setWarnUnderRed] = useState(2)
const [workforceRota, setWorkforceRota] = useState<WorkforceRota | null>(null)
const [wfStaff, setWfStaff] = useState<{ id: string; name: string }[]>([])
const [syncing, setSyncing] = useState(false)
@ -143,6 +146,8 @@ export function Planner() {
const timers = useRef<Record<string, ReturnType<typeof setTimeout>>>({})
const saveMsgTimer = useRef<ReturnType<typeof setTimeout> | null>(null)
const wfStaffLoaded = useRef(false)
const weekStartRef = useRef<string | null>(null)
weekStartRef.current = weekStart // always current, updated every render
function debounce(key: string, fn: () => void, delay = 400) {
clearTimeout(timers.current[key])
@ -162,11 +167,11 @@ export function Planner() {
putLastReviewed(today).catch(() => {})
}
const loadAll = useCallback(async (force = false, wsArg?: string | null, lvArg?: string) => {
const loadAll = useCallback(async (force = false, lvArg?: string) => {
setLoading(true)
setError('')
const today = todayStr()
const ws = wsArg !== undefined ? (wsArg ?? today) : (weekStart || today)
const ws = weekStartRef.current || today
const lv = lvArg !== undefined ? (lvArg || offsetDate(today, -1)) : (lastViewed || savedLastReviewed || offsetDate(today, -1))
try {
const [b, cfg] = await Promise.all([getBookings(ws, lv, force), getConfig()])
@ -175,8 +180,11 @@ export function Planner() {
setStaff(cfg.staff_data || [])
setPickup(cfg.pickup_data || {})
setGeneralTasks(cfg.general_tasks || [])
setTolerance(cfg.tolerance_minutes ?? 30)
setAdjustments(cfg.adjustments || [])
setWarnOverRed(cfg.warn_over_red_hrs ?? 4)
setWarnOverAmber(cfg.warn_over_amber_hrs ?? 1)
setWarnUnderAmber(cfg.warn_under_amber_hrs ?? 1)
setWarnUnderRed(cfg.warn_under_red_hrs ?? 2)
setWorkforceRota(cfg.workforce_rota || null)
if (cfg.last_reviewed) {
setSavedLastReviewed(cfg.last_reviewed)
@ -187,7 +195,7 @@ export function Planner() {
} finally {
setLoading(false)
}
}, [weekStart, lastViewed, savedLastReviewed])
}, [lastViewed, savedLastReviewed])
useEffect(() => { loadAll(false) }, []) // eslint-disable-line react-hooks/exhaustive-deps
@ -218,16 +226,18 @@ export function Planner() {
function handleWeekOffset(days: number) {
const today = todayStr()
const newStart = offsetDate(weekStart || today, days)
const newStart = offsetDate(weekStartRef.current || today, days)
weekStartRef.current = newStart
setWeekStart(newStart)
stampLastReviewed()
loadAll(true, newStart)
loadAll(true)
}
function handleWeekToday() {
weekStartRef.current = null
setWeekStart(null)
stampLastReviewed()
loadAll(true, null)
loadAll(true)
}
function handleRefresh() {
@ -338,7 +348,7 @@ export function Planner() {
<input
type="date"
value={displayWeekStart}
onChange={e => { if (e.target.value) { const v = e.target.value; setWeekStart(v); stampLastReviewed(); debounce('weekStart', () => loadAll(true, v), 600) } }}
onChange={e => { if (e.target.value) { const v = e.target.value; weekStartRef.current = v; setWeekStart(v); stampLastReviewed(); debounce('weekStart', () => loadAll(true), 600) } }}
style={{ border: '1px solid var(--card-border)', borderRadius: '6px', padding: '0.35rem 0.5rem', fontSize: '0.82rem', color: 'var(--text-dark)' }}
/>
<div style={{ width: '1px', height: '24px', background: 'var(--card-border)' }} />
@ -348,7 +358,7 @@ export function Planner() {
<input
type="date"
value={lastViewed}
onChange={e => { if (e.target.value) { const v = e.target.value; setLastViewed(v); debounce('lastViewed', () => loadAll(true, undefined, v), 600) } }}
onChange={e => { if (e.target.value) { const v = e.target.value; setLastViewed(v); debounce('lastViewed', () => loadAll(true, v), 600) } }}
style={{ border: '1px solid var(--card-border)', borderRadius: '6px', padding: '0.3rem 0.5rem', fontSize: '0.78rem', color: 'var(--text-dark)' }}
/>
</label>
@ -447,7 +457,10 @@ export function Planner() {
bookings={bookings}
staff={staff}
required={required}
tolerance={tolerance}
warnOverRed={warnOverRed}
warnOverAmber={warnOverAmber}
warnUnderAmber={warnUnderAmber}
warnUnderRed={warnUnderRed}
onChange={handleStaffChange}
workforceRota={workforceRota}
wfStaff={wfStaff}
@ -773,17 +786,19 @@ function RequiredTable({ bookings, required }: { bookings: BookingsData; require
// ── Staff Table ───────────────────────────────────────────────────────────────
function StaffTable({ bookings, staff, required, tolerance, onChange, workforceRota, wfStaff }: {
function StaffTable({ bookings, staff, required, warnOverRed, warnOverAmber, warnUnderAmber, warnUnderRed, onChange, workforceRota, wfStaff }: {
bookings: BookingsData
staff: StaffMember[]
required: Record<string, RequiredDay>
tolerance: number
warnOverRed: number
warnOverAmber: number
warnUnderAmber: number
warnUnderRed: number
onChange: (next: StaffMember[]) => void
workforceRota: WorkforceRota | null
wfStaff: { id: string; name: string }[]
}) {
const { dates } = bookings
const tolHrs = tolerance / 60
function setMemberName(i: number, name: string) {
const next = staff.map((m, idx) => idx === i ? { ...m, name } : m)
@ -883,44 +898,72 @@ function StaffTable({ bookings, staff, required, tolerance, onChange, workforceR
))}
</tbody>
<tfoot>
<tr>
<td className="col-label" style={{ fontSize: '0.78rem', color: 'var(--text-mid)' }}>Total Available</td>
{dates.map(date => {
const rotaHrs = rotaMembers.reduce((s, m) => s + (m.days[date]?.hours || 0), 0)
{(() => {
const avail: Record<string, number> = {}
for (const date of dates) {
const rotaHrs = rotaMembers.reduce((s, m) => s + (m.days[date]?.hours || 0), 0)
const manualHrs = staff.reduce((s, m) => s + (m.hours[date] || 0), 0)
return <td key={date}>{fmtH(rotaHrs + manualHrs)}</td>
})}
<td />
</tr>
<tr>
<td className="col-label" style={{ fontSize: '0.72rem', color: 'var(--text-mid)' }}>vs Booked</td>
{dates.map(date => {
const rotaHrs = rotaMembers.reduce((s, m) => s + (m.days[date]?.hours || 0), 0)
const manualHrs = staff.reduce((s, m) => s + (m.hours[date] || 0), 0)
const reqHrs = required[date].booked + required[date].general
return <td key={date}><DiffCell available={rotaHrs + manualHrs} required={reqHrs} tolerance={tolHrs} /></td>
})}
<td />
</tr>
<tr>
<td className="col-label" style={{ fontSize: '0.72rem', color: 'var(--text-mid)' }}>vs with Pickup</td>
{dates.map(date => {
const rotaHrs = rotaMembers.reduce((s, m) => s + (m.days[date]?.hours || 0), 0)
const manualHrs = staff.reduce((s, m) => s + (m.hours[date] || 0), 0)
return <td key={date}><DiffCell available={rotaHrs + manualHrs} required={required[date].total} tolerance={tolHrs} /></td>
})}
<td />
</tr>
avail[date] = rotaHrs + manualHrs
}
return (
<>
<tr>
<td className="col-label" style={{ fontSize: '0.78rem', color: 'var(--text-mid)' }}>Total Available</td>
{dates.map(date => <td key={date}>{fmtH(avail[date])}</td>)}
<td />
</tr>
<tr>
<td className="col-label" style={{ fontSize: '0.72rem', color: 'var(--text-mid)' }}>vs Booked</td>
{dates.map(date => (
<td key={date}><PlainDiffCell available={avail[date]} required={required[date].booked} /></td>
))}
<td />
</tr>
<tr>
<td className="col-label" style={{ fontSize: '0.72rem', color: 'var(--text-mid)' }}>with Recurring Tasks</td>
{dates.map(date => (
<td key={date}><PlainDiffCell available={avail[date]} required={required[date].booked + required[date].general} /></td>
))}
<td />
</tr>
<tr>
<td className="col-label" style={{ fontSize: '0.72rem', color: 'var(--text-mid)' }}>with Pickup</td>
{dates.map(date => (
<td key={date}><PlainDiffCell available={avail[date]} required={required[date].booked + required[date].general + required[date].pickup} /></td>
))}
<td />
</tr>
<tr style={{ background: '#e2e8f0' }}>
<td className="col-label" style={{ fontWeight: 700 }}>with Adjustments</td>
{dates.map(date => (
<td key={date}><DiffCell available={avail[date]} required={required[date].total} warnOverRed={warnOverRed} warnOverAmber={warnOverAmber} warnUnderAmber={warnUnderAmber} warnUnderRed={warnUnderRed} /></td>
))}
<td />
</tr>
</>
)
})()}
</tfoot>
</table>
)
}
function DiffCell({ available, required, tolerance }: { available: number; required: number; tolerance: number }) {
function PlainDiffCell({ available, required }: { available: number; required: number }) {
if (available === 0 && required === 0) return <span style={{ color: 'var(--text-mid)' }}></span>
const diff = available - required
if (required === 0 && available === 0) return <span style={{ color: 'var(--text-mid)' }}></span>
if (diff < -tolerance) return <span className="diff-under"> {fmtH(Math.abs(diff))} short</span>
if (diff > tolerance) return <span className="diff-over"> {fmtH(diff)} spare</span>
return <span style={{ color: 'var(--text-mid)', fontSize: '0.82rem' }}>{diff >= 0 ? '+' : ''}{fmtH(diff)}</span>
}
function DiffCell({ available, required, warnOverRed, warnOverAmber, warnUnderAmber, warnUnderRed }: {
available: number; required: number
warnOverRed: number; warnOverAmber: number; warnUnderAmber: number; warnUnderRed: number
}) {
if (available === 0 && required === 0) return <span style={{ color: 'var(--text-mid)' }}></span>
const diff = available - required
if (diff > warnOverRed) return <span className="diff-over-red"> {fmtH(diff)} spare</span>
if (diff > warnOverAmber) return <span className="diff-over"> {fmtH(diff)} spare</span>
if (diff < -warnUnderRed) return <span className="diff-under"> {fmtH(Math.abs(diff))} short</span>
if (diff < -warnUnderAmber) return <span className="diff-under-amber"> {fmtH(Math.abs(diff))} short</span>
return <span className="diff-ok"> {diff >= 0 ? '+' : ''}{fmtH(diff)}</span>
}