feat(mobile): place details block in the place editor

This commit is contained in:
pipipiper
2026-09-22 14:11:42 +08:00
committed by Maurice
parent bb5c5dcfc1
commit 196dc12762
8 changed files with 156 additions and 32 deletions
@@ -277,7 +277,10 @@ describe('PlaceDetailsColumn', () => {
placeEnrichment.mockRejectedValue(new Error('network'))
renderColumn()
expect(await screen.findByText('places.details.error')).toBeInTheDocument()
// The load retries once after 1.5 s before showing the error, so the
// default 1 s findBy timeout is too short.
expect(await screen.findByText('places.details.error', undefined, { timeout: 4000 })).toBeInTheDocument()
expect(placeEnrichment).toHaveBeenCalledTimes(2)
})
it('FE-PDC-014: serves a second look at the same place from cache', async () => {
@@ -51,6 +51,12 @@ interface PlaceDetailsColumnProps {
timeFormat?: string
/** For grouping the rating count's digits. */
locale?: string
/**
* Full width at every breakpoint. The desktop dialog sits the column beside
* the form (sm:w-80); the mobile sheet stacks it inside a scrolling column,
* where a 320 px cap would leave it hugging the left half of a wide screen.
*/
fluid?: boolean
/** False on an instance with no Google key, which is most of them. */
t: TranslationFn
}
@@ -117,6 +123,7 @@ export default function PlaceDetailsColumn({
language,
timeFormat = '24h',
locale = 'en-US',
fluid = false,
t,
}: PlaceDetailsColumnProps): React.ReactElement {
const [data, setData] = useState<MapsPlaceEnrichmentResult | null>(null)
@@ -150,31 +157,48 @@ export default function PlaceDetailsColumn({
setState('loading')
setData(null)
mapsApi
.placeEnrichment(
{
placeId: selection.placeId,
lat: selection.lat,
lng: selection.lng,
name: selection.name,
lang: language,
details: selection.details,
},
controller.signal,
)
.then((result) => {
if (controller.signal.aborted) return
enrichmentCache.set(selectionKey, result)
writeSession(selectionKey, result)
setData(result)
setState('ready')
})
.catch((err: unknown) => {
if (controller.signal.aborted || (err as { code?: string })?.code === 'ERR_CANCELED') return
setState('error')
})
// One quiet retry before the error state: on a phone the first attempt
// regularly dies to a network blip rather than a real failure, and an
// error card the user can only clear by re-picking the place is worse than
// a second and a half of extra spinner.
let retried = false
let retryTimer: ReturnType<typeof setTimeout> | null = null
const run = () => {
mapsApi
.placeEnrichment(
{
placeId: selection.placeId,
lat: selection.lat,
lng: selection.lng,
name: selection.name,
lang: language,
details: selection.details,
},
controller.signal,
)
.then((result) => {
if (controller.signal.aborted) return
enrichmentCache.set(selectionKey, result)
writeSession(selectionKey, result)
setData(result)
setState('ready')
})
.catch((err: unknown) => {
if (controller.signal.aborted || (err as { code?: string })?.code === 'ERR_CANCELED') return
if (!retried) {
retried = true
retryTimer = setTimeout(run, 1500)
return
}
setState('error')
})
}
run()
return () => controller.abort()
return () => {
controller.abort()
if (retryTimer) clearTimeout(retryTimer)
}
// selectionKey folds in the place id (or its coordinates) and the language,
// which is everything the answer depends on. Depending on `selection` itself
// would refetch whenever the object identity changes without the place
@@ -198,7 +222,7 @@ export default function PlaceDetailsColumn({
// tiles, which is too small to tell a facade from a foyer. The column is
// stretched to the form's height by the row it sits in, so the extra room
// costs nothing that was being used.
<aside className="w-full sm:w-80 shrink-0 flex flex-col rounded-xl border border-edge bg-surface-secondary overflow-hidden self-stretch">
<aside className={`w-full ${fluid ? '' : 'sm:w-80'} shrink-0 flex flex-col rounded-xl border border-edge bg-surface-secondary overflow-hidden self-stretch`}>
<div className="flex items-center gap-2 px-3 py-2.5 border-b border-edge shrink-0">
<Landmark size={15} className="text-accent" />
<span className="text-body font-semibold text-content">{t('places.details.title')}</span>
@@ -922,10 +922,11 @@ export default function PlaceFormModal(props: PlaceFormModalProps) {
// Desktop + Collections addon → the saved-place picker on the right. Mobile
// always keeps the original single-column form untouched.
const twoColumn = !isMobile && collectionsEnabled
// The detail column sits on the left on desktop whenever enrichment is on. It
// The detail column sits on the left on desktop whenever enrichment is on; on
// mobile it stacks above the form (the aside is w-full below sm already). It
// stays mounted with the selection null rather than appearing on the first
// pick — otherwise the dialog would jump sideways mid-typing.
const showDetails = !isMobile && placesEnrichEnabled
const showDetails = placesEnrichEnabled
const modalSize = isMobile ? 'lg' : showDetails && twoColumn ? '5xl' : showDetails || twoColumn ? '4xl' : 'lg'
const descriptionRef = useRef<HTMLTextAreaElement | null>(null)
const notesRef = useRef<HTMLTextAreaElement | null>(null)
@@ -957,7 +958,7 @@ export default function PlaceFormModal(props: PlaceFormModalProps) {
</div>
}
>
<div className={twoColumn || showDetails ? 'flex gap-5 items-stretch' : ''}>
<div className={twoColumn || showDetails ? (isMobile ? 'flex flex-col gap-5' : 'flex gap-5 items-stretch') : ''}>
{showDetails && (
<PlaceDetailsColumn
selection={detailsSelection}
@@ -403,8 +403,8 @@ export default function PlaceInspector({
{/* Content — scrollable */}
<div data-testid="inspector-scroll" style={{ flex: '1 1 auto', minHeight: 0, overflowY: 'auto', WebkitOverflowScrolling: 'touch', overscrollBehavior: 'contain', padding: '12px 16px', display: 'flex', flexDirection: 'column', gap: 10 }}>
{/* Info-Chips — hidden on mobile, shown on desktop */}
<div className="hidden sm:flex" style={{ flexWrap: 'wrap', gap: 6, alignItems: 'center' }}>
{/* Info-Chips — wraps on narrow screens; the review snippet inside stays desktop-only */}
<div className="flex" style={{ flexWrap: 'wrap', gap: 6, alignItems: 'center' }}>
{googleDetails?.rating && (() => {
const shortReview = (googleDetails.reviews || []).find(r => r.text && r.text.length > 5)
return (
@@ -14,6 +14,10 @@ import PlPlaceSearch, { type PlSearchPick } from './PlPlaceSearch'
import PlCategoryPicker from './PlCategoryPicker'
import PlTimeFields from './PlTimeFields'
import PlFileAttach from './PlFileAttach'
import PlaceDetailsColumn, { type PlaceDetailsSelection } from '../../../../components/Planner/PlaceDetailsColumn'
import { useTranslation } from '../../../../i18n'
import { useAuthStore } from '../../../../store/authStore'
import { useSettingsStore } from '../../../../store/settingsStore'
import type { Assignment, AssignmentsMap, Place } from '../../../../types'
import type { TripPlanner } from '../MTripShell'
import { useLocationBias } from '../../../../hooks/useLocationBias'
@@ -99,6 +103,14 @@ export default function MPlaceEditSheet({ planner, onOpenExpense }: MPlaceEditSh
const [sheetPlace, setSheetPlace] = useState<Place | null>(null)
const [sheetAssignmentId, setSheetAssignmentId] = useState<number | null>(null)
// The details block under the search field follows the same selection rules
// as the desktop dialog's left column: the picked search result, else the
// place being edited, else whatever a map POI prefilled.
const [detailsSelection, setDetailsSelection] = useState<PlaceDetailsSelection | null>(null)
const placesEnrichEnabled = useAuthStore(s => s.placesEnrichEnabled)
const { language, locale } = useTranslation()
const timeFormat = useSettingsStore(s => s.settings.time_format) || '24h'
// Live rather than the open-time snapshot, so the note's dirty check compares against
// the note as it stands when Save is tapped.
const ctxAssignment = useMemo(
@@ -155,6 +167,26 @@ export default function MPlaceEditSheet({ planner, onOpenExpense }: MPlaceEditSh
} else {
setForm(DEFAULT_FORM)
}
// Same source order as the desktop dialog: an existing place's provider id
// and coordinates, a map POI's prefill, or nothing. Without this the block
// would keep showing the previous sheet's place.
if (editingPlace && editingPlace.lat != null && editingPlace.lng != null) {
setDetailsSelection({
placeId: editingPlace.google_place_id || editingPlace.amap_poi_id || editingPlace.osm_id || undefined,
lat: Number(editingPlace.lat),
lng: Number(editingPlace.lng),
name: editingPlace.name || '',
})
} else if (prefillCoords) {
setDetailsSelection({
placeId: prefillCoords.osm_id || undefined,
lat: prefillCoords.lat,
lng: prefillCoords.lng,
name: prefillCoords.name || '',
})
} else {
setDetailsSelection(null)
}
// A fresh sheet owns nothing yet; one opened on a map POI owns whatever
// that POI filled in. An existing place being edited owns nothing either —
// everything on that form came out of the database.
@@ -188,6 +220,21 @@ export default function MPlaceEditSheet({ planner, onOpenExpense }: MPlaceEditSh
// airport and then a station left the airport's website in the field.
const applyPick = (pick: PlSearchPick) => {
setForm(prev => mergeResult(prev, pick as unknown as Record<string, unknown>, autoFilledRef.current))
// The details block hangs off the same pick, like the desktop column. A
// pick without usable coordinates leaves the previous selection alone.
const lat = Number.parseFloat(pick.lat ?? '')
const lng = Number.parseFloat(pick.lng ?? '')
if (Number.isFinite(lat) && Number.isFinite(lng)) {
setDetailsSelection({
placeId: pick.google_place_id || pick.amap_poi_id || pick.osm_id || undefined,
lat,
lng,
name: pick.name || '',
// Hand the record along: the server needs the same record for the
// enrichment call, and looking it up again costs a provider round trip.
details: pick.details,
})
}
}
const handleClose = () => {
@@ -310,6 +357,23 @@ export default function MPlaceEditSheet({ planner, onOpenExpense }: MPlaceEditSh
<div className="min-h-0 flex-1 overflow-y-auto px-[18px] pb-[6px] pt-[2px]" onPaste={handlePaste}>
<PlPlaceSearch planner={planner} locationBias={locationBias} onPick={applyPick} onResolvingChange={setResolvingPick} />
{placesEnrichEnabled && (
<div className="mt-3">
<PlaceDetailsColumn
selection={detailsSelection}
selectedImageUrl={form.image_url}
onPickImage={(url) => setForm(prev => ({ ...prev, image_url: url ?? undefined }))}
onAdoptDescription={(text) => setForm(prev => ({ ...prev, description: text }))}
hasDescription={!!form.description.trim()}
language={language}
timeFormat={timeFormat}
locale={locale}
fluid
t={t}
/>
</div>
)}
<Eyebrow className="mb-[5px] mt-3 uppercase">{t('places.formName')} *</Eyebrow>
<input
type="text"
@@ -23,6 +23,13 @@ export interface PlSearchPick {
amap_poi_id?: string
website?: string
phone?: string
/**
* The full record the pick came from. mergeResult ignores unknown keys, so
* this rides along for free, and the details block hands it to the server so
* the enrichment call can skip its own details lookup — one fewer provider
* round trip on a phone network.
*/
details?: MapsPlace
}
interface Suggestion {
@@ -72,6 +79,7 @@ function placeToPick(place: MapsPlace): PlSearchPick {
amap_poi_id: s(place.amap_poi_id),
website: s(place.website),
phone: s(place.phone),
details: place,
}
}
@@ -5,12 +5,13 @@ import type { TripPlanner } from '../../../../src/mobile/screens/trip/MTripShell
import type { Assignment, Category, Place } from '../../../../src/types'
import { buildPlanner } from '../../../helpers/mobileTrip'
import { useAddonStore } from '../../../../src/store/addonStore'
import { useAuthStore } from '../../../../src/store/authStore'
import { useTripStore } from '../../../../src/store/tripStore'
import { server } from '../../../helpers/msw/server'
import { resetAllStores, seedStore } from '../../../helpers/store'
import { fireEvent, render, screen, waitFor } from '../../../helpers/render'
// FE-MOB-PLEDIT-001 to FE-MOB-PLEDIT-040, plus the 009b, 025b and 029b variants
// FE-MOB-PLEDIT-001 to FE-MOB-PLEDIT-042, plus the 009b, 025b and 029b variants
// planner.t echoes the key, so every label/placeholder is asserted as its key.
const CATEGORIES = [
@@ -543,4 +544,24 @@ describe('MPlaceEditSheet', () => {
expect(warning).not.toHaveTextContent('ENEOS')
})
})
it('FE-MOB-PLEDIT-041: shows the details block for the place under edit', async () => {
server.use(
http.post('/api/maps/enrichment', () => HttpResponse.json({
photos: [],
description: { text: 'The oldest temple in Tokyo.', source: 'wikipedia', sourceUrl: null, license: null },
facts: [],
rating: null,
hours: null,
})),
)
setup({ editingPlace: EDITED })
expect(await screen.findByText('The oldest temple in Tokyo.')).toBeInTheDocument()
})
it('FE-MOB-PLEDIT-042: keeps the details block off while the instance has enrichment disabled', () => {
seedStore(useAuthStore, { placesEnrichEnabled: false })
setup({ editingPlace: EDITED })
expect(screen.queryByText('places.details.title')).not.toBeInTheDocument()
})
})
@@ -131,6 +131,9 @@ describe('PlPlaceSearch', () => {
osm_id: 'W7444,',
website: 'https://louvre.fr',
phone: '+33 1',
// The full record rides along so the details block can hand it to the
// enrichment call and skip the server's own details lookup.
details: LOUVRE,
})
expect(input).toHaveValue('')
})