Show a week of opening hours as a week

2026-10-18 21:16:47 +00:00 · 2026-08-10 10:13:07 +02:00
parent 0c8788ce44
commit 880e258438
12 changed files with 575 additions and 65 deletions
@@ -189,6 +189,8 @@ describe('PlaceDetailsColumn', () => {
      description: null,
      facts: [
        { kind: 'cuisine', value: 'regional', url: null },
+        // A payload written before hours became a field of their own. The chip
+        // list drops it rather than showing a truncated week beside the block.
        { kind: 'openingHours', value: 'Mo-Sa 17:30+', url: null },
        { kind: 'menu', value: null, url: 'https://example.org/menu' },
        { kind: 'outdoorSeating', value: null, url: null },
@@ -198,7 +200,7 @@ describe('PlaceDetailsColumn', () => {

    // A value is shown verbatim; a plain yes falls back to the translated label.
    expect(await screen.findByText('regional')).toBeInTheDocument()
-    expect(screen.getByText('Mo-Sa 17:30+')).toBeInTheDocument()
+    expect(screen.queryByText('Mo-Sa 17:30+')).not.toBeInTheDocument()
    expect(screen.getByText('places.details.fact.outdoorSeating')).toBeInTheDocument()
    expect(screen.getByRole('link', { name: /places.details.fact.menu/ })).toHaveAttribute(
      'href',
@@ -276,3 +278,127 @@ describe('PlaceDetailsColumn', () => {
    expect(placeEnrichment).toHaveBeenCalledTimes(2)
  })
 })
+
+/**
+ * FE-PDC-019..026 — opening hours and the rating.
+ *
+ * Both used to be chips in the fact row: a whole week joined with dots inside a
+ * `truncate` span, which in a 288px column read "Monday: 11:30 AM – 11:00 PM ·
+ * Tuesday:…", and a rating rendered as the text "3.8 (873)".
+ */
+describe('PlaceDetailsColumn — hours and rating', () => {
+  const WEEK = [
+    'Monday: 09:00-18:00',
+    'Tuesday: 09:00-18:00',
+    'Wednesday: 09:00-18:00',
+    'Thursday: 09:00-18:00',
+    'Friday: 09:00-18:00',
+    'Saturday: 10:00-14:00',
+    'Sunday: ?',
+  ]
+
+  const withHours = (over: Record<string, unknown> = {}) => {
+    placeEnrichment.mockResolvedValue({
+      photos: [],
+      facts: [],
+      description: null,
+      hours: { weekdayDescriptions: WEEK, periods: null, specialDays: null },
+      ...over,
+    })
+  }
+
+  it('FE-PDC-019: shows only today collapsed, not the whole week', async () => {
+    withHours()
+    renderColumn()
+
+    // Tuesday and Wednesday carry the same text as Monday, so count instead of
+    // asserting absence: exactly one line is on screen before expanding.
+    await screen.findByText('inspector.openingHours')
+    expect(screen.queryAllByText('09:00-18:00')).toHaveLength(1)
+    expect(screen.queryByText('10:00-14:00')).not.toBeInTheDocument()
+  })
+
+  it('FE-PDC-020: opens the full week on click', async () => {
+    withHours()
+    renderColumn()
+
+    fireEvent.click(await screen.findByRole('button', { expanded: false }))
+
+    // Assert on the list rather than on text counts: the collapsed header shows
+    // today's line too, so a count would depend on which day the suite runs.
+    const rows = screen.getAllByRole('listitem')
+    expect(rows).toHaveLength(7)
+    expect(rows[5]).toHaveTextContent('10:00-14:00')
+    // A day the tag said nothing about reads as a dash, not a question mark.
+    expect(rows[6]).toHaveTextContent('–')
+  })
+
+  it('FE-PDC-021: honours the 12-hour clock preference', async () => {
+    withHours()
+    renderColumn({ timeFormat: '12h' })
+
+    fireEvent.click(await screen.findByRole('button', { expanded: false }))
+    expect(screen.getAllByRole('listitem')[0]).toHaveTextContent('9:00 AM-6:00 PM')
+  })
+
+  it('FE-PDC-022: says nothing about open or closed without usable periods', async () => {
+    // An hours line the parser could not read is not evidence that somewhere is
+    // shut, and a confident wrong badge is worse than a missing one.
+    withHours()
+    renderColumn()
+
+    await screen.findByText('inspector.openingHours')
+    expect(screen.queryByText('inspector.opened')).not.toBeInTheDocument()
+    expect(screen.queryByText('inspector.closed')).not.toBeInTheDocument()
+  })
+
+  it('FE-PDC-023: reports a place open around the clock as open', async () => {
+    // The 24/7 shape: one period that never closes. Airports and main stations
+    // are tagged this way, and they showed no hours at all before.
+    withHours({
+      hours: {
+        weekdayDescriptions: WEEK.map((_, i) => `${['Monday','Tuesday','Wednesday','Thursday','Friday','Saturday','Sunday'][i]}: 00:00-24:00`),
+        periods: [{ open: { day: 0, hour: 0, minute: 0 }, close: null }],
+        specialDays: null,
+      },
+    })
+    renderColumn()
+
+    expect(await screen.findByText('inspector.opened')).toBeInTheDocument()
+  })
+
+  it('FE-PDC-024: renders the rating as stars with its count', async () => {
+    placeEnrichment.mockResolvedValue({
+      photos: [],
+      facts: [],
+      description: null,
+      rating: { value: 3.8, count: 873 },
+    })
+    renderColumn({ locale: 'de-DE' })
+
+    expect(await screen.findByText('3.8')).toBeInTheDocument()
+    expect(screen.getByText('(873)')).toBeInTheDocument()
+  })
+
+  it('FE-PDC-025: leaves the brackets off when there is no count', async () => {
+    // Google's search results carry a rating but never a count.
+    placeEnrichment.mockResolvedValue({
+      photos: [],
+      facts: [],
+      description: null,
+      rating: { value: 4, count: null },
+    })
+    renderColumn()
+
+    expect(await screen.findByText('4.0')).toBeInTheDocument()
+    expect(screen.queryByText('()')).not.toBeInTheDocument()
+  })
+
+  it('FE-PDC-026: hours alone are content, not an empty column', async () => {
+    withHours()
+    renderColumn()
+
+    await screen.findByText('inspector.openingHours')
+    expect(screen.queryByText('places.details.nothing')).not.toBeInTheDocument()
+  })
+})
@@ -4,6 +4,8 @@ import {
  Bike,
  Check,
  ChefHat,
+  ChevronDown,
+  ChevronUp,
  Clock,
  ExternalLink,
  ImageOff,
@@ -17,8 +19,10 @@ import {
  Sun,
  Wifi,
 } from 'lucide-react'
-import type { MapsPlaceEnrichmentResult, PlaceFact, PlacePhotoCandidate } from '@trek/shared'
+import type { MapsPlaceEnrichmentResult, PlaceFact, PlaceHours, PlacePhotoCandidate, PlaceRating } from '@trek/shared'
 import { mapsApi } from '../../api/client'
+import { resolveOpenNow, resolvePlaceTimeZone, placeWeekdayIndex } from './placeOpenState'
+import { convertHoursLine, isUnknownHoursLine, splitHoursLine } from './placeHoursFormat'
 import type { TranslationFn } from '../../types'

 /** The place the column is describing. Null while nothing is selected. */
@@ -40,6 +44,10 @@ interface PlaceDetailsColumnProps {
  /** True once the form's description field has something in it. */
  hasDescription: boolean
  language: string
+  /** The user's clock preference, so hours read the same as in the inspector. */
+  timeFormat?: string
+  /** For grouping the rating count's digits. */
+  locale?: string
  t: TranslationFn
 }

@@ -77,7 +85,7 @@ function writeSession(key: string, value: MapsPlaceEnrichmentResult): void {
 * without it the tab that was open while the fix shipped keeps replaying the
 * answer the fix was about — and reports it as still broken.
 */
-const ENRICH_CACHE_V = 3
+const ENRICH_CACHE_V = 4

 function cacheKeyFor(selection: PlaceDetailsSelection, language: string): string {
  const id = selection.placeId || `coords:${selection.lat}:${selection.lng}`
@@ -103,6 +111,8 @@ export default function PlaceDetailsColumn({
  onAdoptDescription,
  hasDescription,
  language,
+  timeFormat = '24h',
+  locale = 'en-US',
  t,
 }: PlaceDetailsColumnProps): React.ReactElement {
  const [data, setData] = useState<MapsPlaceEnrichmentResult | null>(null)
@@ -171,7 +181,13 @@ export default function PlaceDetailsColumn({
  useEffect(() => () => abortRef.current?.abort(), [])

  const isEmpty =
-    state === 'ready' && !data?.disabled && !data?.photos.length && !data?.description && !data?.facts.length
+    state === 'ready' &&
+    !data?.disabled &&
+    !data?.photos.length &&
+    !data?.description &&
+    !data?.facts.length &&
+    !data?.hours &&
+    !data?.rating

  return (
    <aside className="w-full sm:w-72 shrink-0 flex flex-col rounded-xl border border-edge bg-surface-secondary overflow-hidden self-stretch">
@@ -206,6 +222,14 @@ export default function PlaceDetailsColumn({
        {selection && state === 'ready' && !data?.disabled && !isEmpty && (
          <>
            <PhotoStrip photos={data?.photos ?? []} selectedImageUrl={selectedImageUrl} onPickImage={onPickImage} t={t} />
+            <RatingRow rating={data?.rating ?? null} locale={locale} />
+            <OpeningHoursBlock
+              hours={data?.hours ?? null}
+              lat={selection.lat}
+              lng={selection.lng}
+              timeFormat={timeFormat}
+              t={t}
+            />
            <FactList facts={data?.facts ?? []} t={t} />
            <DescriptionBlock
              description={data?.description ?? null}
@@ -356,6 +380,137 @@ function PhotoCredit({ photo }: { photo: PlacePhotoCandidate }): React.ReactElem
  )
 }

+/**
+ * The star rating, as stars.
+ *
+ * It used to be a chip reading "3.8 (873)", which is the same information and
+ * none of the recognition — a row of stars is read at a glance and a decimal in
+ * a pill is read as text. The count only appears when there is one: Google's
+ * search results carry a rating without a count, and empty brackets look broken.
+ */
+function RatingRow({ rating, locale }: { rating: PlaceRating | null; locale: string }): React.ReactElement | null {
+  if (!rating) return null
+  const filled = Math.round(rating.value)
+
+  return (
+    <div className="flex items-center gap-1.5">
+      <span className="flex items-center gap-0.5" aria-hidden>
+        {[1, 2, 3, 4, 5].map((step) => (
+          <Star
+            key={step}
+            className={`w-3 h-3 ${step <= filled ? 'fill-current text-accent-on' : 'text-content-faint'}`}
+          />
+        ))}
+      </span>
+      <span className="text-caption font-semibold text-content">{rating.value.toFixed(1)}</span>
+      {rating.count != null && (
+        <span className="text-caption text-content-faint">({rating.count.toLocaleString(locale)})</span>
+      )}
+    </div>
+  )
+}
+
+/**
+ * The week's opening hours: today up front, the rest a click away.
+ *
+ * This replaces a single chip holding the entire week joined with dots, which
+ * in a 288px column truncated to "Monday: 11:30 AM – 11:00 PM · Tuesday:…" and
+ * told a reader nothing at all.
+ *
+ * Open/closed is recomputed from the structured periods in the place's own
+ * timezone rather than taken from whatever the provider reported when the
+ * payload was cached — read from another continent that flag is wrong twice
+ * over (#1680). When the periods do not support an answer there is no badge:
+ * an hours line the parser could not read is not evidence that somewhere is
+ * closed, and a confident wrong badge is worse than a missing one.
+ */
+function OpeningHoursBlock({
+  hours,
+  lat,
+  lng,
+  timeFormat,
+  t,
+}: {
+  hours: PlaceHours | null
+  lat: number
+  lng: number
+  timeFormat: string
+  t: TranslationFn
+}): React.ReactElement | null {
+  const [expanded, setExpanded] = useState(false)
+  const lines = hours?.weekdayDescriptions
+  if (!lines?.length) return null
+
+  const today = placeWeekdayIndex(new Date(), resolvePlaceTimeZone(lat, lng))
+  const openNow = resolveOpenNow({ periods: hours?.periods, specialDays: hours?.specialDays }, lat, lng, null)
+  const [, todayTimes] = splitHoursLine(convertHoursLine(lines[today] ?? '', timeFormat))
+  const todayLabel = todayTimes && !isUnknownHoursLine(todayTimes) ? todayTimes : t('inspector.showHours')
+
+  return (
+    <div className="space-y-2">
+      <Overline>{t('inspector.openingHours')}</Overline>
+      <div className="rounded-lg border border-edge bg-surface overflow-hidden">
+        <button
+          type="button"
+          onClick={() => setExpanded((v) => !v)}
+          aria-expanded={expanded}
+          className="w-full flex items-center gap-2 px-2.5 py-2 text-left hover:bg-surface-hover transition-colors"
+        >
+          <Clock className="w-3.5 h-3.5 shrink-0 text-content-faint" />
+          <span className="min-w-0 flex-1">
+            <span className="block text-caption font-medium text-content truncate">{todayLabel}</span>
+            {openNow !== null && (
+              <span
+                className={`mt-1 inline-flex items-center rounded-full px-1.5 py-px text-caption font-semibold ${
+                  openNow ? 'bg-success-soft text-success' : 'bg-danger-soft text-danger'
+                }`}
+              >
+                {openNow ? t('inspector.opened') : t('inspector.closed')}
+              </span>
+            )}
+          </span>
+          {expanded ? (
+            <ChevronUp className="w-3.5 h-3.5 shrink-0 text-content-faint" />
+          ) : (
+            <ChevronDown className="w-3.5 h-3.5 shrink-0 text-content-faint" />
+          )}
+        </button>
+        {expanded && (
+          <ul className="border-t border-edge-faint px-2.5 py-1.5 space-y-0.5">
+            {lines.map((line, i) => {
+              const [day, times] = splitHoursLine(convertHoursLine(line, timeFormat))
+              // No day part means the provider did not phrase it as
+              // "Day: times" — render it whole rather than inventing columns.
+              if (!day) {
+                return (
+                  <li key={i} className="text-caption text-content-muted break-words">
+                    {times}
+                  </li>
+                )
+              }
+              return (
+                <li
+                  key={i}
+                  className={`flex items-baseline justify-between gap-2 text-caption ${
+                    i === today ? 'text-content font-semibold' : 'text-content-muted'
+                  }`}
+                >
+                  <span className="shrink-0">{day}</span>
+                  {/* Wraps rather than truncates: this is the view someone
+                      opened on purpose, and split shifts run long. */}
+                  <span className="min-w-0 text-right break-words tabular-nums">
+                    {isUnknownHoursLine(times) ? '–' : times}
+                  </span>
+                </li>
+              )
+            })}
+          </ul>
+        )}
+      </div>
+    </div>
+  )
+}
+
 const FACT_ICONS: Record<PlaceFact['kind'], typeof ChefHat> = {
  rating: Star,
  cuisine: ChefHat,
@@ -385,7 +540,9 @@ function FactList({ facts, t }: { facts: PlaceFact[]; t: TranslationFn }): React
    <div className="space-y-2">
      <Overline>{t('places.details.facts')}</Overline>
      <div className="flex flex-wrap gap-1.5">
-        {facts.map((fact) => {
+        {facts
+          .filter((fact) => fact.kind !== 'openingHours' && fact.kind !== 'rating')
+          .map((fact) => {
          const Icon = FACT_ICONS[fact.kind]
          const label = fact.value || t(`places.details.fact.${fact.kind}`)
          const body = (
@@ -5,6 +5,7 @@ import { mapsApi } from '../../api/client'
 import { useAuthStore } from '../../store/authStore'
 import { useCanDo } from '../../store/permissionsStore'
 import { useTripStore } from '../../store/tripStore'
+import { useSettingsStore } from '../../store/settingsStore'
 import { useAddonStore } from '../../store/addonStore'
 import CollectionPicker from '../Collections/CollectionPicker'
 import PlaceDetailsColumn, { type PlaceDetailsSelection } from './PlaceDetailsColumn'
@@ -95,9 +96,10 @@ function usePlaceFormModal(props: PlaceFormModalProps) {
  const acDebounceRef = useRef<ReturnType<typeof setTimeout> | null>(null)
  const acAbortRef = useRef<AbortController | null>(null)
  const toast = useToast()
-  const { t, language } = useTranslation()
+  const { t, language, locale } = useTranslation()
  const { hasMapsKey, placesEnrichEnabled } = useAuthStore()
  const can = useCanDo()
+  const timeFormat = useSettingsStore((s) => s.settings.time_format) || '24h'
  const tripObj = useTripStore((s) => s.trip)
  const canUploadFiles = can('file_upload', tripObj)
  const collectionsEnabled = useAddonStore((s) => s.isEnabled('collections'))
@@ -493,6 +495,8 @@ function usePlaceFormModal(props: PlaceFormModalProps) {
    toast,
    t,
    language,
+    locale,
+    timeFormat,
    hasMapsKey,
    placesEnrichEnabled,
    can,
@@ -625,6 +629,8 @@ export default function PlaceFormModal(props: PlaceFormModalProps) {
          onAdoptDescription={(text) => setForm(prev => ({ ...prev, description: text }))}
          hasDescription={!!form.description.trim()}
          language={language}
+          timeFormat={S.timeFormat}
+          locale={S.locale}
          t={t}
        />
      )}
@@ -30,6 +30,7 @@ import { formatDistance, formatElevation } from '../../utils/units'
 import { getGoogleMapsUrlForPlace } from './placeGoogleMaps'
 import { getOpenStreetMapUrlForPlace } from './placeOpenStreetMap'
 import { resolveOpenNow, resolvePlaceTimeZone, placeWeekdayIndex } from './placeOpenState'
+import { convertHoursLine } from './placeHoursFormat'

 const detailsCache = new Map()

@@ -112,32 +113,6 @@ function getWeekdayIndex(dateStr, timeZone) {
  return jsDay === 0 ? 6 : jsDay - 1
 }

-function convertHoursLine(line, timeFormat) {
-  if (!line) return ''
-  const hasAmPm = /\d{1,2}:\d{2}\s*(AM|PM)/i.test(line)
-
-  if (timeFormat === '12h' && !hasAmPm) {
-    // 24h → 12h: "10:00" → "10:00 AM", "21:00" → "9:00 PM", "Uhr" entfernen
-    return line.replace(/\s*Uhr/g, '').replace(/(\d{1,2}):(\d{2})/g, (match, h, m) => {
-      const hour = parseInt(h)
-      if (isNaN(hour)) return match
-      const period = hour >= 12 ? 'PM' : 'AM'
-      const h12 = hour === 0 ? 12 : hour > 12 ? hour - 12 : hour
-      return `${h12}:${m} ${period}`
-    })
-  }
-  if (timeFormat !== '12h' && hasAmPm) {
-    // 12h → 24h: "10:00 AM" → "10:00", "9:00 PM" → "21:00"
-    return line.replace(/(\d{1,2}):(\d{2})\s*(AM|PM)/gi, (_, h, m, p) => {
-      let hour = parseInt(h)
-      if (p.toUpperCase() === 'PM' && hour !== 12) hour += 12
-      if (p.toUpperCase() === 'AM' && hour === 12) hour = 0
-      return `${String(hour).padStart(2, '0')}:${m}`
-    })
-  }
-  return line
-}
-
 function formatFileSize(bytes) {
  if (!bytes) return ''
  if (bytes < 1024) return `${bytes} B`
@@ -0,0 +1,64 @@
+/**
+ * Formatting for a provider's weekday opening-hours lines.
+ *
+ * These arrive as display text — "Monday: 11:30 AM – 11:00 PM" from Google in
+ * whatever language was requested, "Monday: 09:00-18:00" from the OSM tag
+ * parser. They cannot be computed from (that is what `placeOpenState` and the
+ * structured periods are for), only rendered. Lifted out of PlaceInspector so
+ * the add-place detail column shows hours the same way the inspector does
+ * rather than growing a second, subtly different implementation.
+ */
+
+/** Rewrites a line between 12h and 24h clocks, leaving it alone when it already fits. */
+export function convertHoursLine(line: string, timeFormat: string): string {
+  if (!line) return ''
+  const hasAmPm = /\d{1,2}:\d{2}\s*(AM|PM)/i.test(line)
+
+  if (timeFormat === '12h' && !hasAmPm) {
+    // 24h → 12h: "10:00" → "10:00 AM", "21:00" → "9:00 PM", "Uhr" entfernen
+    return line.replace(/\s*Uhr/g, '').replace(/(\d{1,2}):(\d{2})/g, (match, h, m) => {
+      const hour = parseInt(h)
+      if (isNaN(hour)) return match
+      const period = hour >= 12 ? 'PM' : 'AM'
+      const h12 = hour === 0 ? 12 : hour > 12 ? hour - 12 : hour
+      return `${h12}:${m} ${period}`
+    })
+  }
+  if (timeFormat !== '12h' && hasAmPm) {
+    // 12h → 24h: "10:00 AM" → "10:00", "9:00 PM" → "21:00"
+    return line.replace(/(\d{1,2}):(\d{2})\s*(AM|PM)/gi, (_, h, m, p) => {
+      let hour = parseInt(h)
+      if (p.toUpperCase() === 'PM' && hour !== 12) hour += 12
+      if (p.toUpperCase() === 'AM' && hour === 12) hour = 0
+      return `${String(hour).padStart(2, '0')}:${m}`
+    })
+  }
+  return line
+}
+
+/**
+ * Splits "Monday: 09:00-18:00" into its day and its times.
+ *
+ * On the FIRST separator only, because the times contain colons of their own.
+ * The fullwidth colon is in there because that is what Google returns for
+ * Chinese and Japanese, and splitting on the ASCII one alone left those lines
+ * unsplit and the whole string in the day column.
+ *
+ * A line with no separator at all comes back with an empty day, which callers
+ * render across the full width rather than squeezing into a two-column row.
+ */
+export function splitHoursLine(line: string): [day: string, times: string] {
+  const at = line.search(/[:：]/)
+  if (at < 0) return ['', line.trim()]
+  return [line.slice(0, at).trim(), line.slice(at + 1).trim()]
+}
+
+/**
+ * Whether a line says nothing about that day.
+ *
+ * `parseOpeningHours` writes "Monday: ?" for a day the OSM tag did not cover.
+ * Showing a bare question mark reads like a bug, so callers render a dash.
+ */
+export function isUnknownHoursLine(times: string): boolean {
+  return times.trim() === '?'
+}