feat: Gaps page UI/UX (#1741)

Author: arielvino

src/api/gapsService.ts

@@ -1,4 +1,4 @@
-import dayjs, { ISRAEL_TIMEZONE } from 'src/dayjs'
+import dayjs, { ISRAEL_TIMEZONE, toIsraelTimezone } from 'src/dayjs'
 import { USER_CASE_API } from './apiConfig'
 
 export type Gap = {
@@ -7,6 +7,27 @@ export type Gap = {
   gtfsRideId?: number
 }
 
+// What actually lives in the (localStorage-persisted) React Query cache: JSON-native
+// only. dayjs times are held as ISO strings and revived at the UI edge (GapsTable),
+// so a rehydrated cache can never hand the table a bare string to call .format() on.
+export type SerializedGap = {
+  plannedStartTime?: string
+  actualStartTime?: string
+  gtfsRideId?: number
+}
+
+export const serializeGap = (gap: Gap): SerializedGap => ({
+  gtfsRideId: gap.gtfsRideId,
+  plannedStartTime: gap.plannedStartTime?.toISOString(),
+  actualStartTime: gap.actualStartTime?.toISOString(),
+})
+
+export const reviveGap = (gap: SerializedGap): Gap => ({
+  gtfsRideId: gap.gtfsRideId,
+  plannedStartTime: gap.plannedStartTime ? toIsraelTimezone(gap.plannedStartTime) : undefined,
+  actualStartTime: gap.actualStartTime ? toIsraelTimezone(gap.actualStartTime) : undefined,
+})
+
 export function parseTime(time?: dayjs.ConfigType) {
   if (!time) return undefined
   const utcDayjs = dayjs.utc(time).utcOffset(0, true).tz(ISRAEL_TIMEZONE)

src/pages/gaps/GapsTable.tsx

@@ -13,7 +13,7 @@ import type { TFunction } from 'i18next'
 import React, { memo, useMemo, useState } from 'react'
 import { useTranslation } from 'react-i18next'
 import { Link } from 'react-router'
-import { Gap } from 'src/api/gapsService'
+import { Gap, reviveGap, SerializedGap } from 'src/api/gapsService'
 import dayjs from 'src/dayjs'
 import {
   formatServiceDayTime,
@@ -26,9 +26,11 @@ import DisplayGapsPercentage from '../components/DisplayGapsPercentage'
 import { Row } from '../components/Row'
 
 interface GapsTableProps {
-  gaps?: Gap[]
+  gaps?: SerializedGap[]
   loading?: boolean
   initOnlyGapped?: boolean
+  onlyGapped?: boolean
+  onOnlyGappedChange?: (value: boolean) => void
   singleLineMapBaseHref: string
   date: string
   onStartTimeClick?: (rideTime: string) => void
@@ -89,16 +91,28 @@ function buildTooltip(gap: Gap, t: TFunction): React.ReactNode {
 const getGap = (gap: Gap) => gap.plannedStartTime || gap.actualStartTime
 
 const GapsTable: React.FC<GapsTableProps> = ({
-  gaps,
+  gaps: rawGaps,
   loading,
   initOnlyGapped = false,
+  onlyGapped: onlyGappedProp,
+  onOnlyGappedChange,
   singleLineMapBaseHref,
   date,
   onStartTimeClick,
 }) => {
   const { t } = useTranslation()
+  // The gaps cache is persisted as JSON (dayjs → ISO strings). Revive to dayjs here,
+  // at the single consumption edge, so all the comparison/formatting below is unchanged.
+  const gaps = useMemo(() => rawGaps?.map(reviveGap), [rawGaps])
   const { start: serviceDayStart } = serviceDayBounds(date)
-  const [onlyGapped, setOnlyGapped] = useState(initOnlyGapped)
+  // Controllable: the gaps page owns and persists this via usePageState; the
+  // story leaves it uncontrolled and seeds it with initOnlyGapped.
+  const [onlyGappedState, setOnlyGappedState] = useState(initOnlyGapped)
+  const onlyGapped = onlyGappedProp ?? onlyGappedState
+  const setOnlyGapped = (value: boolean) => {
+    setOnlyGappedState(value)
+    onOnlyGappedChange?.(value)
+  }
 
   const filteredGaps: Gap[] = useMemo(() => {
     if (!gaps) return []
@@ -133,7 +147,7 @@ const GapsTable: React.FC<GapsTableProps> = ({
   }, [gaps])
 
   return (
-    <Widget marginBottom sx={{ overflowY: 'none', maxWidth: '600px' }}>
+    <Widget marginBottom sx={{ overflowY: 'none', maxWidth: '600px', mx: 'auto' }}>
       <Row style={{ justifyContent: 'space-between', fontWeight: 500 }}>
         <FormControlLabel
           control={

src/pages/gaps/gapsTable.stories.tsx

@@ -1,6 +1,6 @@
 import type { Meta, StoryObj } from '@storybook/react-vite'
-import { Gap } from 'src/api/gapsService'
-import dayjs from 'src/dayjs'
+import { Gap, serializeGap } from 'src/api/gapsService'
+import dayjs, { ISRAEL_TIMEZONE } from 'src/dayjs'
 import GapsTable from './GapsTable'
 
 const meta = {
@@ -38,43 +38,55 @@ const meta = {
 export default meta
 
 type Story = StoryObj<typeof meta>
-const yesteday = dayjs().startOf('day').subtract(1, 'day')
+// Fixed Israel-local service day: dayjs.tz(str, tz) reads the digits AS Israel
+// wall-clock, so the rendered times are identical on every machine (CI runs in UTC)
+// and stable across DST — unlike dayjs(str).tz(tz), which re-anchors by the runner's
+// zone. The gaps are passed through serializeGap → reviveGap (the real cache path).
+const serviceDay = dayjs.tz('2025-01-01', ISRAEL_TIMEZONE)
 const mockGaps: Gap[] = [
+  // as planned (green): planned === actual
   {
-    plannedStartTime: yesteday.set('hour', 13),
-    actualStartTime: yesteday.set('hour', 13),
+    plannedStartTime: serviceDay.set('hour', 13),
+    actualStartTime: serviceDay.set('hour', 13),
   },
+  // duplicate (cyan): a second ride sharing the 13:00 actual
   {
     plannedStartTime: undefined,
-    actualStartTime: yesteday.set('hour', 13),
+    actualStartTime: serviceDay.set('hour', 13),
   },
+  // missing (red): a past planned ride with no actual
   {
-    plannedStartTime: yesteday.set('hour', 14),
+    plannedStartTime: serviceDay.set('hour', 14),
     actualStartTime: undefined,
   },
+  // extra (yellow): an actual with no matching planned
   {
     plannedStartTime: undefined,
-    actualStartTime: yesteday.set('hour', 14).set('minute', 30),
+    actualStartTime: serviceDay.set('hour', 14).set('minute', 30),
   },
+  // in the future (blue): GapsTable derives this state by comparing against the
+  // current time, so this row must stay relative to "now" — a fixed past literal
+  // would render as "missing". Being a later calendar day, it also carries the
+  // single 🌙 next-day marker.
   {
-    plannedStartTime: yesteday.add(2, 'day').set('hour', 15),
+    plannedStartTime: dayjs().tz(ISRAEL_TIMEZONE).startOf('day').add(1, 'day').set('hour', 15),
     actualStartTime: undefined,
   },
 ]
 
 export const Default: Story = {
   args: {
-    gaps: mockGaps,
-    date: dayjs().format('YYYY-MM-DD'),
+    gaps: mockGaps.map(serializeGap),
+    date: serviceDay.format('YYYY-MM-DD'),
     singleLineMapBaseHref:
       '/single-line-map?date=2025-01-01&operatorId=3&lineNumber=5&routeKey=10018-2',
   },
 }
 
 export const OnlyGappe: Story = {
   args: {
-    gaps: mockGaps,
-    date: dayjs().format('YYYY-MM-DD'),
+    gaps: mockGaps.map(serializeGap),
+    date: serviceDay.format('YYYY-MM-DD'),
     initOnlyGapped: true,
     singleLineMapBaseHref:
       '/single-line-map?date=2025-01-01&operatorId=3&lineNumber=5&routeKey=10018-2',

src/pages/gaps/index.tsx

@@ -1,12 +1,13 @@
 import { Alert, CircularProgress, Grid, Typography } from '@mui/material'
-import { useCallback, useContext, useEffect, useMemo, useState } from 'react'
+import { useQuery } from '@tanstack/react-query'
+import { useCallback, useContext, useMemo } from 'react'
 import { useTranslation } from 'react-i18next'
 import dayjs, { ISRAEL_TIMEZONE } from 'src/dayjs'
+import { usePageState } from 'src/hooks/usePageState'
 import { GlobalSearchContext } from 'src/model/globalState'
 import { INPUT_SIZE } from 'src/resources/sizes'
-import { Gap, getGapsAsync } from '../../api/gapsService'
+import { getGapsAsync, SerializedGap, serializeGap } from '../../api/gapsService'
 import { getServiceDayRoutes } from '../../api/serviceDayRoutesService'
-import { BusRoute } from '../../model/busRoute'
 import { DateSelector } from '../components/DateSelector'
 import { Label } from '../components/Label'
 import LineNumberSelector from '../components/LineSelector'
@@ -22,9 +23,13 @@ const GapsPage = () => {
   const { t } = useTranslation()
   const { search, setSearch } = useContext(GlobalSearchContext)
   const { operatorId, lineNumber, date, routeKey } = search
-  const [routes, setRoutes] = useState<BusRoute[] | undefined>()
-  const [gaps, setGaps] = useState<Gap[]>()
-  const [gapsIsLoading, setGapsIsLoading] = useState(false)
+
+  // scrollPosition (auto-restored by usePageState) and the "only gaps" toggle are
+  // device-local UI state, kept out of the shareable params.
+  const { ui, setUi } = usePageState('gaps', {
+    params: {},
+    ui: { scrollPosition: 0, gapsOnly: false },
+  })
 
   const singleLineMapBaseHref = useMemo(() => {
     const params = new URLSearchParams()
@@ -35,48 +40,39 @@ const GapsPage = () => {
     return `/single-line-map?${params.toString()}`
   }, [search.date, search.lineNumber, search.operatorId, search.routeKey])
 
-  useEffect(() => {
-    if (!(operatorId && routes && routeKey && date)) return
-    const selectedRoute = routes.find((route) => route.key === routeKey)
-    if (!selectedRoute) return
-
-    setGapsIsLoading(true)
-    const { start, end } = serviceDayBounds(date)
-    getGapsAsync(start, end, operatorId, selectedRoute.lineRef)
-      .then((res) =>
-        setGaps(
-          res.filter((g) => {
-            const t = g.plannedStartTime || g.actualStartTime
-            return t && !t.isBefore(start) && t.isBefore(end)
-          }),
-        ),
-      )
-      .catch((err) => {
-        console.error('Failed to fetch gaps:', err.message)
-        setGaps(undefined)
-      })
-      .finally(() => setGapsIsLoading(false))
-  }, [operatorId, routes, routeKey, date])
-
-  useEffect(() => {
-    if (!operatorId || !lineNumber) {
-      return
-    }
-
-    const controller = new AbortController()
+  const routesQuery = useQuery({
+    queryFn: ({ signal }) => {
+      if (!operatorId || !lineNumber) return null
+      return getServiceDayRoutes(dayjs.tz(date, ISRAEL_TIMEZONE), operatorId, lineNumber, signal)
+    },
+    queryKey: ['gapsRoutes', operatorId, lineNumber, date],
+  })
+  const routes = routesQuery.data ?? undefined
 
-    getServiceDayRoutes(dayjs.tz(date, ISRAEL_TIMEZONE), operatorId, lineNumber, controller.signal)
-      .then((fetchedRoutes) => {
-        if (search.lineNumber === lineNumber) {
-          setRoutes(fetchedRoutes)
-        }
-      })
-      .catch((err) => {
-        console.error('Failed to fetch routes:', err.message)
-      })
+  const selectedRoute = useMemo(
+    () => routes?.find((route) => route.key === routeKey),
+    [routes, routeKey],
+  )
 
-    return () => controller.abort()
-  }, [operatorId, lineNumber, date, setSearch])
+  const gapsQuery = useQuery({
+    queryFn: async (): Promise<SerializedGap[] | null> => {
+      if (!operatorId || !selectedRoute || !date) return null
+      const { start, end } = serviceDayBounds(date)
+      const res = await getGapsAsync(start, end, operatorId, selectedRoute.lineRef)
+      return (
+        res
+          .filter((g) => {
+            const gapTime = g.plannedStartTime || g.actualStartTime
+            return gapTime && !gapTime.isBefore(start) && gapTime.isBefore(end)
+          })
+          // Store JSON-serializable strings, not dayjs, so the persisted cache
+          // rehydrates losslessly; GapsTable revives them to dayjs on read.
+          .map(serializeGap)
+      )
+    },
+    queryKey: ['gaps', operatorId, selectedRoute?.lineRef, date],
+  })
+  const gaps = gapsQuery.data ?? undefined
 
   const handleDateChange = (time: dayjs.Dayjs | null) => {
     if (!time) return
@@ -87,7 +83,9 @@ const GapsPage = () => {
   }
 
   const handleOperatorChange = (operatorId: string) => {
-    setSearch((current) => ({ ...current, operatorId }))
+    // Changing/clearing the operator invalidates the chosen route (routes are
+    // per operator+line), so reset it to close the stale results table.
+    setSearch((current) => ({ ...current, operatorId, routeKey: null }))
   }
 
   const handleLineNumberChange = (lineNumber: string) => {
@@ -96,9 +94,6 @@ const GapsPage = () => {
         ? { ...current }
         : { ...current, lineNumber, routeKey: null },
     )
-    if (lineNumber !== search.lineNumber) {
-      setRoutes(undefined)
-    }
   }
 
   const handleRouteKeyChange = (routeKey?: string) => {
@@ -123,36 +118,28 @@ const GapsPage = () => {
       <Alert severity="info" variant="outlined" icon={false}>
         {t('gaps_page_description')}
       </Alert>
-      <Grid container spacing={2} sx={{ maxWidth: INPUT_SIZE }}>
+      <Grid container spacing={2} sx={{ maxWidth: INPUT_SIZE, width: '100%', mx: 'auto' }}>
         {/* choose date */}
-        <Grid size={{ xs: 4 }}>
-          <Label text={t('choose_date')} />
-        </Grid>
-        <Grid size={{ xs: 8 }}>
+        <Grid size={{ sm: 6, xs: 12 }}>
           <DateSelector time={dayjs.tz(date, ISRAEL_TIMEZONE)} onChange={handleDateChange} />
         </Grid>
         {/* choose operator */}
-        <Grid size={{ xs: 4 }}>
-          <Label text={t('choose_operator')} />
-        </Grid>
-        <Grid size={{ xs: 8 }}>
+        <Grid size={{ sm: 6, xs: 12 }}>
           <OperatorSelector
             operatorId={operatorId ?? undefined}
             setOperatorId={handleOperatorChange}
           />
         </Grid>
         {/* choose line */}
-        <Grid size={{ xs: 4 }}>
-          <Label text={t('choose_line')} />
-        </Grid>
-        <Grid size={{ xs: 8 }}>
+        <Grid size={{ sm: 6, xs: 12 }}>
           <LineNumberSelector
+            disabled={!operatorId}
             lineNumber={lineNumber ?? undefined}
             setLineNumber={handleLineNumberChange}
           />
         </Grid>
-        {/* choose routes */}
-        <Grid size={{ xs: 12 }}>
+        {/* choose route */}
+        <Grid size={{ sm: 6, xs: 12 }}>
           {routes?.length === 0 ? (
             <NotFound>{t('line_not_found')}</NotFound>
           ) : (
@@ -164,22 +151,24 @@ const GapsPage = () => {
             />
           )}
         </Grid>
-        <Grid size={{ xs: 12 }}>
-          {gapsIsLoading && (
+        {gapsQuery.isLoading && (
+          <Grid size={{ xs: 12 }}>
             <Row>
               <Label text={t('loading_gaps')} />
               <CircularProgress />
             </Row>
-          )}
-        </Grid>
+          </Grid>
+        )}
       </Grid>
-      {routeKey && routeKey !== '' && (
+      {selectedRoute && (
         <GapsTable
-          loading={gapsIsLoading}
+          loading={gapsQuery.isLoading}
           gaps={gaps}
           date={date}
           singleLineMapBaseHref={singleLineMapBaseHref}
           onStartTimeClick={handleStartTimeClick}
+          onlyGapped={ui.gapsOnly}
+          onOnlyGappedChange={(value) => setUi((prev) => ({ ...prev, gapsOnly: value }))}
         />
       )}
     </PageContainer>